-
Notifications
You must be signed in to change notification settings - Fork 18
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
Todo Section - Dialogs #28
Conversation
/gcbrun |
Temporary Url for review: https://business-dashboard-dev--pr-28-720ty29n.web.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general OK.
Big question is is there not going to be internationalization (i18n) for these dialogs ?
Also 2 small ones
src/components/bcros/todo/List.vue
Outdated
const loadAffiliationInvitationError = ref(todosStore.loadAffiliationsError.length > 0) | ||
|
||
const authorizeAffiliationErrorOptions: DialogOptionsI = { | ||
title: 'Error updating affiliation invitation.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this going to be internationalized ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I agree I think this should all be from the lang file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many texts in the dashboard are from the imported enums or functions, so the content can't be fully internationalized until those texts from the external source are updated. But I agree that we need to use the lang file in the new dashboard.
I have updated the i18n stuff for the dialogs. There are many other places in the todo section that i18n need to be applied. I will change that later in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a ticket if you dont have it, for the next PR
src/components/bcros/todo/List.vue
Outdated
attach="#todoList" | ||
:display="loadAffiliationInvitationError" | ||
:options="loadAffiliationInvitationErrorOptions" | ||
@close="loadAffiliationInvitationError = false" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to clear error queue if they are acknowledged and have this as a computed value ?
Was there a reason, why the errors should stay in the queue after closing the error dialog ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Updated
/gcbrun |
Temporary Url for review: https://business-dashboard-dev--pr-28-720ty29n.web.app |
Looks good to me now! Thanks for updating texts. |
*Issue:*bcgov/entity#21352
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).