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

(fix) O3-4118: Service queues patient list should not be in a 'widget' / 'tile' #1390

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jwnasambu
Copy link
Contributor

@jwnasambu jwnasambu commented Nov 28, 2024

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

I renamed 'Patients currently in queue' to 'Active visits' for better clarity on the Service queues patient list.

Screenshots

Before making changes
Screenshot 2024-12-20 at 2 05 25 AM

After making changes
Screenshot 2024-12-02 at 4 02 52 PM

Related Issue

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

Other

@jwnasambu
Copy link
Contributor Author

@Chi Bong Ho @Fiona Anderson @antony Ojwang, @Ciarán Duffy Kindly feel free to review this PR at your convenient time, please!

@brandones brandones changed the title fix/03-4118: Service queues patient list should not be in a 'widget' / 'tile' (fix) O3-4118: Service queues patient list should not be in a 'widget' / 'tile' Dec 19, 2024
@brandones
Copy link
Contributor

@jwnasambu , it looks like you've tagged a bunch of random strangers. Sorry, random strangers.

There is quite a bit more to the ticket than what you've implemented here. Do you intend to finish implementing the feature as designed?

@jwnasambu
Copy link
Contributor Author

jwnasambu commented Dec 19, 2024

@jwnasambu , it looks like you've tagged a bunch of random strangers. Sorry, random strangers.

There is quite a bit more to the ticket than what you've implemented here. Do you intend to finish implementing the feature as designed?

Thanks @brandones! You are right I tagged random strangers. I should have tagged them on the Jira issues because I don’t know their Github accounts. Also, I’d like to continue with the implementation, but I am stuck on what to do next because there were some questions regarding what needs to be fixed. I was hoping to get clarification from these members." Apparently I am fixing the merge conflicts.

@ciaranduffy
Copy link

@jwnasambu @jwamalwa

Currently the active visit list (Patients currently in queue in the screenshot below) is placed on a tile
Screenshot 2024-12-19 at 13 53 27

This is incorrect.

It should be simply on the page, with a grey background as you can see in the mock-up below
Screenshot 2024-12-19 at 13 55 09

This mock-up is available on Zeplin here: https://zpl.io/aN3L75n

@jwnasambu jwnasambu marked this pull request as draft December 19, 2024 21:49
@jwnasambu jwnasambu marked this pull request as ready for review December 19, 2024 21:58
@jwnasambu jwnasambu marked this pull request as draft December 19, 2024 22:45
@jwnasambu jwnasambu marked this pull request as ready for review December 19, 2024 22:45
@jwnasambu
Copy link
Contributor Author

jwnasambu commented Dec 19, 2024

@brandones, @ciaranduffy Kindly, this is my finding, and I stand to be corrected. There are two classes with different background colors i.e. one is white and the other gray, which I believe is showing an unnecessary Layer or something and a border around the table, making it look like it’s a card instead of just part of the page. After changing the background colors the issue appears to be fixed. Kindly feel free to correct me, please! and point out what I am missing in my implementation.

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