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

refactor(components): move Grid to leverage compositional patterns #2074

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

ericchernuka
Copy link
Contributor

Makes Grid allow children and leverage Grid.Cell to handle the CSS.

Motivations

Currently, Grid does not allow application developers to abstract away its children into re-usable components. This was due to mapping over the Children directly and proxying their props to an InternalGridCell component that expected the props of the immediate children to be Grid.Cell properties.

The original intent was to guide developers in leveraging Grid.Cell, as the first child causes issues for any developers who wish to create intermediary components for re-use. The solution was to just remove the intermediary guards in favour of rendering the children and allowing CSS to handle the logic of properly handling layout concerns.

Changes

Refactors Grid to allow any children and expose Grid.Cell directly.

Testing

  • Leveraging Storybook, no API changes to the output will be seen because Grid.Cell is now mapped directly to what was previously InternalGridCell.

Changes can be
tested via Pre-release


In Atlantis we use Github's built in pull request reviews.

Random photo of Atlantis

Makes Grid allow children and leverage Grid.Cell to handle the CSS.

Signed-off-by: Eric Chernuka <[email protected]>
Copy link

Could not publish Pre-release for 5c9f7a0. View Logs
The problem is likely in the NPM Publish or NPM CI step in the Trigger Pre-release Build Job.
.
For more troubleshooting steps, see the Pre-release Troubleshooting Guide

Copy link
Contributor

@MichaelParadis MichaelParadis left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for this, it will make other people's lives easier.

@MichaelParadis MichaelParadis merged commit c252714 into master Oct 16, 2024
12 of 13 checks passed
@MichaelParadis MichaelParadis deleted the ec/grid-composition branch October 16, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants