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

Fix styling for filing history items #77

Merged
merged 5 commits into from
Nov 26, 2024

Conversation

patrickpeinanw
Copy link
Collaborator

*Issue:*bcgov/entity#24391

Description of changes:

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

@patrickpeinanw patrickpeinanw self-assigned this Nov 25, 2024
@patrickpeinanw patrickpeinanw changed the title Development Fix styling for filing history items Nov 25, 2024
Copy link
Collaborator

@hfekete hfekete left a comment

Choose a reason for hiding this comment

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

Looks good, can you look into the removed texts though?

src/components/bcros/filing/common/filedAnd/PendingCoa.vue Outdated Show resolved Hide resolved
@hfekete
Copy link
Collaborator

hfekete commented Nov 25, 2024

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Nov 25, 2024

Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

Please provide examples (screenshots are OK) of what each change looks like. I don't know where are the components are used so it's hard for me to verify. Thanks.

Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

This is unrelated to this ticket, but please a new ticket: in Filings UI, the breadcrumb links can be opened in a new window, but in this UI they cannot. Can they be changed?


The inline stuff looks good at different screen sizes 👍 And simpler code, too.

@patrickpeinanw
Copy link
Collaborator Author

patrickpeinanw commented Nov 26, 2024

@severinbeauvais here are some examples you can use

Todo Section:

Pending Section:

Filing History:

I've also created new ticket to update the breadcrumb links so they can be opened in a new tab and a new window

Comment on lines +284 to +287
"directorChange": {
"title": "File Director Change",
"draftTitle": "Director Change"
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Caught this bug and got it fixed. The i18n path for director change was missing so the todo item title was not rendered properly.

Before:
Screenshot 2024-11-25 at 10 34 06 AM

Now:
Screenshot 2024-11-25 at 5 28 33 PM

Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

LGTM. Can you please fix the failed unit test before merging? Thx.

@patrickpeinanw patrickpeinanw force-pushed the development branch 2 times, most recently from 1648cd7 to a722a4b Compare November 26, 2024 22:43
@patrickpeinanw
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-77-opu1ygfd.web.app

@bcgov bcgov deleted a comment from bcregistry-sre Nov 26, 2024
@bcgov bcgov deleted a comment from patrickpeinanw Nov 26, 2024
@bcgov bcgov deleted a comment from bcregistry-sre Nov 26, 2024
@bcgov bcgov deleted a comment from patrickpeinanw Nov 26, 2024
@bcgov bcgov deleted a comment from bcregistry-sre Nov 26, 2024
@patrickpeinanw patrickpeinanw merged commit 37edbf4 into bcgov:main Nov 26, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants