-
Notifications
You must be signed in to change notification settings - Fork 70
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
21983 - Enhance Staff Review Table #2928
21983 - Enhance Staff Review Table #2928
Conversation
Signed-off-by: Qin <[email protected]>
Signed-off-by: Qin <[email protected]>
Signed-off-by: Qin <[email protected]>
auth-web/src/components/auth/staff/continuation-application/ContinuationApplicationTable.vue
Show resolved
Hide resolved
auth-web/src/components/auth/staff/continuation-application/ContinuationApplicationTable.vue
Outdated
Show resolved
Hide resolved
auth-web/src/components/auth/staff/continuation-application/ContinuationApplicationTable.vue
Outdated
Show resolved
Hide resolved
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.
I left a few comments. Overall looks good. Is this all the code changes (other than maybe some extra unit tests)?
Yes, these are all the changes. I will fix the issues. |
Signed-off-by: Qin <[email protected]>
Signed-off-by: Qin <[email protected]>
auth-web/src/components/auth/staff/continuation-application/ContinuationApplicationTable.vue
Show resolved
Hide resolved
Please update the app version :) |
|
Signed-off-by: Qin <[email protected]>
Looks OK to me but double-check with Andy. |
I think the date filter should display a date picker (or 2 date pickers to select a range), and the URL parameters should be in yyyy-mmm-dd format. Did you find any examples that do this? If it's easy, do it in this ticket. Otherwise, create another ticket. |
I find an example here: |
https://github.com/bcgov/ppr/tree/main/ppr-ui Look in views/ for the dashboard. |
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.
LGTM
Arwen, it looks like you want to implement the date filter before this gets merged, right? |
Yes, I'm working on the time picker now :) |
Signed-off-by: Qin <[email protected]>
Signed-off-by: Qin <[email protected]>
|
/gcbrun |
Temporary Url for review: https://bcregistry-account-dev--pr-2928-f8q1wi1m.web.app |
*Issue #:*21983
bcgov/entity#21983
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 sbc-auth license (Apache 2.0).