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-2801 Icons across O3 Navigation MVP #2121

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

(Feat) O3-2801 Icons across O3 Navigation MVP #2121

wants to merge 14 commits into from
+45 −3

Conversation

Munyua123
Copy link

@Munyua123 Munyua123 commented Nov 28, 2024 •
edited
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

Screenshots

Screenshot from 2024-11-28 14-05-54

Related Issue

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

Other

@Munyua123 Munyua123 changed the title O3-2801 Icons across O3 Navigation MVP (Feat) O3-2801 Icons across O3 Navigation MVP Nov 28, 2024
@Munyua123
Copy link
Author

@denniskigen @gracepotma @CynthiaKamau I have made the changes. I had a problem with the last pull request with the build but this one should work just fine. KIndly review

@gracepotma
Copy link
Contributor

Asante SANA @Munyua123!! LGTM <3

@denniskigen denniskigen requested a review from ibacher December 3, 2024 11:58
ibacher
ibacher requested changes Dec 3, 2024
View reviewed changes
Copy link
Member

@ibacher ibacher left a comment

Choose a reason for hiding this comment

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

Nice work! Some minor nits and a redesign suggestion.

packages/esm-patient-common-lib/src/dashboards/DashboardExtension.tsx Outdated Show resolved Hide resolved
packages/esm-patient-common-lib/src/dashboards/DashboardExtension.tsx Outdated Show resolved Hide resolved
packages/esm-patient-common-lib/src/dashboards/dashboard-extension.scss Outdated Show resolved Hide resolved
packages/esm-patient-common-lib/src/dashboards/dashboard-extension.scss Outdated Show resolved Hide resolved
@Munyua123 Munyua123 requested a review from ibacher January 6, 2025 07:41
@Munyua123
Copy link
Author

@ibacher Happy New Year. I'm sorry for the delay in solving the conflicts you raised, I have been able to solve some of the conflicts you raised above. Could you review and revert with any changes?

@Munyua123
Copy link
Author

@ibacher I have added the Icon as a prop to the dashboard link configuration and dashboard extension, where I have put up the Maybeicon as you had instructed and passed the icons to all the menu titles from the dashboard meta. The icons for the appointments and billing history since they are in their modules / esm once this is merged I can be able to add their icons from the modules. Could you review the above? Thank you

@Munyua123 Munyua123 requested a review from ibacher January 14, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ibacher ibacher Awaiting requested review from ibacher

Requested changes must be addressed to merge this pull request.

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

Successfully merging this pull request may close these issues.
3 participants
@Munyua123 @gracepotma @ibacher