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

Try making the Inserter category icons grayscale. #16163

Merged
merged 2 commits into from
Jun 14, 2019

Conversation

mapk
Copy link
Contributor

@mapk mapk commented Jun 13, 2019

Description

Fixes #14180. There's a concern that the full-color icons cause more cognitive load than they do help. While the primary desire is to remove them fully, I'm trying out the secondary option of just making them grayscale. This will minimize their brightness, but still allow them to help guide the user to that section of blocks.

How has this been tested?

Locally.

Screenshots

BEFORE

Screen Shot 2019-06-13 at 10 02 39 AM

AFTER

grayscale

Types of changes

Minor CSS filter added to the icon.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@mapk mapk added Needs Design Feedback Needs general design feedback. Needs Decision Needs a decision to be actionable or relevant Needs Accessibility Feedback Need input from accessibility labels Jun 13, 2019
@mapk mapk self-assigned this Jun 13, 2019
Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! A few considerations:

  • filter won't work on IE11. There's no super-easy override, but that's probably okay, since this isn't a critical change.
  • Also, this will result in icons that use a lot of slightly different grays. If we want to them all to be the same exact color, we could add specifically fill colors for the svg and path elements in here. But that may cause issues if an SVG has multi-colored elements or something. So the current solution is probably the safest.
  • We may optionally want to throw an !important in here if we want to be really strict about it.

I'm not sure that any of those are critical, so this gets a ✅ from me in the meantime.

@youknowriad
Copy link
Contributor

Maybe we can show the colors when we hover the panel header?

@afercia
Copy link
Contributor

afercia commented Jun 14, 2019

From an accessibility perspective, I don't see particular issues. As long as there's meaningful text to describe the sections, icons are purely decorative.

From other perspectives, I fully share the concerns expressed in the related issue, including plugin competition. As I see it, the inserted is not a place for branding or advertising. This should be discussed at a product level though, so it's just my 2 cents 🙂

@afercia afercia removed the Needs Accessibility Feedback Need input from accessibility label Jun 14, 2019
@mapk
Copy link
Contributor Author

mapk commented Jun 14, 2019

@kjellr I agree with your first two points and felt like a CSS filter was the safest bet. I also added an !important to the style.

@youknowriad I'd rather keep them grayscale, even on hover. I think reducing color in the Inserter throughout is the better path forward.

@afercia Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Block category icons are distracting
4 participants