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): Enhancements to the OpenmrsDatePicker #1062

Merged
merged 15 commits into from
Jul 5, 2024

Conversation

NethmiRodrigo
Copy link
Collaborator

@NethmiRodrigo NethmiRodrigo commented Jul 1, 2024

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. Ensure your PR title includes a conventional commit label (such as feat, fix, or chore, among others). See existing PR titles for inspiration.

For changes to apps

If applicable

  • My work includes tests or is validated by existing tests.
  • I have updated the esm-framework mock to reflect any API changes I have made.

Summary

Screenshots

This PR adds styling enhancements to the Date Picker.

  1. Fixes an issue where the label and the asterisk of required inputs were getting broken into two lines by wrapping it up in the <Label> component with the className cds-label.
    After fix:
Screenshot 2024-07-02 at 12 46 58 AM
  1. Adds styling to selected date in the calendar.
Screenshot 2024-07-03 at 1 33 52 AM
  1. Adds support for the prop invalidText. Renders the text passed in to the prop invalidText if the condition isInvalid is true, with the carbon styling for an error message applied.
Screenshot 2024-07-02 at 9 07 33 PM
  1. Adds styling to the current date so that it is highlighted. The styling is the same as which was in the Carbon date picker.
Screenshot 2024-07-03 at 1 33 28 AM
    • Add the alias invalid to isInvalid.

Related Issue

Other

@NethmiRodrigo NethmiRodrigo marked this pull request as draft July 1, 2024 19:29
@NethmiRodrigo
Copy link
Collaborator Author

NethmiRodrigo commented Jul 1, 2024

On hindsight, instead of making separate styles for the asterisk only, there's probably a carbon className I could use.

@NethmiRodrigo NethmiRodrigo changed the title (feat): Automatically add required styling to the label (feat): Add required styling to the label Jul 1, 2024
Copy link
Contributor

github-actions bot commented Jul 1, 2024

Size Change: -364 kB (-8.97%) ✅

Total Size: 3.7 MB

Filename Size Change
packages/shell/esm-app-shell/dist/openmrs.2afe6a1641375ff0.js 0 B -365 kB (removed) 🏆
ℹ️ View Unchanged
Filename Size Change
packages/apps/esm-devtools-app/dist/331.js 27.3 kB 0 B
packages/apps/esm-devtools-app/dist/667.js 6.97 kB 0 B
packages/apps/esm-devtools-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-devtools-app/dist/762.js 2.69 kB 0 B
packages/apps/esm-devtools-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-devtools-app/dist/875.js 9.93 kB 0 B
packages/apps/esm-devtools-app/dist/884.js 15.2 kB 0 B
packages/apps/esm-devtools-app/dist/889.js 166 kB 0 B
packages/apps/esm-devtools-app/dist/988.js 331 B 0 B
packages/apps/esm-devtools-app/dist/main.js 3.15 kB 0 B
packages/apps/esm-devtools-app/dist/openmrs-esm-devtools-app.js 3.19 kB 0 B
packages/apps/esm-help-menu-app/dist/21.js 5.71 kB 0 B
packages/apps/esm-help-menu-app/dist/248.js 7.06 kB 0 B
packages/apps/esm-help-menu-app/dist/474.js 705 B 0 B
packages/apps/esm-help-menu-app/dist/667.js 6.97 kB 0 B
packages/apps/esm-help-menu-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-help-menu-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-help-menu-app/dist/884.js 15.2 kB 0 B
packages/apps/esm-help-menu-app/dist/889.js 166 kB 0 B
packages/apps/esm-help-menu-app/dist/958.js 1.88 kB 0 B
packages/apps/esm-help-menu-app/dist/main.js 9.89 kB 0 B
packages/apps/esm-help-menu-app/dist/openmrs-esm-help-menu-app.js 3.13 kB 0 B
packages/apps/esm-implementer-tools-app/dist/271.js 723 B 0 B
packages/apps/esm-implementer-tools-app/dist/319.js 639 B 0 B
packages/apps/esm-implementer-tools-app/dist/426.js 24.9 kB 0 B
packages/apps/esm-implementer-tools-app/dist/460.js 748 B 0 B
packages/apps/esm-implementer-tools-app/dist/482.js 15.2 kB 0 B
packages/apps/esm-implementer-tools-app/dist/523.js 5.74 kB 0 B
packages/apps/esm-implementer-tools-app/dist/528.js 134 kB 0 B
packages/apps/esm-implementer-tools-app/dist/56.js 3.08 kB 0 B
packages/apps/esm-implementer-tools-app/dist/560.js 14.1 kB 0 B
packages/apps/esm-implementer-tools-app/dist/574.js 563 B 0 B
packages/apps/esm-implementer-tools-app/dist/587.js 2.93 kB 0 B
packages/apps/esm-implementer-tools-app/dist/620.js 126 kB 0 B
packages/apps/esm-implementer-tools-app/dist/625.js 564 B 0 B
packages/apps/esm-implementer-tools-app/dist/644.js 723 B 0 B
packages/apps/esm-implementer-tools-app/dist/657.js 7.03 kB 0 B
packages/apps/esm-implementer-tools-app/dist/71.js 6.98 kB 0 B
packages/apps/esm-implementer-tools-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-implementer-tools-app/dist/757.js 563 B 0 B
packages/apps/esm-implementer-tools-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-implementer-tools-app/dist/791.js 283 B 0 B
packages/apps/esm-implementer-tools-app/dist/803.js 61.4 kB 0 B
packages/apps/esm-implementer-tools-app/dist/807.js 562 B 0 B
packages/apps/esm-implementer-tools-app/dist/833.js 691 B 0 B
packages/apps/esm-implementer-tools-app/dist/889.js 166 kB 0 B
packages/apps/esm-implementer-tools-app/dist/main.js 78.4 kB 0 B
packages/apps/esm-implementer-tools-app/dist/openmrs-esm-implementer-tools-app.js 3.33 kB 0 B
packages/apps/esm-login-app/dist/111.js 1.22 kB 0 B
packages/apps/esm-login-app/dist/126.js 2.5 kB 0 B
packages/apps/esm-login-app/dist/173.js 1.22 kB 0 B
packages/apps/esm-login-app/dist/224.js 258 B 0 B
packages/apps/esm-login-app/dist/236.js 272 B 0 B
packages/apps/esm-login-app/dist/240.js 366 B 0 B
packages/apps/esm-login-app/dist/271.js 962 B 0 B
packages/apps/esm-login-app/dist/272.js 266 B 0 B
packages/apps/esm-login-app/dist/319.js 879 B 0 B
packages/apps/esm-login-app/dist/336.js 233 B 0 B
packages/apps/esm-login-app/dist/460.js 967 B 0 B
packages/apps/esm-login-app/dist/539.js 300 B 0 B
packages/apps/esm-login-app/dist/56.js 3.07 kB 0 B
packages/apps/esm-login-app/dist/574.js 762 B 0 B
packages/apps/esm-login-app/dist/604.js 30.6 kB 0 B
packages/apps/esm-login-app/dist/625.js 762 B 0 B
packages/apps/esm-login-app/dist/627.js 258 B 0 B
packages/apps/esm-login-app/dist/63.js 16.5 kB 0 B
packages/apps/esm-login-app/dist/637.js 2.1 kB 0 B
packages/apps/esm-login-app/dist/644.js 962 B 0 B
packages/apps/esm-login-app/dist/667.js 6.97 kB 0 B
packages/apps/esm-login-app/dist/673.js 286 B 0 B
packages/apps/esm-login-app/dist/735.js 2.62 kB 0 B
packages/apps/esm-login-app/dist/757.js 908 B 0 B
packages/apps/esm-login-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-login-app/dist/807.js 1.13 kB 0 B
packages/apps/esm-login-app/dist/833.js 902 B 0 B
packages/apps/esm-login-app/dist/883.js 32 kB 0 B
packages/apps/esm-login-app/dist/884.js 15.2 kB 0 B
packages/apps/esm-login-app/dist/889.js 166 kB 0 B
packages/apps/esm-login-app/dist/940.js 22.7 kB 0 B
packages/apps/esm-login-app/dist/main.js 66.6 kB 0 B
packages/apps/esm-login-app/dist/openmrs-esm-login-app.js 3.38 kB 0 B
packages/apps/esm-offline-tools-app/dist/271.js 1.19 kB 0 B
packages/apps/esm-offline-tools-app/dist/319.js 1.13 kB 0 B
packages/apps/esm-offline-tools-app/dist/460.js 1.3 kB 0 B
packages/apps/esm-offline-tools-app/dist/538.js 91.3 kB 0 B
packages/apps/esm-offline-tools-app/dist/56.js 3.08 kB 0 B
packages/apps/esm-offline-tools-app/dist/574.js 1.03 kB 0 B
packages/apps/esm-offline-tools-app/dist/625.js 1.03 kB 0 B
packages/apps/esm-offline-tools-app/dist/63.js 16.5 kB 0 B
packages/apps/esm-offline-tools-app/dist/644.js 1.19 kB 0 B
packages/apps/esm-offline-tools-app/dist/667.js 6.97 kB 0 B
packages/apps/esm-offline-tools-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-offline-tools-app/dist/757.js 1.19 kB 0 B
packages/apps/esm-offline-tools-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-offline-tools-app/dist/807.js 1.1 kB 0 B
packages/apps/esm-offline-tools-app/dist/833.js 1.21 kB 0 B
packages/apps/esm-offline-tools-app/dist/884.js 15.2 kB 0 B
packages/apps/esm-offline-tools-app/dist/889.js 166 kB 0 B
packages/apps/esm-offline-tools-app/dist/975.js 57 kB 0 B
packages/apps/esm-offline-tools-app/dist/main.js 148 kB 0 B
packages/apps/esm-offline-tools-app/dist/openmrs-esm-offline-tools-app.js 3.31 kB 0 B
packages/apps/esm-primary-navigation-app/dist/271.js 270 B 0 B
packages/apps/esm-primary-navigation-app/dist/319.js 232 B 0 B
packages/apps/esm-primary-navigation-app/dist/460.js 266 B 0 B
packages/apps/esm-primary-navigation-app/dist/567.js 23 kB 0 B
packages/apps/esm-primary-navigation-app/dist/574.js 231 B 0 B
packages/apps/esm-primary-navigation-app/dist/577.js 7.63 kB 0 B
packages/apps/esm-primary-navigation-app/dist/625.js 231 B 0 B
packages/apps/esm-primary-navigation-app/dist/63.js 16.5 kB 0 B
packages/apps/esm-primary-navigation-app/dist/644.js 270 B 0 B
packages/apps/esm-primary-navigation-app/dist/667.js 6.98 kB 0 B
packages/apps/esm-primary-navigation-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-primary-navigation-app/dist/757.js 237 B 0 B
packages/apps/esm-primary-navigation-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-primary-navigation-app/dist/807.js 291 B 0 B
packages/apps/esm-primary-navigation-app/dist/833.js 258 B 0 B
packages/apps/esm-primary-navigation-app/dist/884.js 15.2 kB 0 B
packages/apps/esm-primary-navigation-app/dist/889.js 166 kB 0 B
packages/apps/esm-primary-navigation-app/dist/958.js 22.8 kB 0 B
packages/apps/esm-primary-navigation-app/dist/main.js 47.3 kB 0 B
packages/apps/esm-primary-navigation-app/dist/openmrs-esm-primary-navigation-app.js 3.24 kB 0 B
packages/framework/esm-api/dist/openmrs-esm-api.js 16.2 kB 0 B
packages/framework/esm-config/dist/openmrs-esm-module-config.js 8.05 kB 0 B
packages/framework/esm-context/dist/openmrs-esm-context.js 1.09 kB 0 B
packages/framework/esm-dynamic-loading/dist/openmrs-esm-dynamic-loading.js 2.89 kB 0 B
packages/framework/esm-error-handling/dist/openmrs-esm-error-handling.js 891 B 0 B
packages/framework/esm-extensions/dist/openmrs-esm-extensions.js 8.16 kB 0 B
packages/framework/esm-feature-flags/dist/openmrs-esm-feature-flags.js 1.66 kB 0 B
packages/framework/esm-framework/dist/126.openmrs-esm-framework.js 2.48 kB 0 B
packages/framework/esm-framework/dist/278.openmrs-esm-framework.js 14.5 kB 0 B
packages/framework/esm-framework/dist/530.openmrs-esm-framework.js 2.93 kB 0 B
packages/framework/esm-framework/dist/619.openmrs-esm-framework.js 6.49 kB 0 B
packages/framework/esm-framework/dist/645.openmrs-esm-framework.js 9.3 kB 0 B
packages/framework/esm-framework/dist/735.openmrs-esm-framework.js 2.65 kB 0 B
packages/framework/esm-framework/dist/746.openmrs-esm-framework.js 6.14 kB 0 B
packages/framework/esm-framework/dist/788.openmrs-esm-framework.js 42.9 kB 0 B
packages/framework/esm-framework/dist/openmrs-esm-framework.js 376 kB +923 B (+0.25%)
packages/framework/esm-globals/dist/openmrs-esm-globals.js 791 B 0 B
packages/framework/esm-navigation/dist/openmrs-esm-navigation.js 9.34 kB 0 B
packages/framework/esm-offline/dist/openmrs-esm-offline.js 34.4 kB 0 B
packages/framework/esm-react-utils/dist/openmrs-esm-react-utils.js 15.8 kB 0 B
packages/framework/esm-routes/dist/openmrs-esm-utils.js 1.49 kB 0 B
packages/framework/esm-state/dist/openmrs-esm-state.js 910 B 0 B
packages/framework/esm-styleguide/dist/openmrs-esm-styleguide.js 54 kB 0 B
packages/framework/esm-translations/dist/openmrs-esm-core-translations.js 1.77 kB 0 B
packages/framework/esm-utils/dist/openmrs-esm-utils.js 18.3 kB 0 B
packages/shell/esm-app-shell/dist/06c1b95a262085a6.js 15.5 kB 0 B
packages/shell/esm-app-shell/dist/0f85df5904268497.js 915 B 0 B
packages/shell/esm-app-shell/dist/132c5e2b0b1864df.js 936 B 0 B
packages/shell/esm-app-shell/dist/1836f7c03d9970f0.js 1.19 kB 0 B
packages/shell/esm-app-shell/dist/2e155ac371cb8028.js 914 B 0 B
packages/shell/esm-app-shell/dist/2e1e924f130d5c4a.js 913 B 0 B
packages/shell/esm-app-shell/dist/37af5826facf2ea3.js 7.38 kB 0 B
packages/shell/esm-app-shell/dist/5cabd4dde7fa1017.js 914 B 0 B
packages/shell/esm-app-shell/dist/68de905d6d73abf9.js 1.19 kB 0 B
packages/shell/esm-app-shell/dist/6c47b0ee6bc3b834.js 1.59 kB 0 B
packages/shell/esm-app-shell/dist/aff9b27d855b4be9.js 960 B 0 B
packages/shell/esm-app-shell/dist/d41c5c08280221b2.js 1.18 kB 0 B
packages/shell/esm-app-shell/dist/openmrs.a1cab98407e908e5.js 365 kB 0 B
packages/shell/esm-app-shell/dist/service-worker.js 45.9 kB 0 B
packages/tooling/openmrs/dist/cli.js 2.9 kB 0 B
packages/tooling/openmrs/dist/commands/assemble.js 3.19 kB 0 B
packages/tooling/openmrs/dist/commands/build.js 1.32 kB 0 B
packages/tooling/openmrs/dist/commands/debug.js 543 B 0 B
packages/tooling/openmrs/dist/commands/develop.js 2.71 kB 0 B
packages/tooling/openmrs/dist/commands/index.js 437 B 0 B
packages/tooling/openmrs/dist/commands/start.js 850 B 0 B
packages/tooling/openmrs/dist/index.js 517 B 0 B
packages/tooling/openmrs/dist/runner.js 640 B 0 B
packages/tooling/openmrs/dist/utils/config.js 726 B 0 B
packages/tooling/openmrs/dist/utils/debugger.js 575 B 0 B
packages/tooling/openmrs/dist/utils/dependencies.js 643 B 0 B
packages/tooling/openmrs/dist/utils/helpers.js 397 B 0 B
packages/tooling/openmrs/dist/utils/importmap.js 3.07 kB 0 B
packages/tooling/openmrs/dist/utils/index.js 443 B 0 B
packages/tooling/openmrs/dist/utils/logger.js 368 B 0 B
packages/tooling/openmrs/dist/utils/npmConfig.js 831 B 0 B
packages/tooling/openmrs/dist/utils/untar.js 725 B 0 B
packages/tooling/openmrs/dist/utils/variables.js 192 B 0 B
packages/tooling/openmrs/dist/utils/webpack.js 278 B 0 B
packages/tooling/webpack-config/dist/index.js 3.58 kB 0 B

compressed-size-action

Comment on lines 297 to 298
color: #da1e28 !important;
margin-left: 0.25rem;
Copy link
Member

Choose a reason for hiding this comment

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

I think you could also get the color right by applying the .cds--form-requirement class.

Suggested change
color: #da1e28 !important;
margin-left: 0.25rem;
color: theme.$text-error !important;
margin-left: spacing.$spacing-02;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried cds--form-requirement as the className, but it didn't work. I tried looking in Carbon's playground but I couldn't find any required styling for the inputs, so I updated the styles, as suggested.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just saw that by default cds--form-requirement has a display: none that gets overriden by some selectors based on if the input is disabled and so on.

packages/framework/esm-styleguide/src/datepicker/index.tsx Outdated Show resolved Hide resolved
packages/framework/esm-styleguide/src/datepicker/index.tsx Outdated Show resolved Hide resolved
@NethmiRodrigo NethmiRodrigo changed the title (feat): Add required styling to the label Enhancements to the OpenmrsDatePicker Jul 2, 2024
@ibacher
Copy link
Member

ibacher commented Jul 2, 2024

Any reason this is still marked as draft?

@NethmiRodrigo
Copy link
Collaborator Author

Any reason this is still marked as draft?

@ibacher Yeah I want to fix the higlighting selected date styling and I'm currently working on adding support for invalidText - just fixing up the styling there, and I also want to tackle highlighting the current date (I saw it being done in React Aria's example so I don't think it should take long)

@NethmiRodrigo
Copy link
Collaborator Author

@ibacher Wherever we've used Carbon inputs in form engine, we have passed in a custom component for labelText to add the asterisk if the input is required. Should we follow the norm or is it okay that this PR would do it automatically? I have no obstruction to removing that part.

@ibacher
Copy link
Member

ibacher commented Jul 2, 2024

Should we follow the norm

Yes, we should keep things consistent, I think.

@NethmiRodrigo NethmiRodrigo changed the title Enhancements to the OpenmrsDatePicker (feat): Enhancements to the OpenmrsDatePicker Jul 2, 2024
@NethmiRodrigo NethmiRodrigo marked this pull request as ready for review July 5, 2024 13:48
@ibacher ibacher force-pushed the feat/enhance-openmrs-date-picker branch from da6521a to c296e16 Compare July 5, 2024 14:07
@ibacher
Copy link
Member

ibacher commented Jul 5, 2024

That is the first time I've seen Turbo cause a build failure...

@ibacher ibacher merged commit e450bab into main Jul 5, 2024
11 checks passed
@ibacher ibacher deleted the feat/enhance-openmrs-date-picker branch July 5, 2024 14:51
Twiineenock pushed a commit to Twiineenock/openmrs-esm-core that referenced this pull request Jul 18, 2024
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