(fix) O3-5368 Appointments - make appointment table always show rows …#2275
Merged
(fix) O3-5368 Appointments - make appointment table always show rows …#2275
Conversation
Contributor
|
Size Change: +114 B (0%) Total Size: 8.17 MB ℹ️ View Unchanged
|
chibongho
requested review from
brandones,
denniskigen,
ibacher,
mogoodrich and
mseaton
…of configured status
chibongho
force-pushed
the
appointments-table
branch
2 times, most recently
from
3dd3c1e to
851c2e1
Compare
brandones
approved these changes
Feb 19, 2026
Contributor
brandones
left a comment
brandones
left a comment
There was a problem hiding this comment.
This is a good change, both fixing a bug and aligned with my sense of what best practices are (or ought to be).
However, I want to note this ongoing conversation: #2151 that actually has some follow-up in a DM convo between Eudson and me. Basically no one knows why we have this panel system any more. I made several attempts to get in touch with Palladium about it, since they are the only org that might possibly be using it, and have never heard back from anyone there.
So I am in favor of replacing all this complexity, which I suspect is overwrought, with a much simpler appointments table. This was also part of the specification for last summer's rework of the home/queues/appointments UI, in this ticket: https://openmrs.atlassian.net/browse/O3-4814 . That reduction in complexity would close the door to bugs like this one.
Regarding the framework, it seems like this is caused by imperfect prop updating across parcel boundaries. I'm not sure if we're doing something wrong on our side. This is something that would be fixed, at least in React-extension-in-React-app cases like this, by framework-native mounting.
Contributor
Author
|
Thanks. I'm also in favor of replacing this complexity with a combined table. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
…of configured status
Requirements
Summary
The issue
Sometimes, the
expected-appointments-panelshows all appointments instead of just appointments of status "Scheduled". In this video, you can tell that that is happening by 1) the rows being display, and 2) the title sometimes being shown as "Appointments" instead of "Expected Appointments".video_1280.mp4
Why?
According to this,
extension.configisnulluntil the slot is mounted. That means that in theExtensionWrapper, thetitleandstatuspassed in to the<Extension>could be null, causing this issue.However, I cannot explain why that making
<Extension>only renderif extension.config != nullis an insufficient fix (I included it in this PR anyway). In my testing, somehow thestatusandtitlepassing in to<Extension>could still be null. It seems like the only safe way to read the extension's config inside the extension is to useuseConfig(), rather than being passed in to<Extension>. This PR refactors the code do that.I also tried to fix this issue at the core level but failed. I suspected that we need to have
stateinuseMemo/useCallbackdependency array here and here, but 1) I cannot repro this bug when I runyarn run:shellfrom core because of timing issues, and 2) addingstatein the useCallback (2nd link) seems to cause infinite re-rendering.Testing done
Repeated refresh the appointments tab to verify that the table always shows "Expected Appointments"
Screenshots
Related Issue
https://issues.openmrs.org/browse/O3-5368
Other