-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Storybook: Add more max-width
containers
#68080
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: 0 B Total Size: 1.84 MB ℹ️ View Unchanged
|
Flaky tests detected in 6ba8e0f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12389717827
|
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.
This makes sense to me 👍
I'd just propose keeping the same field structure to avoid weird issues (see inline comment).
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
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.
LGTM and tests well 👍
Just one little suggestion before 🚢
Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com>
Thank you for working on this! I also wanted to share that Storybook has a native feature where the viewport can be resized: Kapture.2024-12-19.at.18.03.28.mp4Instead of writing and maintaining a custom add-on, should we tweak the configuration and add those same viewport sizes? @WordPress/gutenberg-components |
I think this is a really good idea. Not sure why we didn't go with that in the first place - it seems to be supported in older versions. @ntsekouras if you want to take on it, we'd be happy to assist! |
I remember some reasons being:
|
Makes sense. Well, it can be helpful to have them both then. After all, sometimes we may want to test in different container widths, and other times - in a different viewport width. |
What?
This PR adds some more
![Screenshot 2024-12-18 at 10 07 29 AM](https://private-user-images.githubusercontent.com/16275880/396818686-9ec1a4dc-d5c6-49df-984c-8592d1f1e111.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1NzI2MzgsIm5iZiI6MTczOTU3MjMzOCwicGF0aCI6Ii8xNjI3NTg4MC8zOTY4MTg2ODYtOWVjMWE0ZGMtZDVjNi00OWRmLTk4NGMtODU5MmQxZjFlMTExLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE0VDIyMzIxOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTE0YWQ2YzE5YjlhZGM2OGM3ZTA0YzA5Y2QxYWYxMDI1NWU2ZmY2Nzk5OThmMGRmNWEwYmE1OWMyOGFlNGE5ZmMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.ggaAdzWox3soNrEvUbrWSpiI-enb49r6QuP4qcnv5hM)
max-width
containers in Storybook as it's valuable to have them for many components. An example is DataViews and this code was extracted by: #68078.