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

chore: allow shape prop to use object notation with breakpoints as keys #2455

Closed
wants to merge 1 commit into from

Conversation

P1X3L
Copy link
Contributor

@P1X3L P1X3L commented Jun 7, 2024

To avoid Cumulative Layout Shift, we have spotted that we could leverage the usage of the object notation on Button's shape prop (like we do on style props)
(more info here 👉 https://www.debugbear.com/project/865/pageLoad/6293/overview?metric=cumulativeLayoutShift focus on the share button)

Until now, button's shape attribute had the type 'circle' | square

It can now be circle | square | default | Record<BREAKPOINT KEYS HERE, ('circle' | 'square' | 'default')>

We have an app using Button with the shape prop being dynamically set with JS to circle depending on breakpoints.
Unfortunately, these dynamic styles are not working on SSR so it is better to use css to do so.

@P1X3L P1X3L requested review from a team as code owners June 7, 2024 18:00
Copy link

github-actions bot commented Jun 7, 2024

👀 Visit Preview

@P1X3L P1X3L force-pushed the chore-upgrade-button-shape-prop branch 4 times, most recently from 4ebda81 to 818b4e6 Compare June 10, 2024 08:29
@P1X3L P1X3L force-pushed the chore-upgrade-button-shape-prop branch from 818b4e6 to 318b7f5 Compare June 10, 2024 08:42
@mleralec
Copy link
Contributor

From my point of view, this code adds a lot of complexity for a very small use case (especially since we maybe want to remove the "shape" prop in the future?) I like to keep WUI as simple as possible 🤷🏻‍♂️

@P1X3L
Copy link
Contributor Author

P1X3L commented Jun 10, 2024

Ok, I see your point, I'll make the changes where I found them.
I'm closing the issue then.

@P1X3L P1X3L closed this Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants