DataViews: Make grid selected item styles more noticeable like table view - #80095
DataViews: Make grid selected item styles more noticeable like table view#80095prasadkarmalkar wants to merge 4 commits into
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. |
| &.is-selected { | ||
| background-color: var(--wpds-color-background-interactive-brand-weak-active); | ||
| border-color: var(--wpds-color-stroke-surface-brand); | ||
| padding: var(--wpds-dimension-padding-sm); |
There was a problem hiding this comment.
I don't believe we should add padding on selected items, as it creates a layout shift.
Is there an alternative way to do that, except for the approach of adding padding to every card (even if not selected)? I think adding default padding would mess with how things align (e.g search bar, etc..) and the spacing of the grid might not be ideal too (padding + gap).
|
I agree with Nik, we should not add padding. That's the main challenge of addressing this issue: it will need to have, in a clean way, a backdrop that extends outwards from its selection, and without overlapping with any other selected items. Thank you for contributing. |
|
Hi @ntsekouras @jasmussen , Thanks for the review. Without top margin
Latest changes
Let me know if this looks good or if any changes are required 🙇 |
|
Definitely no padding to prevent layout changes. I just checked the latest changes and it looks good to me.
Except for the compact density where selected elements have double border as being side by side.
However, it looks clearer to the current. Specially on small viewports.
I don't personally truly bother that selected elements are side by side. But I think we might have two options. AlternativesShrink the inset to be tighterFrom
Shift the whole gap ladder upMove the spacing values to render 40px (3xl), 32px (2xl), 24px (xl). Here is a test on the inspector.
I'm swinging between the two alternatives so curious about @jasmussen and @WordPress/gutenberg-components think about this. |
I think we could try to tweak the inset on smaller viewports, it seems like a small and clean change that looks better, and doesn't sacrifice the originally intended look at larger viewports |
| gap: var(--wpds-dimension-gap-xl); | ||
| container-type: inline-size; | ||
| margin-bottom: auto; | ||
| margin-top: 8px; |
There was a problem hiding this comment.
This rule will leak in other places too, eg. groupedBy grids, free composition etc.. Additionally we should use a css var probably instead of hardcoding.
The problem seems to be about the first row on the top side and not in the grid container itself. Can we achieve what we want with a more contained rule/technique?
Finally, and not related to this line of code, should we adjust the focus rings (keyboard navigation)?
There was a problem hiding this comment.
Agree, one question that I had was to see the new selected styles together with keyboard focus
There was a problem hiding this comment.
Can we add a margin to the first-row grid cards only when the .dataviews-selection-checkbox element (selection checkbox) is present?
--- a/packages/dataviews/src/components/dataviews-layouts/grid/style.scss
+++ b/packages/dataviews/src/components/dataviews-layouts/grid/style.scss
@@ -8,7 +8,6 @@
gap: var(--wpds-dimension-gap-xl);
container-type: inline-size;
margin-bottom: auto;
- margin-top: 8px;
@media not (prefers-reduced-motion) {
transition: padding ease-out 0.1s;
@@ -34,6 +33,10 @@
display: grid;
gap: var(--wpds-dimension-gap-xl);
+ &:first-of-type .dataviews-view-grid__card:has(.dataviews-selection-checkbox) {
+ margin-top: var(--wpds-dimension-padding-sm);
+ }
+
.dataviews-view-grid__row__gridcell {
border-radius: var(--wpds-border-radius-md);
position: relative;There was a problem hiding this comment.
We can test it out together with the rest of the requested changes? Although I'm afraid we will see a little jump when enabling the checkboxes, which won't be pleasant to the eye











What?
Closes #80043
Why?
Improves the visual styling of selected items in the DataViews grid layout to match the table layout.
How?
::afterbox-shadow overlay on.dataviews-view-grid__mediawith a background color (--wpds-color-background-interactive-brand-weak-active) and brand-colored border (--wpds-color-stroke-surface-brand) on the selected grid item itself.box-sizing: border-boxand a transparent border by default so the border added on selection doesn't shift layout..is-selectedto give the new border/background breathing room to be visible.Testing Instructions
Use of AI Tools
Yes
Claude Code - Sonnet 5
Used for - Base changes and code exploration