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.

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

(feat) O3-4198: Use shared page header component in bed management page header #1382

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mccarthyaaron
Copy link
Contributor

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

This PR introduces the shared PageHeader component to be used in the page header of the bed management app, replacing the custom header which was being used before

Screenshots

bed-management-header

Related Issue

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

Copy link

@dilankavishka dilankavishka left a comment

Choose a reason for hiding this comment

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

@denniskigen @mccarthyaaron In the header name should be changed Summary to Bed Management right?

@mccarthyaaron
Copy link
Contributor Author

mccarthyaaron commented Nov 20, 2024

@denniskigen @mccarthyaaron In the header name should be changed Summary to Bed Management right?

@dilankavishka I don't think so. "Summary" shows that we are on the summary page just like "Ward allocation", "Bed type" and "Bed tag". Prior to this PR, "Bed Management" was shown above "Summary" but the PageHeader component uses the implementation name from the configuration (defaulting to "Clinic") in that position where "Bed Management" was. So for now we can't show "Bed Management" where it initially was without first changing the implementation of the PageHeader component. And we can't place it where "Summary" is because that spot is for showing which page the user is currently on. This issue would require further guidance for the right solution

@denniskigen
Copy link
Member

denniskigen commented Nov 21, 2024

@dilankavishka is right. @mccarthyaaron we'd need to tweak a few things to make this possible:

  • Remove the route property from HeaderProps. We can use headerTitle only (or even the more concise title)

  • Refactor all occurrences of route as a prop of <Header>, replacing them with title. So, <Header title={t('bedManagement', 'Bed management')} instead of <Header route="Bed management">

  • Renaming the landing page title to Bed management:

     <section className={styles.section}>
       <Header title={t('bedManagement', 'Bed management')} />
       <BedManagementSummary />
     </section>
  • Adding translation strings for the left panel links in index.ts as comment strings:

     export const summaryLeftPanelLink = getSyncLifecycle(
       // t('summary', 'Summary')
       createLeftPanelLink({
         name: 'bed-management',
         title: 'Summary',
       }),
       options,
     );

@mccarthyaaron
Copy link
Contributor Author

@denniskigen I have pushed some changes. Could kindly take a look

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.

3 participants