Update: Grid layout: Allow users to adjust grid density #63367
Conversation
jorgefilipecosta
added
[Type] Enhancement
|
Size Change: +704 B (+0.04%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
Thanks @jorgefilipecosta, this is very cool :) Could we apply Would it be feasible to define upper and lower bounds based on breakpoint? For example you'd never want to use a 5 column arrangement on mobile, so it doesn't make sense to offer that option.
the + / - icons feel a bit counter-intuitive to the behavior. I think I'd expect the + to increase the size of the grid item, similar to this UI in MacOS Photos: 
With that said, I wonder if we should flip things around. Instead of 'Grid density' it could be 'Item size'. IE 'Decrease item size' and 'Increase item size'. As the range value increase, the number of columns decreases. Hope that makes sense 🤞 |
|
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. |
jorgefilipecosta
force-pushed
the
update/Data-views-grid-layout-Allow-users-to-adjust-grid-density-and-consumers-to-set-default-density
branch
from
602d40a to
1b3bbbc
Compare
|
Hi @jameskoster, I updated the code to follow the logic you suggested. Let me know if the behavior is what you had in mind. |
jorgefilipecosta
force-pushed
the
update/Data-views-grid-layout-Allow-users-to-adjust-grid-density-and-consumers-to-set-default-density
branch
from
27042f4 to
cbc9711
Compare
|
Thanks Jorge, this is getting close and working pretty well. Would it be possible to update the display on container resize? One small issue currently is that; if you set a large number of columns on a wide screen, then scale down, the large number of columns persists (and doesn't work very well) on the smaller screen. The control also behaves a bit strangely. Video demo: scaling.mp4Ideally the display and control update in line with the container size. Somewhat related: is it possible to set a default for each scale? E.g. above 1920 you can choose between 2 and 6 columns, but the default is 4? |
|
Another tiny point, the values that appear as tooltips on the range don't really communicate anything useful, we can probably hide them. 
|
jorgefilipecosta
force-pushed
the
update/Data-views-grid-layout-Allow-users-to-adjust-grid-density-and-consumers-to-set-default-density
branch
from
cbc9711 to
4f02361
Compare
|
Hi @jameskoster, thank you for the review and suggesting enhancements, the suggestions were applied. |
|
Flaky tests detected in be85633. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10008365009
|
|
Hey @jorgefilipecosta, I pushed a small update to the config. One issue is that the columns.mp4One other request if possible: if you set a non-default value, could that value persist as you resize the screen (assuming the value is compatible with the associated breakpoint)? For example if I set 3 columns at the |
|
I have an idea that could help provide visual indication of what’s happening with available sizes/layout for different viewport sizes. That is to borrow the styling from Unfortunately, the styles would have to be duplicated as they’re not inherent to the mark styling of |
jorgefilipecosta
force-pushed
the
update/Data-views-grid-layout-Allow-users-to-adjust-grid-density-and-consumers-to-set-default-density
branch
from
3c75796 to
02b78bc
Compare
Hi @jameskoster, your suggestions were implemented, let me know if this looks good. |
jorgefilipecosta
force-pushed
the
update/Data-views-grid-layout-Allow-users-to-adjust-grid-density-and-consumers-to-set-default-density
branch
from
02b78bc to
be85633
Compare
|
Hi @stokesman, I think depending on @WordPress/gutenberg-design thoughts on the suggestion we could explore adding marks to the possible breaks on the range similar to what the SpacingSizesControl does on a follow-up PR. |
|
That's an excellent suggestion @stokesman, thanks for pointing that out. I agree it's something that would improve the experience here. We should probably consider making that component public. @jorgefilipecosta I think this is working well now, thanks for all the adjustments ❤️ One question; is it possible for consumers to supply different configs? For instance the Pages data view might need to supply different density options to the Templates data view. Similarly it should probably be possible for a data view to opt out of this option altogether. |
jorgefilipecosta
force-pushed
the
update/Data-views-grid-layout-Allow-users-to-adjust-grid-density-and-consumers-to-set-default-density
branch
from
be85633 to
b5a9c22
Compare
Squashed commits: [bd24def8e8] Fix logic for defaults [1367c74410] Implement the logic to refer to item size instead of number of columns [b18403961a] Update: Data views grid layout: Allow users to adjust grid density, and consumers to set default density
|
I noticed |
Hi @jameskoster, Yes, I tried the marks, but it does not seems like what we want: We could of course hide labels and get this: Do you think using marks with labels hidden would be a good idea? |
Right now it is not consumer configurable the main goal of this PR is to get an initial version that does not changes API's for the consumer. But I have an hardcoded config object (for now) so changing to something consumer configurable will be easy. We just need to discuss the API for how the config object could be passed, and our hardcoded one could start to become the default in case nothing is passed. I guess we could work on the consumer configuration in a follow-up? |
jorgefilipecosta
deleted the
update/Data-views-grid-layout-Allow-users-to-adjust-grid-density-and-consumers-to-set-default-density
branch
|
Very nice! |
|
I will reiterate the feedback that I shared before. I think all view configuration should be in the ViewConfig dropdown. I'm concerned that we keep adding buttons and controls outside of that dropdown. |
|
In this case the range control is much more ergonomic to use than a menu. That said, perhaps there's scope to move the range inside the menu? I'd be interested to hear the a11y story around something like this: 
@jorgefilipecosta if it's not a lot of work, it might be worth trying this in a separate PR to gather feedback? |
Sorry @youknowriad, with the discussion that followed I lost track of this suggestion. I can give a try to the suggestion @jameskoster proposed. But I'm not sure if our components allow that, I will look. |
Using a range input or interactive form controls in general as a menu item may not be allowed. From the
And from There is even an active conversation happening at the ARIA standards level about introducing a role Having said that, I don't know if we could find a way to include a slider inside a |
|
For me that dropdown is not a menu, I'm not a simple select box and I'm not navigating anywhere, it's a config dropdown, I'm editing the "view" config. I can see things like text inputs, sliders, selects (the existing menus)... So I think we should treat it like that instead |
|
@youknowriad that's quite a different proposition in terms of design, are you imagining something more Settings (#63624) oriented? Rough mockup: 
|
|
If it helps, we had a similar conversation in #55100 (comment) and following comments. |
That mockup works but I don't see why closer design can't work in a dropdown. |
A popover might work, but without the flyouts to help organise things it could end up being quite large and feel a bit unwieldy. If we're committed to moving away from a menu lets open an issue to explore the options. |
|
I'm trying options at #63852. I guess we can discuss the possibilities there, and then I update the code to reflect the option we commit to. |
A However, there are other roles that could be useful here. A simple alternative could be a popover with a vertical A more complex solution could be a single-column |
priethor
added
the
[Feature] DataViews
Fixes: #60630
Part of: #63128
cc: @jameskoster
This PR adds a UI on the grid view to allow users to select a density.
The density depends on the viewport, and we kept that behavior if the viewport changes density can increase or decrease.
Screenshot
Testing Instructions
I verified I was able to change the density of the grid on patterns, templates and pages.