-
Notifications
You must be signed in to change notification settings - Fork 226
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-1895 Edit Past Visit button does not work #1269
Conversation
Made this a draft because I've been unable to test the routing update in 9a87890. Also I need to update this with respect to the core change. |
Updated core and tested using Rebased new changes in and the commit/push hooks did some funny things. Dunno why. I committed the results, anyway. |
Anyone know why the build is broken?
|
Could the |
4dfd88c
to
f6701b8
Compare
Size Change: -415 kB (-5%) ✅ Total Size: 7.23 MB
ℹ️ View Unchanged
|
f6701b8
to
e8b4e7a
Compare
Thanks for the fixup @denniskigen ! And @ibacher , that looks like the old UI exactly. Are you sure you got the changes loaded? I was going to try it again but I think I'm running into a bug in the extension renderer. Using |
Ah... yes, we had an issue with some of these apps not rendering for reasons I wasn't entirely clear on. This PR dealt with that problem, but you probably need to update the versions of |
🤦 I just realised I got that result because I had forgotten to run the banner app locally. Sorry for the noise! |
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.
Nice work, @brandones!
Requirements
Summary
Fixes the "edit past visit" functionality. Modifies the active visit tag so you can tell. Fixes active visit tag tooltip styling.
It's split into two commits, the change and a refactor. Review separately for pleasant diffs.
Note that this is not quite right—the Retrospective Entry tag should appear also after clicking "Start visit" and entering a visit in the past. But this requirement maybe needs to be specified a bit better. What if it's a multi-day visit started the previous day, which has not ended yet? Getting the logic right for identifying a retrospective visit is left to O3-2242.
Screenshots
Old:
New:
Related Issue
https://issues.openmrs.org/browse/O3-1895
Other
Depends on openmrs/openmrs-esm-core#724