Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mhr summary page remove alert bar #1629

Merged

Conversation

RuoxuanPengBC
Copy link
Contributor

Issue #: /bcgov/entity#18341

Description of changes:

  1. removed alert bar of mhr information page
  2. updated the test to align the new design

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the PPR license (Apache 2.0).

ppr-ui/src/views/mhrInformation/MhrInformation.vue Outdated Show resolved Hide resolved
@@ -587,9 +591,7 @@ export default defineComponent({
alertMsg: computed((): string => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prop name should be updated, since there is no more message but only the date.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see it modified.

ppr-ui/src/resources/unitNotes.ts Show resolved Hide resolved
@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-assets-dev--pr-1629-zfnd2uav.web.app

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Merging #1629 (093b20c) into main (9255e77) will increase coverage by 4.86%.
Report is 331 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1629      +/-   ##
==========================================
+ Coverage   72.35%   77.21%   +4.86%     
==========================================
  Files         307      381      +74     
  Lines       12767     7054    -5713     
  Branches     2630     1130    -1500     
==========================================
- Hits         9237     5447    -3790     
+ Misses       3518     1575    -1943     
- Partials       12       32      +20     
Flag Coverage Δ
pprui 77.21% <ø> (+4.86%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
ppr-ui/src/App.vue 100.00% <ø> (+56.78%) ⬆️
ppr-ui/src/components/collateral/Collateral.vue 100.00% <ø> (+10.97%) ⬆️
...nents/collateral/generalCollateral/GenColAmend.vue 100.00% <ø> (+18.00%) ⬆️
...onents/collateral/generalCollateral/GenColEdit.vue 100.00% <ø> (+15.15%) ⬆️
...nts/collateral/generalCollateral/GenColSummary.vue 100.00% <ø> (+8.00%) ⬆️
...collateral/generalCollateral/GeneralCollateral.vue 100.00% <ø> (+6.66%) ⬆️
...ts/collateral/vehicleCollateral/EditCollateral.vue 100.00% <ø> (+22.58%) ⬆️
...collateral/vehicleCollateral/VehicleCollateral.vue 100.00% <ø> (+19.41%) ⬆️
...cleCollateral/factories/useCollateralValidation.ts 71.42% <ø> (ø)
...llateral/vehicleCollateral/factories/useVehicle.ts 70.14% <ø> (-0.91%) ⬇️
... and 34 more

... and 283 files with indirect coverage changes

@RuoxuanPengBC
Copy link
Contributor Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-assets-dev--pr-1629-zfnd2uav.web.app

ppr-ui/src/resources/unitNotes.ts Show resolved Hide resolved
@@ -587,9 +591,7 @@ export default defineComponent({
alertMsg: computed((): string => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see it modified.

@dimak1
Copy link
Collaborator

dimak1 commented Nov 23, 2023

@RuoxuanPengBC looks good, just minor comment.

@RuoxuanPengBC
Copy link
Contributor Author

@dimak1 Regarding the 'alertMsg', alert bar still exists in other scenario, but no longer exists in residential exemptions. So I created a new variable called exemptDate to replace part of alertMsg.

@dimak1
Copy link
Collaborator

dimak1 commented Nov 23, 2023

@RuoxuanPengBC thanks for clarification, i see it now 👍🏻

@RuoxuanPengBC RuoxuanPengBC force-pushed the fix/mhr-summary-page-remove-alert-bar branch from 937ddfc to 9826c11 Compare November 24, 2023 00:31
@RuoxuanPengBC RuoxuanPengBC force-pushed the fix/mhr-summary-page-remove-alert-bar branch from 9826c11 to 093b20c Compare November 24, 2023 21:40
@dimak1 dimak1 merged commit 0f137f5 into bcgov:main Nov 24, 2023
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants