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-3897: Improvements to the 'all encounters' tab on the visits page #2024

Merged
merged 3 commits into from
Sep 23, 2024

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 aims at making the encounter tabs in the visists section of the patient chat implement the right scroll behaviour as per this video

Screenshots

03-3897.mp4

Related Issue

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

@ciaranduffy
Copy link

@mccarthyaaron, thanks for the work on this.

There are a few elements not quite right, visually though.

Screenshot 2024-09-20 at 12 21 41
  1. The tabs should be in a container that's 40px in height, with a 2px bottom border that's #E0E0E0 or Border-subtle-00 in colour.

  2. The position of the tabs is not correct. They tabs should sit immediately below the vitals header in their container. The first visit card should be 16px from the bottom of the tabs container.

@mccarthyaaron
Copy link
Contributor Author

@ciaranduffy I have made some changes. Though there is still an issue with "The tabs should sit immediately below the vitals header". That is being prevented by a margin applied generically to all slots as shown here. Changing it would affect all other widgets(medications, orders, etc). We could do something like this , applying a different styles to the 'patient-chart-encounters-dashboard-slot'. Let me know what you think!

@ciaranduffy
Copy link

@mccarthyaaron, I'm not a developer so I don't really understand the code snippets you shared with me. Could you share a screenshot or a video of the end result on your side? Thanks

@mccarthyaaron
Copy link
Contributor Author

@ciaranduffy Here is a clip.

tabs.mp4

@ciaranduffy
Copy link

Thanks @mccarthyaaron, I think for now this is fine. If we need to add internal padding, when the visit summary cards themselves are iterated to match the design, hopefully that's a possibility?

Thanks

@mccarthyaaron
Copy link
Contributor Author

Thanks @mccarthyaaron, I think for now this is fine. If we need to add internal padding, when the visit summary cards themselves are iterated to match the design, hopefully that's a possibility?

Thanks

Yes it is @ciaranduffy. So should I go ahead and commit the changes from the screen recording

@ciaranduffy
Copy link

From the design side, I have no objections. Thanks

@@ -14,6 +14,11 @@
margin: 1.3125rem;
}

// See https://zpl.io/lrlmdq0 for the Visits dashboard design
[data-extension-slot-name='patient-chart-encounters-dashboard-slot'] {
Copy link
Member

Choose a reason for hiding this comment

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

#2024 (comment) addressed here.

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, @mccarthyaaron. The one thing that's outstanding here is blending the borders so that the green border of the active tab blends in with the 2px bottom border of the containing div. We can address that in a future iteration of implementing the designs.

@denniskigen denniskigen merged commit 535c41c into openmrs:main Sep 23, 2024
6 checks passed
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