-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix hardcoded colors in Data Views #65215
Comments
I feel like the background-color and box-shadow are unnecessary here. 🤔 I've opened #65218 to address this. |
Maybe we could achieve the same result without using hardcoded colors:
|
The catch is that the selected state background color is mostly transparent so won’t obscure what’s under it. It could work if an opaque version of it were available. Which is something that has been proposed as worthwhile before #63299 (comment).
The fade feels like a compromised decision and if the truncation is there shouldn’t be needed. I’ve made #65376. While it’s a lot of ++-- it’s mostly just moving things around. |
I looked more into this, and one aspect to consider if we want to cause "real" truncation by making the container narrower (add padding, reduce the width, ...), is what would happen to the "field" elements in the bottom row: We can't know for sure how many items would be in that row. And if we were to make the container narrower, there's a risk they would "wrap" to a new line, which would cause the whole row height to "jump". Personally, I think that the best long-term solution is to re-design the list view and give the action buttons a fixed place (ie. always visible), which would greatly simplify UX and code. (@WordPress/gutenberg-design ) If we need a short-term solution, we'd need to stick with overlapping the actions on top of the content (basically what we do at the moment). The only improvement that we could make (and that would fix this specific issue) is to apply whatever background color is required to the list wrapper, and then use Separately, the code in the list view could do with some polishing:
|
It kind of would in the way it does currently — just by applying the same background color as the item (via
Yup! My point is that, if we want "real" truncation by making the item content narrower, that will potentially push the items on more rows, causing the height of the list item to increase. Applying truncation to that part of the UI isn't easy since we're not dealing with simple text either. |
I don't think this would look very good 😬 Edit: I suppose something like this might work: toolbar.mp4
Oh, yes it would be good to avoid that. In fact that's one of the reasons we swapped to the current implementation; the actions 'column' in the row was causing the fields to wrap and leave a lot of empty space on the right-hand side of the row. Another short-term option would be to keep the hard-coded value, but generate it from the theme color variable using a function to lighten it accordingly (if that's possible). This would fix the color mis-match when using an admin theme other than Modern. |
This is true, but the context back then was that the actions were vertically centered. Now they are vertically aligned to the first row so they don’t have to push any other rows, just the first/title. As such, it appears that #65376 is ticking all the boxes. If I'm missing something I’d appreciate being made aware. |
The list view has some hardcoded colors that are most noticeable with admin theme that uses a warm color for buttons (e.g. Midnight or Sunrise). Note the item actions in this screenshot.
The relevant section in the codebase:
gutenberg/packages/dataviews/src/dataviews-layouts/list/style.scss
Lines 60 to 65 in 1cc30c5
Those are the ones noticeable in the screenshot. That file has some other hardcoded values (like
#f8f8f8
) that seem like they may need to be updated as well.The text was updated successfully, but these errors were encountered: