-
Notifications
You must be signed in to change notification settings - Fork 203
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) O3-2426: Closing drug search panel should send user back to order basket if they previously had it open #1452
Conversation
packages/esm-patient-chart-app/src/workspace/workspace-window.component.tsx
Outdated
Show resolved
Hide resolved
b4feaaa
to
ddd0e30
Compare
…r basket if they previously had it open
ddd0e30
to
c831434
Compare
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.
So this looks correct to me for what’s specified in the ticket. I do think that, eventually, we’ll need to add some more complex state (specifically, if the user just opens the drug order panel, they should get the close button and the close button should restore the previous workspace and a better solution would introduce some sort of “workspace stack”), but this is an improvement on the current behaviour.
It would be nice to also integrate the icon change @vasharma05 suggested in the ticket.
Thanks @jwnasambu for this!
Hi @ibacher ! Instead of either choosing the onClose /closeWorkspace, the closeWorkspace should call the onClose function. |
...sm-patient-medications-app/src/drug-order-basket-panel/drug-order-basket-panel.extension.tsx
Outdated
Show resolved
Hide resolved
The workspace system already operates like a stack, but the trouble is that it only supports one workspace of each type. Since the order-basket and add-drug-order workspaces have the same type, only one can exist in the workspace stack at a time, so when add-drug-order opens it displaces order-basket. Let's weigh the two approaches: onCloseThis is preferable if we always want one workspace to close into another, regardless of where the user came from. So if we want sequences like
which, the latter I'm sure about, the first I don't know. I have forgotten the context in which I made the ticket. But the ticket suggests that we want just the latter, not the first. stackThis is preferable if we want to go to the last workspace only if it was previously open. This is already how it works for workspaces of different types. This is if we want the sequences
If it's the case that we actually want flow (A), then we really want to have the I'm not sure if this will apply to any other workspace flows (e.g. the form workspace; if a user closes a form, should it take them back to the form selector?). If we want to support multiple workspaces of the same type in the stack, maybe we can just add a parameter to |
So, I think going to the last workspace only if it was previously opened makes the most sense. Specifically, I think that if the user opens a workspace from the side rail and then opens a child workspace (form entry / add drug order / add lab order / etc.), when closing the child workspace it should go back to the parent. Conversely, if a "child workspace" (form entry / add drug order / add lab order / etc.) is opened directly from a patient chart component, (e.g., the "Add +" buttons, Actions menus), it seems appropriate to simply close the workspace once that "child workspace" is done. There are some other cases that I think also fit with this pattern, specifically, launching what I've aboved termed "child workspace"s from a workspace that is not normally the workspaces parent. For example, if in a form I want to add a Condition, it would be appropriate to open the "add condition" workspace, but when that workspace is closed, I should be returned to the form, even though the Condition workspace is not normally part of the form side-rail app. Similarly, with the task list, if I have a task to order a medication for a patient, I should be able to click the link, view the add-drug-order workspace and, when I'm done, still be back on the task list. It's hard for me to conceive of a case where the "right" behaviour is always to go back to some default-per-workspace page (e.g., I opened the add-drug-order workspace from the task list, when I'm done with the order, I probably want to be looking at the task list, not the order basket). There are some clear corner cases implied by this, though. For example, if I open the "Add Conditions" workspace from the condition list view, then the workspace should probably open in what the UX docs call the "Independent actions" variant, i.e., with a close button instead of a collapse button. But if I open the "Add Conditions" workspace from within a form (which has a side-rail icon), it should be opened in what the UX docs call the "Siderail" variant, with a collapse button that collapses back to the form entry side rail. (This also means that if I've opened the "Add Conditions" workspace from within a form and collapsed it, when I re-open the form workspace from the side rail, I should see the "Add Conditions" page in the state that I left it in). |
Ok, sounds like let's go ahead with making the workspace stack tolerate multiple workspaces of the same type (when the right param is passed to |
Thanks for your work on this @jwnasambu , sorry about the change in approach. |
Requirements
Summary
I added openDrugSearch function which Launches a patient workspace with the name 'add-drug-order' and passes an object with a property onCloseWorkspace, which is set to the onClose function so that when the 'add-drug-order' workspace is launched, the onClose function closes a workspace with the name 'add-drug-order' and passes true as a parameter. It then launches a patient workspace with the name 'order-basket'.
Screenshots
screencast.2023-11-07.12.AM-54-03.mp4
Related Issue
https://issues.openmrs.org/browse/O3-2426
Other