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

fix: remove mui/material-ui ArrayFieldTemplate grid wrapper #3864

Merged
merged 5 commits into from
Oct 5, 2023
Merged

fix: remove mui/material-ui ArrayFieldTemplate grid wrapper #3864

merged 5 commits into from
Oct 5, 2023

Conversation

jroebu14
Copy link
Contributor

@jroebu14 jroebu14 commented Sep 11, 2023 •
edited
Loading

Reasons for making this change

fixes #3863

Removes a wrapping Grid component in ArrayFieldTemplate that wraps ArrayFieldItemTemplate. This is for the MUI and material-ui themes only.

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run 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

@jroebu14 jroebu14 force-pushed the fix-material-ui-array-field-template-item-double-grid-container branch from 7dac32e to 0a76e68 Compare September 12, 2023 07:10
heath-freenome
heath-freenome reviewed Sep 15, 2023
View reviewed changes
packages/material-ui/src/ArrayFieldItemTemplate/ArrayFieldItemTemplate.tsx Outdated
registry={registry}
/>
)}
<Grid item={true} xs={12}>
Copy link
Member

Choose a reason for hiding this comment

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

What about entirely removing the Grid container on line 63 of ArrayFieldTemplate instead?

Copy link
Contributor Author

@jroebu14 jroebu14 Sep 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes that makes more sense. There's just the question about what to do with the key prop on that Grid container. I could leave it there and change the Grid to a Box or just remove it entirely along with the key prop. I'm not sure the key prop is necessary so maybe the latter makes more sense.

Copy link
Member

Choose a reason for hiding this comment

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

Honestly the key prop is only required/used by React when it is being generated from some sort of map function, which this is not so you can remove it.

packages/mui/src/ArrayFieldItemTemplate/ArrayFieldItemTemplate.tsx Outdated
registry={registry}
/>
)}
<Grid item={true} xs={12}>
Copy link
Member

Choose a reason for hiding this comment

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

What about entirely removing the Grid container on line 63 of ArrayFieldTemplate instead?

@jroebu14 jroebu14 force-pushed the fix-material-ui-array-field-template-item-double-grid-container branch from 0a76e68 to 948d65a Compare September 18, 2023 10:30
@jroebu14 jroebu14 force-pushed the fix-material-ui-array-field-template-item-double-grid-container branch from 948d65a to db8be21 Compare September 21, 2023 07:30
@jroebu14 jroebu14 changed the title fix: change mui/material-ui ArrayFieldItemTemplate wrapper to grid item fix: remove mui/material-ui ArrayFieldTemplate grid wrapper Sep 21, 2023
@heath-freenome
Copy link
Member

@jroebu14 This looks great! Can you update the CHANGELOG.md to add sections for @rjsf/material-ui and @rjsf/mui underneath the # 5.13.1 section

heath-freenome
heath-freenome reviewed Oct 5, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@jroebu14
chore: update changelog with @rjsf/mui and @rjsf/material-ui fixes
a2465c5 
Signed-off-by: Jonathan Roebuck <jroebuck@spotify.com>
@jroebu14 jroebu14 force-pushed the fix-material-ui-array-field-template-item-double-grid-container branch from 1a31c32 to a2465c5 Compare October 5, 2023 21:03
@heath-freenome heath-freenome merged commit 6157bd9 into rjsf-team:main Oct 5, 2023
4 checks passed
@jroebu14 jroebu14 deleted the fix-material-ui-array-field-template-item-double-grid-container branch October 6, 2023 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heath-freenome heath-freenome heath-freenome approved these changes

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

Successfully merging this pull request may close these issues.

MUI/material-ui theme's ArrayFieldTemplate renders items in double grid container
2 participants
@jroebu14 @heath-freenome