Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#31345 closed defect (bug) (fixed)

Admin menu icon colors fixes

Reported by: afercia's profile afercia Owned by: helen's profile helen
Milestone: 4.2 Priority: high
Severity: normal Version: 4.2
Component: Administration Keywords: has-patch
Focuses: ui Cc:

Description

After r31422 IE 8 users will feel a bit blue :) See screenshot:

https://cldup.com/oAVakHF21e.png

the icons color was: #999 and changed in this

#adminmenu div.wp-menu-image:before {
	color: #00b9eb;               <-- light blue
	color: rgba(240,245,250,0.6); <-- rgba light grey
}

and IE 8 doesn't support rgba so it will get the blue.

The proposed patch fixes also the icon color when tabbing (focus) the submenu items, this should pair the "focus" with the "hover" behavior. See before/after screenshot. Doesn't work in IE 8, just modern browsers.

https://cldup.com/EwZlqjPnVS.png

Attachments (5)

31345.patch (1.7 KB) - added by afercia 9 years ago.
31345.2.patch (2.5 KB) - added by afercia 9 years ago.
31345.mp4 (411.1 KB) - added by ericlewis 9 years ago.
31345.force-ie-repaint.patch (3.0 KB) - added by afercia 9 years ago.
31345.3.patch (2.8 KB) - added by afercia 9 years ago.

Download all attachments as: .zip

Change History (23)

@afercia
9 years ago

#1 @afercia
9 years ago

  • Keywords has-patch added

#2 @DrewAPicture
9 years ago

  • Keywords ui-feedback added
  • Milestone changed from Awaiting Review to 4.2

#3 @hugobaeta
9 years ago

Thank you for catching that @afercia! Awesome! :)

#4 @afercia
9 years ago

hi @hugobaeta you're welcome :) tried to use the new colors, feel free to update the patch if I've missed something.

#5 follow-up: @helen
9 years ago

  • Keywords needs-patch added; has-patch ui-feedback removed

Patch seems to need to consider the other color schemes, available in build.

#6 in reply to: ↑ 5 @afercia
9 years ago

Replying to helen:

Patch seems to need to consider the other color schemes, available in build.

@helen not sure, do you refer to "the icon color when tabbing (focus) the submenu items"? as far as I see, alternate color schemes do not change the icon color on hover/focus.

#7 @helen
9 years ago

@afercia - I think that was the issue, please test it - the blue shows momentarily for me in other color schemes, which is not desirable. I did not do any debugging of it, only testing.

@afercia
9 years ago

#8 @afercia
9 years ago

Refreshed patch updates the sass part for the alternate color schemes. @helen should be ok now, please let me know when you have a chance.

#9 @MikeHansenMe
9 years ago

  • Keywords has-patch added; needs-patch removed

#10 @samuelsidler
9 years ago

  • Priority changed from normal to high

Regression in the 4.2-cycle. We should get this fixed.

#11 @mattheweppelsheimer
9 years ago

tl;dr: patch needs more testing.

I tried testing this with crossbrowsertesting.com, but could not confirm that it works. The 2nd patch applies cleanly and I see the CSS rules I expected to in Chrome's web inspector tools, but my IE8 session continued to show just blue icons for the menu items.

I a layer of caching is to blame, somewhere between my local server and the remote VM running IE8. I am unfamiliar with the CSS build process… maybe there's a gotcha in there somewhere.

FWIW.

#12 @DrewAPicture
9 years ago

  • Owner set to helen
  • Status changed from new to reviewing

@ericlewis
9 years ago

#13 follow-up: @ericlewis
9 years ago

I can confirm attachment:31345.2.patch fixes the regression, see screencast in attachment:31345.mp4.

There seems to be some stickiness with the color, unclear why.

#14 in reply to: ↑ 13 @afercia
9 years ago

Replying to ericlewis:

There seems to be some stickiness with the color, unclear why.

hi @ericlewis
yes, IE 8 doesn't repaint pseudo elements on a parent class name change, so the icon color won't change. See for example:
http://stackoverflow.com/questions/8703799/forcing-ie8-to-rerender-repaint-before-after-pseudo-elements

If we want to fix this, we need a way to force a repaint. The simpler way would be to just add an extra space to the content CSS property but this would mean changing all the dashicons CSS and just for IE 8. There are also JavaScript hacks but I'm very, very, reluctant to suggest them and honestly I wouldn't recommend to do this. See attached demo patch.

It would be great if someone knew some simpler, less intrusive, way to force a repaint in IE 8 :)

This ticket was mentioned in Slack in #core by afercia. View the logs.


9 years ago

#16 @afercia
9 years ago

Agreed on Slack it's acceptable to have IE 8 ignoring the icon color change in some cases. See attached patch to fix the "stickiness". Color change will still work on hover. The only thing IE 8 will miss is the color change when navigating with the keyboard and focusing sub-items in the fly-out menus.

Last edited 9 years ago by afercia (previous) (diff)

@afercia
9 years ago

This ticket was mentioned in Slack in #core by drew. View the logs.


9 years ago

#18 @helen
9 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 32075:

Admin menu: fix colors for focus state and in IE8.

props afercia.
fixes #31345.

Note: See TracTickets for help on using tickets.