-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Update: Data views grid layout: Make aspect ratio consumer configurable. #63487
Update: Data views grid layout: Make aspect ratio consumer configurable. #63487
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. |
/** | ||
* True if the grid has a 1:1 aspect ratio the default, and false if it doesn't. | ||
*/ | ||
hasOneOneAspectRatio?: boolean; |
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.
Should we instead have the possibility to actually pick an aspect ratio (like an enum instead of a boolean)?
@@ -134,6 +135,12 @@ export default function DataViews< Item >( { | |||
setOpenedFilter={ setOpenedFilter } | |||
/> | |||
</HStack> | |||
{ view.type === LAYOUT_GRID && ( |
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.
Instead of putting this in its own menu, I think we should put it in the ViewActions instead and maybe offer a few different aspect ratio choices?
Not sure this is a great user experience. Without actual masonry layouts, which is an open question how well this can work for keyboard navigation, this is just going to create visual rivers in the text and cause things to not line up. It seems like if we want to allow changing the aspect ratio, it should change the aspect ratio of all previews. |
This feature provides parity with the original Patterns page, which feedback suggests was an enjoyable preview experience compared with the cropping we see on trunk. ![]()
That should be what happens. Aspect ratio is either:
The latter comes at the expense of perfectly aligned text, which seems a reasonable trade-off to me. To perfectly align the text it would need to appear before the preview, which looks a little odd. Perhaps I'm missing an alternative approach? It should work nicely with #63367, and I think we should probably concentrate on merging that one first so that we can better test this one. |
Personally I'd echo Joen. This feels a bit weird and maybe too prominent? I'm also not sure about the feature parity argument, since the new list is now quite different with way more info. Could this be something baked in the preview like a scrollbar or some interaction on hover (example zoom or something)? |
I agree, but I don't think scanning text is always the main concern in this layout. If a user wants to browse that way it would make more sense to switch to Table layout, or to toggle the aspect ratio to something strict like 1:1. As a primarily visual layout, Grid should be optimised for scanning the Previews, and in some cases the 1:1 cropping makes that experience less ergonomic. Another example is the media library, where you might want to see the orientation of a media item: |
I appreciate your desire to improve this view, but without actual masonry, even scanning just the previews becomes hard, when the whitespace causes those rivers. Even if this is an optional view configuration, it does not seem useful in this state. How do other similar apps or interfaces handle this? How about horizontal masonry, where wide images push the next item in the row to the right? |
Agreed. This is not ideal: Not sure if it should be configurable by the user, but if we wanted a very minimal, non-1:1 previews, it could resemble many photos apps design: increased gutter spacing, no visible text. It's actually not bad for patterns (and media library of course): As seen in MacOS Photos: |
That could be interesting to try (hiding the text, aligning each row along the x axis). I think user configuration makes sense, particularly for media where cropping is situationally useful. |
@jameskoster and @jorgefilipecosta can you confirm that this will be punted to 6.8? I'm assuming this is the case since it's not on the 6.7 project board, but this issue was listed in the 6.7 Roadmap, so I wanted to double-check. Thanks! |
Yes let's punt this one, it's not high-prio until we get to the media library data view. |
Closing this PR as it is outdated if we still want to have user-configurable aspect ratio we will need to open a new one taking into account the new view configuration UI. |
Fixes: #60891
Adds the possibility for the user to configure the aspect ratio on grid items between auto and 1/1 (the default).
cc: @jameskoster
Screenshots
Testing Instructions
Verified I was able to change the aspect ratio successfully.