Skip to content

Fix 2280 by moving the bootstrap 3 classes out of SchemaField #4579

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

Merged
merged 4 commits into from
Apr 29, 2025

Conversation

heath-freenome
Copy link
Member

@heath-freenome heath-freenome commented Apr 25, 2025

Reasons for making this change

Fixed #2280 by moving marker classes from SchemaField to WrapIfAdditionalTemplate

  • In @rjsf/utils, updated WrapIfAdditionalTemplateProps to pick up the hideError and rawErrors props from FieldTemplateProps
  • In @rjsf/core, updated SchemaField to move the form-group class to the WrapIfAdditionalTemplate
    • Also moved the has-error and has-danger error classes
    • Updated the grid snapshot due to the change
    • Prefixed all rjsf marker classes with rjsf-
  • Updated the snapshots in other themes due to the removed marker classes
  • Updated the CHANGELOG_v6.md and v6x upgrade guide.md to reflect these changes

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@nickgros
Copy link
Contributor

@heath-freenome field is not actually a Bootstrap 3 class: https://bootstrapclasses.com/bootstrap-3-css-classes-index/

Should we keep the field class, and only remove form-group?

Fixed #2280 by moving marker classes from `SchemaField` to `WrapIfAdditionalTemplate`
- In `@rjsf/utils`, updated `WrapIfAdditionalTemplateProps` to pick up the `hideError` and `rawErrors` props from `FieldTemplateProps`
- In `@rjsf/core`, updated `SchemaField` to move the `form-group` and `field` marker classes to the `WrapIfAdditionalTemplate`
  - Also moved the `field-error`, `has-error` and `has-danger` error marker classes
  - Updated the grid snapshot due to the change
- Updated the snapshots in other themes due to the removed marker classes
- Updated the `CHANGELOG_v6.md` and `v6x upgrade guide.md` to reflect these changes
@heath-freenome
Copy link
Member Author

@heath-freenome field is not actually a Bootstrap 3 class: https://bootstrapclasses.com/bootstrap-3-css-classes-index/

Should we keep the field class, and only remove form-group?

@nickgros Hmmm, probably... I wonder if I should also keep the field-error marker class in SchemaField too?

@heath-freenome heath-freenome changed the title Fix 2280 by moving the bootstrap 3 marker classes out of SchemaField Fix 2280 by moving the bootstrap 3 classes out of SchemaField Apr 29, 2025
icon='copy'
/>
);
return <IconButton title={translateString(TranslatableString.CopyButton)} {...props} icon='copy' />;
Copy link
Member Author

Choose a reason for hiding this comment

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

These are added in the ArrayFieldItemButtonsTemplate

@@ -68,7 +76,7 @@ export default function WrapIfAdditionalTemplate<
<div className='col-xs-2'>
<RemoveButton
id={buttonId<T>(id, 'remove')}
className='array-item-remove btn-block'
className='rjsf-object-property-remove btn-block'
Copy link
Member Author

Choose a reason for hiding this comment

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

This one was wrong, it's for an object not array

@@ -79,7 +79,7 @@ export default function RatingWidget<
};

return (
<div className='field field-array'>
<>
Copy link
Member Author

Choose a reason for hiding this comment

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

We didn't need these

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the fragment can be removed

Comment on lines +23 to +24
const validateFns = superSchemaFns as unknown as ValidatorFunctions;
const validateOptionsFns = superSchemaOptionsFns as unknown as ValidatorFunctions;
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed a build error in chakra-ui with this

Copy link
Contributor

@nickgros nickgros left a comment

Choose a reason for hiding this comment

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

Looks great, nice migration guide

@heath-freenome heath-freenome merged commit d45af8b into rjsf-v6 Apr 29, 2025
4 checks passed
@heath-freenome heath-freenome deleted the fix-2280 branch April 29, 2025 21:39
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