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

O3-2696 Add ability to configure dispensing app to homepage #94

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

CynthiaKamau
Copy link
Contributor

@CynthiaKamau CynthiaKamau commented Jan 5, 2024

Requirements

  • This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.
  • 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 will add the dispensing app to the home app. It is a configurable link and implementer's who need the link can add it like using :

"@openmrs/esm-home-app": {
    "extensionSlots": {
      "homepage-dashboard-slot": {
        "add": [
          "dispensing-dashboard-link"
        ]
      }
    }
  }

Screenshots

https://www.loom.com/share/8c2e46878666400390841fa726360af1?sid=e7f8c4b3-b74c-4c0a-9c0c-0938fcd9a5e8

Related Issue

Other

@ibacher
Copy link
Member

ibacher commented Jan 5, 2024

I would prefer we create the extension but do not make it part of the default homepage. That way implementers that need this functionality can opt-in, which seems simpler, especially as the dispensing app is not a core EMR feature.

@CynthiaKamau
Copy link
Contributor Author

I would prefer we create the extension but do not make it part of the default homepage. That way implementers that need this functionality can opt-in, which seems simpler, especially as the dispensing app is not a core EMR feature.

Thanks @ibacher , i have made the necessary changes

@ebambo ebambo merged commit f7d2d20 into main Jan 8, 2024
4 checks passed
@ebambo ebambo deleted the home-link branch January 8, 2024 15:12
@mseaton
Copy link
Member

mseaton commented Jan 8, 2024

A bit of a late comment, but I'm curious why this is needed. There is already a link to the dispensing-link component, which is in the app-menu-slot. It seems odd that we'd need to add to the dispensing esm a bunch of components that are specific to the slots into which they might be added. Can't this be driven from configuration in some standard way, or have one component reused?

@ibacher
Copy link
Member

ibacher commented Jan 8, 2024

@mseaton That's a good point. I didn't look too closely. In general the left-menu on the home page should not be a configurable link like this. Configurable links belong in the app-menu-slot. Things on the home page should have a display on the home page rather than redirecting.

@ebambo
Copy link

ebambo commented Jan 8, 2024

@mseaton @ibacher this is how we have designed OHRI from the beginning, you can have a look at this design of the OHRI Home page. I do agree redirects from the home page are not ideal and if we could embed the dispensing (and the other apps a per the design) would be very helpful for us. Having the apps on the app-menu-slot on the top right is not user friendly as per the comments we received when Paul did the user testing / interviews. Happy to discuss this more in-depth in the appropriate channels.

@CynthiaKamau
Copy link
Contributor Author

@mseaton That's a good point. I didn't look too closely. In general the left-menu on the home page should not be a configurable link like this. Configurable links belong in the app-menu-slot. Things on the home page should have a display on the home page rather than redirecting.

Thanks @mseaton and @ibacher for the observations. I mostly tried to follow what already exists on the patient management repo for the side nav links and it uses the configuration link eg : https://github.com/openmrs/openmrs-esm-patient-management/blob/41ab54a95887bb914c85ebd88e45a1d5a60213de/packages/esm-appointments-app/src/createDashboardLink.component.tsx#L25 , perhaps later we can work on improving that

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.

4 participants