Skip to content
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.

Sign up for GitHub

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

Jump to bottom

(feat) O3-3782: Use styleguide page header component #1275

Merged
merged 1 commit into from
Sep 2, 2024
Merged

(feat) O3-3782: Use styleguide page header component #1275

merged 1 commit into from
Sep 2, 2024

Conversation

Twiineenock
Copy link
Contributor

@Twiineenock Twiineenock commented Aug 11, 2024 •
edited by denniskigen
Loading

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 conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

A reuseable dashboard header component was implemented in the styleguide here. Therefore this PR adapts all the openmrs-esm-patient-management apps to use this.

Screenshots

Appointments

appointments-header

Patient Lists

patient-lists-pageHeader

Service queues

service-queues-pageHeader

Related Issue

https://openmrs.atlassian.net/browse/O3-3782

Other

@Twiineenock Twiineenock changed the title (chore) O3-3782: Tweak PageHeader to use Styleguide version (feat) O3-3782: Use styleguide page header component in patient-management apps Aug 13, 2024
@Twiineenock Twiineenock changed the title (feat) O3-3782: Use styleguide page header component in patient-management apps (feat) O3-3782: Use styleguide page header component Aug 13, 2024
chibongho
chibongho approved these changes Aug 13, 2024
View reviewed changes
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.

Thanks and nice work!

@denniskigen
Copy link
Member

@mogoodrich @chibongho note that this removes two elements from the page header - the location span and the datepicker shown in the top right. Is that acceptable?

@mogoodrich
Copy link
Member

@mogoodrich @chibongho note that this removes two elements from the page header - the location span and the datepicker shown in the top right. Is that acceptable?

Where are these @denniskigen ? In the very top header? (Maybe we were already hiding them in our app and I forgot they existed)

@denniskigen
Copy link
Member

Yep:

CleanShot 2024-08-13 at 5  10 15@2x

@chibongho
Copy link
Contributor

Internally we talked about those 2 elements not needing to be there for every app. So I think this standardized design change is a step in the right direction.

However... In the Appointments app, we definitely need a way to pick the calendar date. Is there another way to do that with this change?

cc @ojwanganto

@brandones
Copy link
Contributor

@chibongho Yes, you can put anything on the right side of the header as the children of the header component. If they are in the designs, @Twiineenock , please fix the implementation so they are not removed. Otherwise this would be a regression.

@Twiineenock
Copy link
Contributor Author

@chibongho Yes, you can put anything on the right side of the header as the children of the header component. If they are in the designs, @Twiineenock , please fix the implementation so they are not removed. Otherwise this would be a regression.

@brandones , did you mean we retain everything on the right in the patient management as below?

retain

@chibongho
Copy link
Contributor

@chibongho Yes, you can put anything on the right side of the header as the children of the header component. If they are in the designs, @Twiineenock , please fix the implementation so they are not removed. Otherwise this would be a regression.

@brandones , did you mean we retain everything on the right in the patient management as below?

retain

Just the calendar picker. I believe that is the only UI component in the appointments app to be able to change which date we want to view the appointments.

@Twiineenock
Copy link
Contributor Author

Twiineenock commented Aug 21, 2024 •
edited
Loading

Hi @brandones , @chibongho ,

I've committed the changes to ensure that the Datepicker remains on the right side of the page-header in the UI.

datePicker.webm

CC @denniskigen , @mogoodrich

@denniskigen denniskigen force-pushed the chore/use-styleguide-pageHeader branch from 7b30658 to c64736f Compare August 23, 2024 18:14
@denniskigen denniskigen mentioned this pull request Aug 27, 2024
Closed
4 tasks
denniskigen added a commit to openmrs/openmrs-esm-core that referenced this pull request Aug 27, 2024
@denniskigen
(chore) Extend useConfig stub to handle options object
bc85c65 
This commit extends the useConfig stub to accept an optional `options` object which can specify an `externalModuleName`. If the `externalModuleName` is provided, the stub will return a mock config for that external module. Otherwise, it will return the default mock implementation for the current module.
This allows for more flexibility in testing different configurations and modules. More importantly, it should potentially fix a bug where the useConfig stub fails silently when an `externalModuleName` is provided in the options object. See openmrs/openmrs-esm-patient-management#1275 for more information.
@denniskigen denniskigen force-pushed the chore/use-styleguide-pageHeader branch from 4e081b5 to 9849833 Compare September 2, 2024 09:58
@Twiineenock @denniskigen
(feat) Reuse PageHeader component from the styleguide
e1310fe 
This PR refactors page headers for apps in Patient Management to use the PageHeader component from the styleguide.
@denniskigen denniskigen force-pushed the chore/use-styleguide-pageHeader branch from 9849833 to e1310fe Compare September 2, 2024 10:21
denniskigen
denniskigen approved these changes Sep 2, 2024
View reviewed changes
Copy link
Member

@denniskigen denniskigen left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @Twiineenock.

@denniskigen denniskigen dismissed brandones’s stale review September 2, 2024 11:04

Resolved in subsequent commits

@denniskigen denniskigen merged commit d6baed3 into openmrs:main Sep 2, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandones brandones brandones left review comments

@chibongho chibongho chibongho approved these changes

@denniskigen denniskigen denniskigen approved these changes

@mogoodrich mogoodrich Awaiting requested review from mogoodrich

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Twiineenock @denniskigen @mogoodrich @chibongho @brandones