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) Implement the print functionality for Vitals lists #1196

Merged
merged 3 commits into from
Jul 25, 2023

Conversation

nanfuka
Copy link
Contributor

@nanfuka nanfuka commented Jun 7, 2023

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 implements the print feature. This feature will later be implemented in the esm core, however, for testing purposes, I have implemented it in the vitals package within the patient chart.

Screenshots

Print preview

Print preview

Landscape

Landscape

Portrait

Portrait

@nanfuka nanfuka marked this pull request as draft June 7, 2023 11:01
@nanfuka
Copy link
Contributor Author

nanfuka commented Jun 7, 2023

@hadijahkyampeire and @denniskigen kindly review.

Copy link
Contributor

@hadijahkyampeire hadijahkyampeire left a comment

Choose a reason for hiding this comment

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

Thanks, @nanfuka for the progress, just a few things plus good PR conventions. The title Print feature is too big, it should probably be (feat) Implement the print functionality for Vitals lists.
Remember this print functionality is going to appear in many places, so the PR should have a title that clearly state exactly what it is changing.

@hadijahkyampeire
Copy link
Contributor

Also about the PR details, instead of linking to the wiki GSoC overview, please create tickets for all the expected print tickets in the OpenMRS 3.x board and always attach them to their specific PRs. That helps the review to know the PR requirements. Let me know if you need some guidance on that.

@nanfuka nanfuka changed the title Print feature (feat) Implement the print functionality for Vitals lists Jun 9, 2023
@hadijahkyampeire hadijahkyampeire marked this pull request as ready for review June 21, 2023 10:45
@hadijahkyampeire
Copy link
Contributor

@nanfuka I don't see the header when I test the branch locally. Also please fix the failing tests.

@denniskigen
Copy link
Member

denniskigen commented Jun 21, 2023

We should come up with a good fallback for this situation too:

Screenshot 2023-06-21 at 5 38 16 PM

@nanfuka nanfuka force-pushed the print-feature branch 2 times, most recently from e3d2ed8 to 7bc95cc Compare July 4, 2023 19:18
Copy link
Contributor

@hadijahkyampeire hadijahkyampeire left a comment

Choose a reason for hiding this comment

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

Thank you so much @nanfuka for this great work, let's merge it and move forward.

@denniskigen
Copy link
Member

denniskigen commented Jul 13, 2023

Maybe best to get some final design feedback on today's squad call before merging. Also, please wait for an updated code review from me.

@denniskigen denniskigen force-pushed the print-feature branch 6 times, most recently from 4f18f07 to c196ec7 Compare July 24, 2023 22:41
@denniskigen denniskigen merged commit ffeb9e9 into openmrs:main Jul 25, 2023
5 checks passed
@denniskigen
Copy link
Member

Good start, @nanfuka. Keen to see how this work progresses.

@nanfuka
Copy link
Contributor Author

nanfuka commented Jul 28, 2023

thanks @denniskigen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants