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-3970 Re-enable readonly fields #397

Merged
merged 6 commits into from
Sep 30, 2024

Conversation

pirupius
Copy link
Member

@pirupius pirupius commented Sep 18, 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

This PR enables readonly mode in the field types that might be having it as well as adding the utility to cater for boolean string values

NOTE: For the date, we're using OpenmrsDatePicker and the readonly still shows the date picker so this might be either in newer versions of the framework or needs to be implemented in esm-core

Screenshots

Text
Screenshot 2024-09-26 at 13 35 25

Multi Select
Screenshot 2024-09-27 at 13 33 27

Number
Screenshot 2024-09-26 at 13 35 37

Radio
Screenshot 2024-09-26 at 13 35 51

Dropdown
Screenshot 2024-09-26 at 13 57 51

Textarea
Screenshot 2024-09-26 at 13 59 13

Toogle
Screenshot 2024-09-26 at 14 03 14

Ui-select extended
Screenshot 2024-09-26 at 14 05 03

Workspace
Screenshot 2024-09-26 at 14 08 00

Related Issue

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

Other

Copy link

github-actions bot commented Sep 18, 2024

Size Change: +53 B (0%)

Total Size: 1.15 MB

ℹ️ View Unchanged
Filename Size Change
dist/151.js 300 kB 0 B
dist/225.js 2.57 kB 0 B
dist/277.js 1.84 kB 0 B
dist/300.js 642 B 0 B
dist/335.js 968 B 0 B
dist/353.js 3.02 kB 0 B
dist/41.js 3.37 kB 0 B
dist/422.js 6.8 kB 0 B
dist/501.js 108 kB 0 B
dist/540.js 2.63 kB 0 B
dist/55.js 758 B 0 B
dist/572.js 252 kB +21 B (+0.01%)
dist/617.js 86.9 kB 0 B
dist/635.js 14.3 kB 0 B
dist/70.js 483 B 0 B
dist/901.js 11.8 kB 0 B
dist/99.js 691 B 0 B
dist/993.js 3.09 kB 0 B
dist/main.js 342 kB +32 B (+0.01%)
dist/openmrs-esm-form-engine-lib.js 3.67 kB 0 B

compressed-size-action

@pirupius pirupius requested a review from samuelmale September 18, 2024 12:41
@samuelmale
Copy link
Member

@pirupius Can you attach screenshots or a demo video or tests cases showing the readonly feat in action for the different renderings.

@pirupius pirupius force-pushed the fix/O3-3970/re-enable-readonly-fields branch from cfa1c3f to 05dcd6d Compare September 26, 2024 11:09
@pirupius
Copy link
Member Author

@samuelmale this is ready

src/components/inputs/date/date.component.tsx Outdated Show resolved Hide resolved
@@ -80,7 +80,7 @@ const MultiSelect: React.FC<FormFieldInputProps> = ({ field, value, errors, warn
return field.isRequired ? <FieldLabel field={field} /> : <span>{t(field.label)}</span>;
}, [field.isRequired, field.label, t]);

return sessionMode == 'view' || sessionMode == 'embedded-view' ? (
return sessionMode == 'view' || sessionMode == 'embedded-view' || isTrue(field.readonly) ? (
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

Copy link
Member Author

Choose a reason for hiding this comment

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

For this component we're using CheckboxGroup and it has a finicky behavior where regardless of if that property is passed, the user can still interact with the input field.

We might have to consider refactoring this component to use MultiSelect from Carbon later on that has support for this natively but for now this defaults it to the same display as the view only mode.

Copy link
Member Author

Choose a reason for hiding this comment

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

As per the call, am deprecating this change in favor of passing the attributes to the children for consistency.

@samuelmale samuelmale merged commit 29071e5 into main Sep 30, 2024
4 checks passed
@samuelmale samuelmale deleted the fix/O3-3970/re-enable-readonly-fields branch September 30, 2024 14:41
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.

2 participants