(feat) O3-3122: Improve UI for Queue by status view in the service queues #1158
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -155 kB (-4.26%) Total Size: 3.49 MB
ℹ️ View Unchanged
|
mseaton
requested changes
May 28, 2024
There was a problem hiding this comment.
See my comments. It looks a lot better, but it's not clear from the screenshot or code I reviewed that all of the bullet points listed in the description of https://openmrs.atlassian.net/browse/O3-3122 are addressed. Please address all of the bullet points listed.
packages/esm-service-queues-app/src/patient-queue-header/patient-queue-header.component.tsx
Outdated
Show resolved
Hide resolved
| status={null} | ||
| headerLabel={t('averageWaitingTimeTitle', 'Average waiting time')} | ||
| value={!isNaN(averageWaitTime) ? averageWaitTime : 0} | ||
| /> |
There was a problem hiding this comment.
The metrics cards were intended to be out of scope for this ticket. It is unclear to me what "Average Wait Time" means in a view that displays patients in multiple states.
There was a problem hiding this comment.
I actually went through the designs, but now that I think of it with your statement, it makes sense.
I'll remove this.
Thanks!
packages/esm-service-queues-app/src/views/queue-tables-for-all-statuses.component.tsx
Show resolved
Hide resolved
packages/esm-service-queues-app/src/views/queue-tables-for-all-statuses.component.tsx
Outdated
Show resolved
Hide resolved
| setViewState({ selectedPatientUuid }); | ||
| }, | ||
| }} | ||
| {allowedStatuses?.map((status) => ( |
There was a problem hiding this comment.
The rendering of these need to be reversed. The expectation and designs are for the status to be listed in reverse order.
There was a problem hiding this comment.
What defines the order for the status?
Is it on the basis of some key, because just reverting the keys of the array doesn't make sense and might not be meaningful for other implementations.
There was a problem hiding this comment.
The statuses are returned in a specific order based on the order in which a patient is typically placed in the statuses in a queue. The order is controlled by the order the statuses exist in the underlying concept set. The requirement is to display patients who are already being seen / in progress at the top, followed by patients who are waiting at the bottom. Reversing the statuses is the only consistent way to do this now. Please do it.
mseaton
reviewed
May 29, 2024
There was a problem hiding this comment.
See follow-up comments. Please also review the bullet points in the description here: https://openmrs.atlassian.net/browse/O3-3122
packages/esm-service-queues-app/src/queue-screen/queue-screen.component.tsx
Show resolved
Hide resolved
packages/esm-service-queues-app/src/views/queue-tables-for-all-statuses.component.tsx
Show resolved
Hide resolved
packages/esm-service-queues-app/src/patient-queue-header/patient-queue-header.component.tsx
Outdated
Show resolved
Hide resolved
| setViewState({ selectedPatientUuid }); | ||
| }, | ||
| }} | ||
| {allowedStatuses?.map((status) => ( |
There was a problem hiding this comment.
The statuses are returned in a specific order based on the order in which a patient is typically placed in the statuses in a queue. The order is controlled by the order the statuses exist in the underlying concept set. The requirement is to display patients who are already being seen / in progress at the top, followed by patients who are waiting at the bottom. Reversing the statuses is the only consistent way to do this now. Please do it.
mseaton
reviewed
Jun 5, 2024
There was a problem hiding this comment.
One minor comment on the dashboard title config @vasharma05 . I think this generally looks good, would be good to see an updated gif showing the UI that highlights the improvements requested in the ticket. I can't tell if the existing screenshots attached to this PR are old or new.
3 tasks
|
HI @mseaton! |
mseaton
approved these changes
Jun 25, 2024
packages/esm-service-queues-app/src/views/queue-tables-for-all-statuses.component.tsx
Show resolved
Hide resolved
…ll implementation of the same
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Summary
Improved UI for "Queue entries by Queue and status" in the service queues dashboard.
Screenshots
Screen.Recording.2024-06-23.at.15.10.13.mov
Related Issue
https://issues.openmrs.org/browse/O3-3122
Other
None