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

(refactor) Refactor modals to match naming conventions #1198

Merged
merged 1 commit into from
Jun 21, 2024
Merged

(refactor) Refactor modals to match naming conventions #1198

merged 1 commit into from
Jun 21, 2024

Conversation

denniskigen
Copy link
Member

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 refactors modal files to match the *.modal.tsx naming convention introduced in denniskigen/o3-docs#42 and other places.

Screenshots

Related Issue

Other

@github-actions GitHub Actions
Copy link
Contributor

Size Change: -17 kB (-0.49%)

Total Size: 3.48 MB

ℹ️ View Unchanged
Filename Size Change
packages/esm-active-visits-app/dist/130.js 178 kB 0 B
packages/esm-active-visits-app/dist/255.js 2.5 kB 0 B
packages/esm-active-visits-app/dist/271.js 783 B 0 B
packages/esm-active-visits-app/dist/316.js 42.9 kB 0 B
packages/esm-active-visits-app/dist/319.js 687 B 0 B
packages/esm-active-visits-app/dist/382.js 1.22 kB 0 B
packages/esm-active-visits-app/dist/443.js 6.98 kB 0 B
packages/esm-active-visits-app/dist/460.js 794 B 0 B
packages/esm-active-visits-app/dist/574.js 592 B 0 B
packages/esm-active-visits-app/dist/635.js 1.22 kB 0 B
packages/esm-active-visits-app/dist/644.js 783 B 0 B
packages/esm-active-visits-app/dist/729.js 3.08 kB 0 B
packages/esm-active-visits-app/dist/757.js 700 B 0 B
packages/esm-active-visits-app/dist/784.js 2.63 kB 0 B
packages/esm-active-visits-app/dist/788.js 590 B 0 B
packages/esm-active-visits-app/dist/807.js 935 B 0 B
packages/esm-active-visits-app/dist/833.js 742 B 0 B
packages/esm-active-visits-app/dist/835.js 14.2 kB 0 B
packages/esm-active-visits-app/dist/875.js 50.5 kB 0 B
packages/esm-active-visits-app/dist/879.js 3.02 kB 0 B
packages/esm-active-visits-app/dist/main.js 69.5 kB 0 B
packages/esm-active-visits-app/dist/openmrs-esm-active-visits-app.js 3.33 kB 0 B
packages/esm-appointments-app/dist/130.js 178 kB 0 B
packages/esm-appointments-app/dist/152.js 259 B 0 B
packages/esm-appointments-app/dist/224.js 42.1 kB +1 B (0%)
packages/esm-appointments-app/dist/255.js 2.51 kB 0 B
packages/esm-appointments-app/dist/265.js 1.79 kB 0 B
packages/esm-appointments-app/dist/271.js 1.99 kB 0 B
packages/esm-appointments-app/dist/303.js 259 B 0 B
packages/esm-appointments-app/dist/309.js 0 B -879 B (removed) 🏆
packages/esm-appointments-app/dist/319.js 1.88 kB 0 B
packages/esm-appointments-app/dist/4.js 0 B -1.78 kB (removed) 🏆
packages/esm-appointments-app/dist/445.js 249 kB 0 B
packages/esm-appointments-app/dist/460.js 2.09 kB 0 B
packages/esm-appointments-app/dist/501.js 7.02 kB 0 B
packages/esm-appointments-app/dist/574.js 1.93 kB 0 B
packages/esm-appointments-app/dist/591.js 16.8 kB 0 B
packages/esm-appointments-app/dist/644.js 1.99 kB 0 B
packages/esm-appointments-app/dist/729.js 3.08 kB 0 B
packages/esm-appointments-app/dist/757.js 1.75 kB 0 B
packages/esm-appointments-app/dist/784.js 2.62 kB 0 B
packages/esm-appointments-app/dist/788.js 1.69 kB 0 B
packages/esm-appointments-app/dist/807.js 2.31 kB 0 B
packages/esm-appointments-app/dist/833.js 1.98 kB 0 B
packages/esm-appointments-app/dist/857.js 14.2 kB 0 B
packages/esm-appointments-app/dist/903.js 878 B 0 B
packages/esm-appointments-app/dist/904.js 20.7 kB 0 B
packages/esm-appointments-app/dist/main.js 307 kB +1 B (0%)
packages/esm-appointments-app/dist/openmrs-esm-appointments-app.js 3.39 kB 0 B
packages/esm-patient-list-management-app/dist/130.js 178 kB 0 B
packages/esm-patient-list-management-app/dist/139.js 22.4 kB 0 B
packages/esm-patient-list-management-app/dist/255.js 2.51 kB 0 B
packages/esm-patient-list-management-app/dist/271.js 1.58 kB 0 B
packages/esm-patient-list-management-app/dist/319.js 1.51 kB 0 B
packages/esm-patient-list-management-app/dist/382.js 1.23 kB 0 B
packages/esm-patient-list-management-app/dist/443.js 6.98 kB 0 B
packages/esm-patient-list-management-app/dist/460.js 1.72 kB 0 B
packages/esm-patient-list-management-app/dist/548.js 4.79 kB 0 B
packages/esm-patient-list-management-app/dist/574.js 1.34 kB 0 B
packages/esm-patient-list-management-app/dist/591.js 16.9 kB 0 B
packages/esm-patient-list-management-app/dist/635.js 1.23 kB 0 B
packages/esm-patient-list-management-app/dist/644.js 1.58 kB 0 B
packages/esm-patient-list-management-app/dist/729.js 3.08 kB 0 B
packages/esm-patient-list-management-app/dist/757.js 1.5 kB 0 B
packages/esm-patient-list-management-app/dist/784.js 2.63 kB 0 B
packages/esm-patient-list-management-app/dist/788.js 1.34 kB 0 B
packages/esm-patient-list-management-app/dist/807.js 1.85 kB 0 B
packages/esm-patient-list-management-app/dist/833.js 1.59 kB 0 B
packages/esm-patient-list-management-app/dist/930.js 98.3 kB 0 B
packages/esm-patient-list-management-app/dist/main.js 125 kB 0 B
packages/esm-patient-list-management-app/dist/openmrs-esm-patient-list-management-app.js 3.31 kB 0 B
packages/esm-patient-registration-app/dist/130.js 178 kB 0 B
packages/esm-patient-registration-app/dist/152.js 264 B 0 B
packages/esm-patient-registration-app/dist/169.js 6.71 kB 0 B
packages/esm-patient-registration-app/dist/207.js 0 B -2.5 kB (removed) 🏆
packages/esm-patient-registration-app/dist/255.js 2.5 kB 0 B
packages/esm-patient-registration-app/dist/271.js 2.05 kB 0 B
packages/esm-patient-registration-app/dist/303.js 264 B 0 B
packages/esm-patient-registration-app/dist/319.js 1.99 kB 0 B
packages/esm-patient-registration-app/dist/34.js 61.3 kB 0 B
packages/esm-patient-registration-app/dist/371.js 547 B 0 B
packages/esm-patient-registration-app/dist/460.js 2.15 kB 0 B
packages/esm-patient-registration-app/dist/501.js 7.03 kB 0 B
packages/esm-patient-registration-app/dist/572.js 37.3 kB -1 B (0%)
packages/esm-patient-registration-app/dist/574.js 1.78 kB +68 B (+3.98%)
packages/esm-patient-registration-app/dist/591.js 16.8 kB 0 B
packages/esm-patient-registration-app/dist/644.js 2.05 kB 0 B
packages/esm-patient-registration-app/dist/729.js 3.08 kB 0 B
packages/esm-patient-registration-app/dist/735.js 465 B 0 B
packages/esm-patient-registration-app/dist/757.js 2.08 kB 0 B
packages/esm-patient-registration-app/dist/784.js 2.63 kB 0 B
packages/esm-patient-registration-app/dist/788.js 1.71 kB 0 B
packages/esm-patient-registration-app/dist/807.js 2.45 kB 0 B
packages/esm-patient-registration-app/dist/833.js 1.99 kB 0 B
packages/esm-patient-registration-app/dist/879.js 3.03 kB 0 B
packages/esm-patient-registration-app/dist/975.js 2.5 kB 0 B
packages/esm-patient-registration-app/dist/main.js 101 kB -1 B (0%)
packages/esm-patient-registration-app/dist/openmrs-esm-patient-registration-app.js 3.35 kB 0 B
packages/esm-patient-search-app/dist/130.js 178 kB 0 B
packages/esm-patient-search-app/dist/255.js 2.5 kB 0 B
packages/esm-patient-search-app/dist/271.js 920 B 0 B
packages/esm-patient-search-app/dist/299.js 23.2 kB 0 B
packages/esm-patient-search-app/dist/319.js 861 B 0 B
packages/esm-patient-search-app/dist/354.js 22 kB 0 B
packages/esm-patient-search-app/dist/382.js 1.23 kB 0 B
packages/esm-patient-search-app/dist/443.js 6.98 kB 0 B
packages/esm-patient-search-app/dist/460.js 939 B 0 B
packages/esm-patient-search-app/dist/574.js 742 B 0 B
packages/esm-patient-search-app/dist/591.js 16.8 kB 0 B
packages/esm-patient-search-app/dist/635.js 1.23 kB 0 B
packages/esm-patient-search-app/dist/644.js 920 B 0 B
packages/esm-patient-search-app/dist/729.js 3.08 kB 0 B
packages/esm-patient-search-app/dist/757.js 871 B 0 B
packages/esm-patient-search-app/dist/784.js 2.63 kB 0 B
packages/esm-patient-search-app/dist/788.js 736 B 0 B
packages/esm-patient-search-app/dist/807.js 1.04 kB 0 B
packages/esm-patient-search-app/dist/833.js 877 B 0 B
packages/esm-patient-search-app/dist/main.js 48.6 kB 0 B
packages/esm-patient-search-app/dist/openmrs-esm-patient-search-app.js 3.3 kB 0 B
packages/esm-service-queues-app/dist/130.js 178 kB 0 B
packages/esm-service-queues-app/dist/152.js 261 B 0 B
packages/esm-service-queues-app/dist/169.js 6.98 kB 0 B
packages/esm-service-queues-app/dist/185.js 49.4 kB -1 B (0%)
packages/esm-service-queues-app/dist/199.js 1.35 kB 0 B
packages/esm-service-queues-app/dist/233.js 0 B -1.75 kB (removed) 🏆
packages/esm-service-queues-app/dist/237.js 3.17 kB 0 B
packages/esm-service-queues-app/dist/255.js 2.51 kB 0 B
packages/esm-service-queues-app/dist/271.js 4.17 kB 0 B
packages/esm-service-queues-app/dist/276.js 1.85 kB 0 B
packages/esm-service-queues-app/dist/303.js 261 B 0 B
packages/esm-service-queues-app/dist/319.js 3.55 kB 0 B
packages/esm-service-queues-app/dist/401.js 3.06 kB 0 B
packages/esm-service-queues-app/dist/430.js 156 kB 0 B
packages/esm-service-queues-app/dist/460.js 4.42 kB 0 B
packages/esm-service-queues-app/dist/501.js 7.03 kB 0 B
packages/esm-service-queues-app/dist/574.js 3.81 kB 0 B
packages/esm-service-queues-app/dist/591.js 16.8 kB 0 B
packages/esm-service-queues-app/dist/6.js 1.75 kB 0 B
packages/esm-service-queues-app/dist/60.js 4.17 kB 0 B
packages/esm-service-queues-app/dist/644.js 4.17 kB 0 B
packages/esm-service-queues-app/dist/647.js 1.83 kB 0 B
packages/esm-service-queues-app/dist/650.js 3.29 kB 0 B
packages/esm-service-queues-app/dist/669.js 3.2 kB 0 B
packages/esm-service-queues-app/dist/696.js 0 B -1.39 kB (removed) 🏆
packages/esm-service-queues-app/dist/703.js 0 B -4.17 kB (removed) 🏆
packages/esm-service-queues-app/dist/729.js 3.08 kB 0 B
packages/esm-service-queues-app/dist/738.js 0 B -1.35 kB (removed) 🏆
packages/esm-service-queues-app/dist/752.js 1.62 kB 0 B
packages/esm-service-queues-app/dist/757.js 3.55 kB 0 B
packages/esm-service-queues-app/dist/764.js 2.61 kB 0 B
packages/esm-service-queues-app/dist/784.js 2.63 kB 0 B
packages/esm-service-queues-app/dist/788.js 3.54 kB 0 B
packages/esm-service-queues-app/dist/800.js 1.68 kB 0 B
packages/esm-service-queues-app/dist/806.js 0 B -1.62 kB (removed) 🏆
packages/esm-service-queues-app/dist/807.js 4.84 kB 0 B
packages/esm-service-queues-app/dist/828.js 1.39 kB 0 B
packages/esm-service-queues-app/dist/833.js 4.11 kB 0 B
packages/esm-service-queues-app/dist/877.js 2.98 kB 0 B
packages/esm-service-queues-app/dist/917.js 2.78 kB 0 B
packages/esm-service-queues-app/dist/940.js 21.4 kB 0 B
packages/esm-service-queues-app/dist/981.js 0 B -1.68 kB (removed) 🏆
packages/esm-service-queues-app/dist/main.js 208 kB +4 B (0%)
packages/esm-service-queues-app/dist/openmrs-esm-service-queues-app.js 3.31 kB 0 B
packages/esm-ward-app/dist/130.js 178 kB 0 B
packages/esm-ward-app/dist/443.js 6.97 kB 0 B
packages/esm-ward-app/dist/49.js 5.75 kB 0 B
packages/esm-ward-app/dist/574.js 302 B 0 B
packages/esm-ward-app/dist/591.js 16.8 kB 0 B
packages/esm-ward-app/dist/697.js 14.9 kB 0 B
packages/esm-ward-app/dist/729.js 3.07 kB 0 B
packages/esm-ward-app/dist/784.js 2.62 kB 0 B
packages/esm-ward-app/dist/main.js 22.4 kB 0 B
packages/esm-ward-app/dist/openmrs-esm-ward-app.js 3.22 kB 0 B

compressed-size-action

ibacher
ibacher approved these changes Jun 20, 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.

Code looks good, but I think we should standardize tests having a similar convention to the components they are tests for.

packages/esm-appointments-app/src/appointments/common-components/end-appointment.test.tsx
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be named end-appointment.modal.test.tsx?

@denniskigen
Copy link
Member Author

We don't do component.test.tsx for test files, right? It's typically just the filename (sans the extension) ending with .test.tsx. Same for scss files (sorry about the lack of formatting - typed on my phone).

@ibacher
Copy link
Member

ibacher commented Jun 20, 2024

@denniskigen Yeah, that's normally how we'd do it. I'm a little concerned about what happens when you have a component and, e.g., a modal that embeds that component. I.e., I'd rather let component.tsx -> .test.tsx be the norm and everything else be namespaced, but maybe that's an unlikely scenario?

@denniskigen
Copy link
Member Author

Yeah, I'd imagine that's unlikely. I'll merge this and review the situation post-merge once all the commits are in.

@denniskigen denniskigen merged commit 1a8a555 into main Jun 21, 2024
6 checks passed
@denniskigen denniskigen deleted the refactor/rename-modals-to-match-conventions branch June 21, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ibacher ibacher ibacher approved these changes

@brandones brandones Awaiting requested review from brandones

@chibongho chibongho Awaiting requested review from chibongho

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

Successfully merging this pull request may close these issues.
2 participants
@denniskigen @ibacher