(feat) O3-4200 Make visit form usable in other apps #2134
Merged
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
3 tasks
chibongho
requested review from
denniskigen,
ibacher,
mseaton,
mogoodrich,
ojwanganto and
donaldkibet
mogoodrich
approved these changes
Dec 6, 2024
| name="visit-form-queue-slot" | ||
| patientUuid={patientUuid} | ||
| visitFormOpenedFrom={openedFrom} | ||
| setOnVisitCreatedOrUpdatedCallbacks={setOnVisitCreatedOrUpdatedCallbacks} |
There was a problem hiding this comment.
More a question than a request for a change... why do the queues and appointments have specific extension slots, rather than a single "visit-form-extension-slot" that both queues and appointments insert into? As we have it now, if another ESM wanted to insert it's own functionality into the visit form, would it have to add another extension slot to the form?
There was a problem hiding this comment.
Yeah, with this change, those 2 extensions look more similar than before. I'm keeping it as is because where the extensions appear are different (the appointments extension is near the top, close to where the visit start time fields are, vs the queues extension is near the bottom). It might be possible to slot them into one single slot (and keeping the existing ordering) if we slice up the visit form further so that even the visit form fields are extensions themselves , but I think that's a pretty drastic change.
There was a problem hiding this comment.
I don't love that the slots are named after specific apps ... It kind of violates the idea that slots are just a location on a screen that extensions can appear. I'm kind of ok with having multiple slots if it makes sense, e.g., there are two places on a screen we want extensions to appear.
There was a problem hiding this comment.
Ok, I changed the slot named to visit-form-top-slot and visit-form-bottom-slot
There was a problem hiding this comment.
Thanks @chibongho and @ibacher !
ibacher
approved these changes
Dec 6, 2024
| name="visit-form-queue-slot" | ||
| patientUuid={patientUuid} | ||
| visitFormOpenedFrom={openedFrom} | ||
| setOnVisitCreatedOrUpdatedCallbacks={setOnVisitCreatedOrUpdatedCallbacks} |
There was a problem hiding this comment.
I don't love that the slots are named after specific apps ... It kind of violates the idea that slots are just a location on a screen that extensions can appear. I'm kind of ok with having multiple slots if it makes sense, e.g., there are two places on a screen we want extensions to appear.
|
Size Change: -301 kB (-1.85%) Total Size: 16 MB
ℹ️ View Unchanged
|
|
Thanks so much @chibongho for standardizing the UX. It makes a lot of sense to me. I know this may sound off at this point but do you think we can provide better wordings for the visit attributes? I know most users will not even know what visit attributes mean. |
ojwanganto
approved these changes
Dec 6, 2024
"Additional visit information" maybe? |
|
@ojwanganto Sure. Easy change. |
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
This PR refactors the patient chart visit form for it to be re-usable in different apps (like appointments and queues), without needing logic specific to those apps in the visit form. See corresponding changes in esm-patient-management here. While some changes are opinionated, there is no breaking change.
Despite having extension slots for queues and appointments UIs (which can be turned on via config), the visit form still contains code related to submitting queue entry and checking in an appointment. This change provides a way for those extensions to supply their own callbacks to run after a visit has been created / updated. This allows us to decouple the visit form from any unrelated logic.
Notes:
showServiceQueueFieldsandshowUpcomingAppointmentsto specify whether those corresponding extensions should show, but now, the code to actually check for those values live within the extensions themselves. Arguably, we should make a breaking change to have those configs be within their respective apps.extra-visit-attribute-slot, currently used by the Billing app, could benefit from a similar refactoring as other extension slots to supply its own onVisitCreatedOrUpdated callback, but I'm leaving it untouched for now as I'm not that familiar with that code base.Screenshots
The visit form, when opened from the queues app:
On submit:
Related Issue
https://issues.openmrs.org/browse/O3-4200
Other