-
Notifications
You must be signed in to change notification settings - Fork 248
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) O3-2716: Prompt users to close workspaces when navigating from the patient chart #1607
Conversation
…orkspaces are open
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.
this looks good. Maybe a screenshot should be added
const navigateToNewUrl = () => { | ||
function getUrlWithoutPrefix(url: string) { | ||
return url.split(window['getOpenmrsSpaBase']())?.[1]; | ||
} | ||
navigate({ to: `\${openmrsSpaBase}/${getUrlWithoutPrefix(newUrl)}` }); | ||
}; | ||
|
||
closeAllWorkspaces(navigateToNewUrl); |
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.
Why is this not adequate?
const navigateToNewUrl = () => { | |
function getUrlWithoutPrefix(url: string) { | |
return url.split(window['getOpenmrsSpaBase']())?.[1]; | |
} | |
navigate({ to: `\${openmrsSpaBase}/${getUrlWithoutPrefix(newUrl)}` }); | |
}; | |
closeAllWorkspaces(navigateToNewUrl); | |
closeAllWorkspaces(() => navigate({ to: newUrl });); |
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 referred this in the patient registration from Florian.
The newUrl
contains the domain as well, hence it might not be the same handling by single-spa (navigating to /home vs navigating to dev3.openmrs.org/openmrs/spa/home
). Navigate function calls the single-spa's navigation.
|
||
closeAllWorkspaces(navigateToNewUrl); | ||
} else { | ||
resetWorkspaceStore(); |
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.
You should not be using resetWorkspaceStore
, per the docstring for that function. What is your intention? Why should the workspace state be cleared?
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.
Since the user is leaving the patient chart, the workspace can be reset to the initial state.
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!
Thanks!
Requirements
Summary
Screenshots
add_prompt-on-navigating-from-patient-chart-with-open-workspaces.mp4
Related Issue
O3-2716
Other