DataViews: preview size picker falls back to the smallest available size#71218
DataViews: preview size picker falls back to the smallest available size#71218
Conversation
When a grid dataview is passed a previewSize option which doesn't match one of the discrete sizes available, it falls back to the next smallest option. But if previewSize was smaller than the smallest available size it would throw an exception. The preview size picker control now uses the smallest available size if the previewSize option is too small.
|
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. |
p-jackson
added
[Type] Bug
|
Flaky tests detected in cf8e1d7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/16983121288
|
| .filter( ( size ) => size.value <= layoutPreviewSize ) | ||
| .filter( | ||
| ( size ) => size.index === 0 || size.value <= layoutPreviewSize | ||
| ) |
There was a problem hiding this comment.
How about checking whether the result is available before accessing it's value. Would it be clearer to handle the error 🤔
[ 0 ]?.index ?? breakValues[ 0 ].indexThere was a problem hiding this comment.
🤔 hmmm maybe this is clearer. I was so focused on making sure the array wasn't empty that I didn't just use ?.
Is breakValues[ 0 ].index just guaranteed to always be 0 though? I think it is.
I'll just use 0.
andrewserong
left a comment
andrewserong
left a comment
There was a problem hiding this comment.
For us it happened because we persist the user's selected size as a page query parameter. Since the page query parameter is editable, it is possible users edit this data and corrupt it.
Oh, fascinating, I hadn't even thought of that as a possibility. Nice catch with this PR!
This is looking good to me, too, with the default of having the 3rd size be the default, but the 1st option preserved for this case. Thanks for including a test case, too 👍
Before
After
LGTM 🚀
3 participants
What?
Fixes a crash caused by
<PreviewSizePicker>when an invalidpreviewSizeis passed to the data view.Why?
When a grid dataview is passed a
previewSizeoption which doesn't match one of the discrete sizes available, it falls back to the next smallest option. But ifpreviewSizewas smaller than the smallest available size it would throw an exception.previewSizecould be invalid for a number of reasons. For us it happened because we persist the user's selected size as a page query parameter. Since the page query parameter is editable, it is possible users edit this data and corrupt it. Since dataviews doesn't export the list of valid preview sizes we aren't able to reliably sanitize the input on our end.How?
The preview size picker control now shows the smallest available size as selected if the
previewSizeprop is too small.Testing Instructions
I've added a unit test to cover this situation, but you could manually test this in storybook.
First change this to
'grid'so that the default dataview in storybook is a grid.gutenberg/packages/dataviews/src/components/dataviews/stories/fixtures.tsx
Line 698 in 045f89b
Then add
previewSize: 100to the to the default layout options.When you use storybook the dataviews will show small icons, but when you open the view options it will no longer crash. When you change the preview size it will pop back to one of the allowed preview sizes.
CleanShot.2025-08-15.at.16.31.54.mp4
Here's the crash:
CleanShot.2025-08-15.at.16.33.18.mp4
Testing Instructions for Keyboard
No UI changes.