Skip to content

Fix hardcoded colors in Data Views  #65215

Closed

Description

@stokesman

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.
image

The relevant section in the codebase:

&.is-selected {
.dataviews-view-list__item-actions {
background-color: rgb(247 248 255);
box-shadow: -12px 0 8px 0 rgb(247 248 255);
}
}

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.

Activity

  1. added
    [Type] BugAn existing feature does not function as intended
    [Feature] DataViewsWork surrounding upgrading and evolving views in the site editor and beyond
    on Sep 10, 2024
  2. colorful-tones commented on Sep 10, 2024

    @colorful-tones
    Member

    I feel like the background-color and box-shadow are unnecessary here. 🤔 I've opened #65218 to address this.

  3. ciampo commented on Sep 16, 2024

    @ciampo
    Contributor

    Maybe we could achieve the same result without using hardcoded colors:

    • use inherit for the background color
    • change the padding of the title (.dataviews-view-list__primary-field) when the actions are visible, so that text truncation kicks in correctly)
    • for the fade, if still necessary after tweaking the truncation, we could add a pseudo element to the actions div and try to use mask-image without the need of hardcoded colors
  4. stokesman commented on Sep 16, 2024

    @stokesman
    ContributorAuthor
    • use inherit for the background color

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

    • for the fade, if still necessary

    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.

  5. ciampo commented on Sep 19, 2024

    @ciampo
    Contributor

    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:

    Screenshot 2024-09-19 at 10 38 14

    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 background-color: inherit to the action items wrapper.


    Separately, the code in the list view could do with some polishing:

    • we should not use the internal .components-button classname
    • is there a reason why use JS mouseenter and mouseleave events instead of the :hover CSS pseudo-selector?
    • for composite items, instead of relying on :focus and :focus-within, we could use the [data-active-item] and/or [data-focus-visible] attributes, as recommended by ariakit guidelines
    • instead of :focus-within we could use :has(), for example: :has( *[data-focus-visible] ), :has(button:focus-visible), which has the advantage of working with focus visible (instead of focus)
  6. jameskoster commented on Sep 19, 2024

    @jameskoster
    Contributor

    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

    The problem is permanently visible icons add a lot of noise to the UI.

    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 background-color: inherit to the action items wrapper.

    This wouldn't fix the shadow/mask/truncation effect though, would it? 🤔

    I don't love it, but an alternative short-term option could be to go back to the solid background color. Then the shadow can utilise the theme color and work regardless:

    Screenshot 2024-09-19 at 16 36 53 Screenshot 2024-09-19 at 16 38 50

    We can't know for sure how many items would be in that row

    This is already the case since users can figure field visibility. Currently they wrap:

    Screenshot 2024-09-19 at 16 40 26
  7. ciampo commented on Sep 20, 2024

    @ciampo
    Contributor

    This wouldn't fix the shadow/mask/truncation effect though, would it? 🤔

    It kind of would in the way it does currently — just by applying the same background color as the item (via background-color: inherit), so make sure that the colors are always in sync, whatever theme is applied. So, not a real truncation, but more of a UI overlap, which hides the content under it.

    This is already the case since users can figure field visibility. Currently they wrap:

    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.

  8. jameskoster commented on Sep 20, 2024

    @jameskoster
    Contributor

    So, not a real truncation, but more of a UI overlap, which hides the content under it.

    I don't think this would look very good 😬

    Edit: I suppose something like this might work:

    toolbar.mp4

    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.

    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.

  9. stokesman commented on Sep 20, 2024

    @stokesman
    ContributorAuthor

    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.

    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.

  10. moved this from 🏗️ In Progress to ✅ Done in WordPress 6.7 Editor Taskson Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Needs Design FeedbackNeeds general design feedback.[Feature] DataViewsWork surrounding upgrading and evolving views in the site editor and beyond[Status] In ProgressTracking issues with work in progress[Type] BugAn existing feature does not function as intended

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions