Skip to content

O3-4814 Appointments: Show patients with all statuses in one big appointments table; remove status selector bar; add status filter dropdown#2151

Open
EDSONZ-WASSWA wants to merge 17 commits intoopenmrs:mainfrom
EDSONZ-WASSWA:O3-4814-appointments-show-patients-with-all-statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown
Open

O3-4814 Appointments: Show patients with all statuses in one big appointments table; remove status selector bar; add status filter dropdown#2151
EDSONZ-WASSWA wants to merge 17 commits intoopenmrs:mainfrom
EDSONZ-WASSWA:O3-4814-appointments-show-patients-with-all-statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown

Conversation

@EDSONZ-WASSWA
Copy link
Contributor

@EDSONZ-WASSWA EDSONZ-WASSWA commented Dec 5, 2025

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work is based on designs, which are linked or shown either in the Jira ticket or the description below.
  • My work includes tests or is validated by existing tests.

Summary

Worked on the Appointments table,
Instead showing appointments with all statuses in one table. That table then has a “Filter by status” dropdown.

Screenshots

After
final attachement
fixed-Add "All" status filter option
finepic

before
image-attachement

Related Issue

Other

One PR for both tickets below
https://openmrs.atlassian.net/browse/O3-4815
https://openmrs.atlassian.net/issues/?filter=-1&selectedIssue=O3-4814

@denniskigen denniskigen requested a review from brandones December 8, 2025 11:56
@denniskigen
Copy link
Member

Could you review this when you get the chance, @brandones?

@brandones
Copy link
Contributor

Sure, thanks for the ping @denniskigen .

@EDSONZ-WASSWA could you please make the default status in the filter dropdown "All"? When "All" is selected, the table should include a column showing the status. Let me know if you need clarification.

@EDSONZ-WASSWA EDSONZ-WASSWA force-pushed the O3-4814-appointments-show-patients-with-all-statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown branch from ae88d96 to cb4687c Compare December 8, 2025 21:06
@EDSONZ-WASSWA EDSONZ-WASSWA reopened this Dec 8, 2025
…statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown
@EDSONZ-WASSWA
Copy link
Contributor Author

@brandones I have made the fix as you requested

@brandones
Copy link
Contributor

@EDSONZ-WASSWA this looks obviously bad

image

It looks like there are a bunch of style overrides that are probably causing problems. Much like human newbie devs, AI likes to add a lot of unnecessary CSS, and you should generally try to delete as much of it as you can. It's amazing how many problems you can fix by deleting CSS other people (or machines) wrote.

Also it says "All Appointments Appointments".

Look for reference at the service queues one. The top bar there is white rather than grey, but we can still get some sense of how the elements are supposed to look together:

image

Try to make it look decent.

Independent of this PR we should open a conversation with Ciarán, since Appointments has diverged substantially from the designs, but also the designs don't have a search bar at all, nor any filters.

Copy link
Contributor

@brandones brandones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it not look bad

…statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown
…-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown' of https://github.com/EDSONZ-WASSWA/openmrs-esm-patient-management into O3-4814-appointments-show-patients-with-all-statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown
@EDSONZ-WASSWA
Copy link
Contributor Author

Make it not look bad
@brandones I have removed the unnecessary scss as you requested,
hope this looks good

All-atachement1 I tried following the service queues fully

@brandones
Copy link
Contributor

That is improved. A lot of what makes it look bad, you'll notice, is the bottom border getting chopped off:

Screenshot 2025-12-10 at 9 14 32 AM

The bottom border should be continuous up until the Download button.

@EDSONZ-WASSWA
Copy link
Contributor Author

That is improved. A lot of what makes it look bad, you'll notice, is the bottom border getting chopped off:

Screenshot 2025-12-10 at 9 14 32 AM The bottom border should be continuous up until the Download button.

@brandones any more fix needed here?
finepic

@brandones
Copy link
Contributor

Why is 'expected' lower case now?

@brandones
Copy link
Contributor

Please look at the results yourself and ask yourself, "Do I see any problems? Does the text look right?" and then if you do not see problems, post here. Please don't make me identify obvious issues for you.

…ith-all-statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown
…ith-all-statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown
@EDSONZ-WASSWA
Copy link
Contributor Author

Why is 'expected' lower case now?

@brandones I have taken time to go throughout everything. I do not expect any error or wrong spelling
finished
I think it's ready to merge

status?: string;
status?: string | null;
title: string;
// Optional props for status dropdown (I passed from Extension state)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindly this comment doesn’t add value at this point. Please remove it to improve code readability.

location: appointment.location?.name ?? '--',
provider: appointment.providers?.[0]?.name ?? '--',
status: <AppointmentActions appointment={appointment} />,
_appointment: appointment, // for expansion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

shouldShow ? showExtensionTab(extension.name) : hideExtensionTab(extension.name);
}, [extension, dateType, showExtensionTab, hideExtensionTab]);

// When "all" is selected, show only the first extension with null status to get all appointments
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

startDate: startDate,
endDate: endDate,
};
// .Only include status in request body if it's not null/undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@brandones
Copy link
Contributor

Thanks @jwnasambu . The code looks really bad, tons of prop threading etc. But before we rework it I'm waiting to hear back from Palladium about whether we actually need this "ExtensionWrapper" stuff. Hopefully we can just get rid of it and simplify all this code a lot. https://openmrs.slack.com/archives/C039BLMGMMX/p1765473129593799

@EDSONZ-WASSWA EDSONZ-WASSWA force-pushed the O3-4814-appointments-show-patients-with-all-statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown branch from 0e6a7b7 to 99e92fd Compare December 18, 2025 18:32
…statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown
@chibongho
Copy link
Contributor

Thanks @jwnasambu . The code looks really bad, tons of prop threading etc. But before we rework it I'm waiting to hear back from Palladium about whether we actually need this "ExtensionWrapper" stuff. Hopefully we can just get rid of it and simplify all this code a lot. https://openmrs.slack.com/archives/C039BLMGMMX/p1765473129593799

I think enough time has passed that we should just get rid of the ExtensionWrapper. IIRC, it's used to determine whether the "Expected / Competed / Missed Appointments" tabs should be displayed. With this PR merging them all into one big table (with filter), I don't think we need it anymore.

…statuses-in-one-big-appointments-table-remove-status-selector-bar-add-status-filter-dropdown
Copy link
Contributor

@chibongho chibongho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs more work:

  • There are compilation errors.
    • I don't think we can define additional fields like _appointment field for the row object. (I'd love for that to be the case though)
  • Remove <ExtensionWrapper> and its corresponding <ExtensionSlot>, since those were only needed to support the different tabs, which are going away.
  • Remove this whole schema that configures the tabs, since those are no longer in place.
  • Figure out what to do with the "Early appointments" tab. See ticket comment.
  • Figure out whether to have the filter selections be sticky across date switch / page refresh. See ticket comment.
  • The "empty state" that we show within a table looks different than the one in the queues table. See screenshots.
    • To be fair, I'm not 100% sure which one is the one we want. But right, now it has extra white padding, and the message "to display to display" is incorrect.
image image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants