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

Update: Data views grid layout: Make aspect ratio consumer configurable. #63487

Conversation

jorgefilipecosta
Copy link
Member

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

Screenshot 2024-07-12 at 15 21 10 Screenshot 2024-07-12 at 15 20 49

Testing Instructions

Verified I was able to change the aspect ratio successfully.

@jorgefilipecosta jorgefilipecosta added the [Type] Enhancement A suggestion for improvement. label Jul 12, 2024
@jorgefilipecosta jorgefilipecosta requested review from youknowriad, ntsekouras and a team July 12, 2024 14:21
@jorgefilipecosta jorgefilipecosta self-assigned this Jul 12, 2024
Copy link

github-actions bot commented Jul 12, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: richtabor <richtabor@git.wordpress.org>
Co-authored-by: ndiego <ndiego@git.wordpress.org>

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;
Copy link
Contributor

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 && (
Copy link
Contributor

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?

@jasmussen
Copy link
Contributor

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.

@jameskoster
Copy link
Contributor

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.

pattern-site-editor-643

it should change the aspect ratio of all previews

That should be what happens. Aspect ratio is either:

  • 1:1: handy if you want a perfect grid with everything lined up.
  • auto: useful when you want the preview to more accurately reflect the pattern/template.

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.

@ntsekouras
Copy link
Contributor

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)?

@jameskoster
Copy link
Contributor

jameskoster commented Jul 15, 2024

Agree on the prominence. It could be an option we include in the view options menu instead:

aspect

I still think this is a situationally handy feature. The constrained square preview isn't a great way to browse templates, which are often quite 'tall'.

@jasmussen
Copy link
Contributor

The main problem is these horizontal rivers:

Screenshot 2024-07-16 at 09 35 13

When the text doesn't line up, it's hard to scan.

@jameskoster
Copy link
Contributor

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:

aspect

@jasmussen
Copy link
Contributor

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?

@richtabor
Copy link
Member

richtabor commented Jul 31, 2024

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.

Agreed. This is not ideal:

CleanShot 2024-07-31 at 10 01 57


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):

CleanShot 2024-07-31 at 10 04 47

As seen in MacOS Photos:

CleanShot 2024-07-31 at 10 03 23

@jameskoster
Copy link
Contributor

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
Copy link
Contributor

Now that view options live in a popover, here's an alternative design we might try; 'Preview display' with options 'Cropped' (square) and 'Uncropped':

Screenshot 2024-08-13 at 18 27 13

When uncropped all fields would be locked as hidden except for the title which could appear on hover, just like the checkbox.

patterns

@ndiego
Copy link
Member

ndiego commented Sep 26, 2024

@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!

@jameskoster
Copy link
Contributor

jameskoster commented Sep 27, 2024

Yes let's punt this one, it's not high-prio until we get to the media library data view.

@jorgefilipecosta
Copy link
Member Author

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.

@youknowriad youknowriad deleted the update/Data-views-grid-layout-Make-aspect-ratio-consumer-configurable branch October 9, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data views grid layout: Make aspect ratio consumer configurable
7 participants