-
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
(feat) Launching workspaces should prompt user for unsaved forms, especially for workspaces that cannot be collapsed or workspaces with same type #1364
Conversation
Hi @AlexanderMizgirev @icrc-jofrancisco @kdaud , please update the translations. I've used the google translate for the same. |
Size Change: +422 B (0%) Total Size: 8.39 MB ℹ️ View Unchanged
|
Also, in Pt. 4 & 5, shouldn't the exclamation mark appear on the forms widget when switching to the visit note? I thought the idea here was to highlight to the user that they have an unfinished form? |
Yes, it should. But in the screenshot, I didn't open the form, just the forms dashboard, and it cannot hide. If there is a form, the forms will also have the ! mark. |
Actually, I did the changes for the exclamation work after recording the videos for launchPatientWorkspace changes. |
@ibacher , this is supposed to be my last PR for improvement work for workspaces. If you have anything you feel that needs to be done as well in workspaces, please feel free to share the same. |
Screen.Recording.2023-09-19.at.8.31.29.PM.mov |
Nice! |
…ient-chart into workspaces-improvement
Hi @vasharma05 , |
…rs/openmrs-esm-patient-chart into workspaces-improvement
Yes BRAVO Vineet for this thoughtful addition of tests coverage! 👏 |
Thank you @jayasanka-sack and @gracepotma !!! |
if (store.getState().workspaceWindowState === 'hidden') { | ||
updateWorkspaceWindowState( | ||
store.getState().openWorkspaces[0].preferredWindowSize === 'maximized' ? 'maximized' : 'normal', | ||
); | ||
} |
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.
Can we work this update logic into the setState()
cal above? The idea here is to minimise the number of calls to getState()
to minimise the chances of getting inconsistent results, i.e.,
store.setState((state) => {
let workspaceWindowState = state.workspaceWindowState;
if (workspaceWindowState === 'hidden') {
workspaceWindowState = workspaceToBeAdded.preferredWindowSize === 'maximized' ? 'maximized' : 'normal';
}
return { ...state, openWorkspaces: [workspaceToBeADded, ...(restWorkspaces ?? state.openWorkspaces)], workspaceWindowState };
});
(Notice I've also moved the restWorkspaces
into here. Effectively, one call to getState()
and then a single update to the store state).
There are a couple of reasons for this:
- As above, ensuring we have a "consistent" (really, "transactional") view of the workspace state.
- We only update the state once, as a single entity. Every call to
setState()
will ultimately result in an update to the state and then that update being sent to all state subscribers; in essence, this can help avoid multiple React re-renders for a single state change operation.
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.
Right, I totally missed it out.
Thanks for catching this!!!
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.
Please re-review.
Thanks!
Echoing Grace—nice work and huge thanks for all your work on the order basket. Lots of edge cases to get right and I really appreciate your effort to deal with them all in an elegant way. |
LGTM. Thanks @vasharma05 for all the work on this. |
Thank you @brandones and @ibacher ! |
Requirements
Summary
This PR adds handling to the
launchPatientWorkspace
to launch new workspaces on the following workflows:canHide = false
, launching new workspace should confirm the user from opening the new workspace, as it will then close the opened workspace.clinical-form
and then opens another workspace B with typeform
and cannot be collapsed (canHide = false
), and the user tries to open the workspace C, with typeclinical-form
(matching with workspace A), then the user will be shown the first modal to confirm closing the workspace B (which cannot be collapsed), and then the second modal for workspace A, which has the same typeclinical-form
(since no 2 workspaces with same type cannot be opened at the same type).Alongside, the siderail buttons will show
!
when the workspace is opened in the background (not in the front or the workspace is hidden), confirmed with Ciarán.Screenshots
Pt.1
Screen.Recording.2023-09-19.at.3.46.45.PM.mov
Pt. 3
Screen.Recording.2023-09-19.at.3.48.19.PM.mov
Pt.4 & Pt. 5 (P.S. the notes form has the same type that of the clinical form for presentation purpose)
Screen.Recording.2023-09-19.at.3.50.55.PM.mov
Showing
!
when workspace opened in the backgroundScreen.Recording.2023-09-19.at.4.07.52.PM.mov
Related Issue
Other