-
Notifications
You must be signed in to change notification settings - Fork 2.3k
added feature: allowClear in input field
#4895
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
added feature: allowClear in input field
#4895
Conversation
|
@heath-freenome , Hi, just a gentle follow-up to check if you’ve had a chance to look at this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Suyog241005 Sorry for the delay just got back from vacation on Monday and I've been busy.
First of all, thank you for being willing to add new features to RJSF, we really appreciate it.
I've given you some general feedback on the approach you took and a few high-level requests for changes.
@heath-freenome I’ve gone through your comments and I’m actively reworking the implementation to better align with RJSF’s architecture (templates, localization via translateString, and theme-specific button handling). |
004c188 to
88adf72
Compare
|
@heath-freenome I’ve pushed updates addressing the feedback:
Please let me know if anything needs further adjustment. |
heath-freenome
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Suyog241005 Almost there. Thanks for responding to my previous feedback. Sorry for the slow response, between the end-of-year rush at work plus the holidays I haven't had much time to look over these PRs
865c632 to
68da392
Compare
|
@heath-freenome I’ve updated the implementation based on the feedback:
Please let me know if this aligns with the expected direction or if further tweaks are needed. |
packages/core/src/components/templates/ButtonTemplates/IconButton.tsx
Outdated
Show resolved
Hide resolved
|
@heath-freenome Please let me know if there’s anything else I should adjust. |
packages/daisyui/src/templates/BaseInputTemplate/BaseInputTemplate.tsx
Outdated
Show resolved
Hide resolved
packages/fluentui-rc/src/BaseInputTemplate/BaseInputTemplate.tsx
Outdated
Show resolved
Hide resolved
packages/react-bootstrap/src/BaseInputTemplate/BaseInputTemplate.tsx
Outdated
Show resolved
Hide resolved
packages/semantic-ui/src/BaseInputTemplate/BaseInputTemplate.tsx
Outdated
Show resolved
Hide resolved
packages/semantic-ui/src/BaseInputTemplate/BaseInputTemplate.tsx
Outdated
Show resolved
Hide resolved
…aced the value of ClearButton to �[3J�[H�[2J and adjusted the classname for ClearButton in theme
…t and updated CHANGELOG.md
7b271b6 to
ec13901
Compare
Last change
|
@Suyog241005 thanks for all your hard work on this |
| /** If submitButtonOptions is provided it should match the `UISchemaSubmitButtonOptions` type */ | ||
| submitButtonOptions?: UISchemaSubmitButtonOptions; | ||
| /** Flag, if set to `true`, will allow the text input fields to be cleared */ | ||
| allowClearTextInputs?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Suyog241005 I just realized that we need to update the uiSchema docs to add this new flag. Are you able to do that?
Reasons for making this change
This PR adds support for a new ui:allowClear flag.
-Updated all theme packages to support allowClear feature
Fixes #4671
Checklist
npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:updateto update snapshots, if needed.