Make WordPress Core

Opened 2 years ago

Last modified 2 months ago

#48590 reviewing defect (bug)

Change the outline button hover background color

Reported by: sourav926 Owned by: audrasjb
Milestone: Future Release Priority: normal
Severity: normal Version: 5.3
Component: Administration Keywords: has-screenshots 2nd-opinion has-patch
Focuses: ui, accessibility, css Cc:

Description

Admin panel's new outline button hover background color is hardly understandable.

It can be background: #016087; color: #fff;

Attachments (12)

wp.png (20.2 KB) - added by sourav926 2 years ago.
Before and after of implementation
48590.diff (413 bytes) - added by melchoyce 2 years ago.
6de1fc0a6d970d6bad9dea049d8f5621.gif (401.0 KB) - added by audrasjb 2 years ago.
Before - hover color value = 016087
16f8e921f32cb391023f11183b05b0f2.gif (377.0 KB) - added by audrasjb 2 years ago.
Proposal - hover color value = 00405C
a2741da452f94ea32851b4e50be9e163.gif (93.0 KB) - added by audrasjb 2 years ago.
Current - hover color value = 016087
a9659eeb3afcb4e3caec0573004302e5.gif (85.0 KB) - added by audrasjb 2 years ago.
Proposed color - hover color value = 016087 - - it's very dark!
48590.2.diff (385 bytes) - added by sourav926 3 months ago.
Primary button hover border-color changed
48590.3.diff (396 bytes) - added by sourav926 3 months ago.
I made a mistake, the previous patch was created from WordPress production repository. So, now I've make a new patch from development repository.
Capture d’écran 2021-11-11 à 22.12.44.png (19.9 KB) - added by audrasjb 3 months ago.
Before 48590.3.diff
Capture d’écran 2021-11-11 à 22.12.59.png (24.2 KB) - added by audrasjb 3 months ago.
After 48590.3.diff
48590.blue70.diff (461 bytes) - added by sabernhardt 2 months ago.
option: Blue 70 on hover and focus, for background and border color
48590.comparison.gif (349.0 KB) - added by sabernhardt 2 months ago.
animated comparison of both patch options against the current styles, showing 3 different button-style links, going from default to hover and default to focus (Firefox/Windows)

Download all attachments as: .zip

Change History (50)

@sourav926
2 years ago

Before and after of implementation

This ticket was mentioned in Slack in #design by sourav926. View the logs.


2 years ago

#2 @SergeyBiryukov
2 years ago

  • Focuses ui accessibility added
  • Summary changed from Change the outline button hover backgroun color to Change the outline button hover background color

#3 follow-up: @mapk
2 years ago

  • Keywords needs-design-feedback removed

Thanks @sourav926! The design style you're proposing looks similar to the current primary button style. Take a look at the two styles below:

http://cldup.com/FvjHAmZzdW.png

I'd refrain from making all the buttons assume the primary styling. I know there is work to refine the secondary button styling to work better.

#4 in reply to: ↑ 3 @sourav926
2 years ago

Replying to mapk:

  1. Primary and Secondary both buttons should look different in Normal state.
  2. On the Hover state, a secondary button can look like the Primary button or a darker gray color.
  3. The main point is, "on hover" design of secondary button is not user-friendly.

#5 follow-up: @afercia
2 years ago

  • Keywords close added; needs-patch removed

@sourav926 thanks for your report. Worth noting the new secondary buttons style is a design choice that was discussed and implemented in #34904 see https://core.trac.wordpress.org/ticket/34904#comment:73 and following comments.

Actually, the color contrast or the new button style is greater than the previous one and that's one of the reasons for the change. One more reason is the new style is now consistent with the block editor buttons style (see screenshot below).

Same goes for the hover style: it's now consistent with the one used by the block editor buttons. I guess this is hardly going to change, as it's a deliberate design choice.

Regarding accessibility concerns, the hover state is indicated by the cursor change in the first place thus I wouldn't be greatly concerned.

Worth also considering on touch devices the hover state doesn't exist and it's generally less and less important on all devices.

For these reasons, I'd lean towards closing this ticket unless the design team feels some change is necessary.

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


2 years ago

#7 @afercia
2 years ago

  • Keywords needs-patch has-screenshots added; close removed
  • Milestone changed from Awaiting Review to 5.3.1

This ticket was discussed during today's accessibility bug-scrub. While some tend to think the hover style is way less important today than in the past, a quick improvement could be using the same background color change that was used on WordPress 5.2 together with a darker border. If a patch comes in in the very next days, this could be considered for 5.3.1.

#8 @melchoyce
2 years ago

I agree the contrast could get bumped up a bit on hover — it is pretty subtle now — but I'm struggling with finding an appropriate increase. I think I might want to wait for @kjellr to return from sabbatical before making any changes.

The color schemes have the same problem, though, and the greys are easy to darken. Attaching a patch to do so.

@melchoyce
2 years ago

#9 @audrasjb
2 years ago

  • Type changed from enhancement to defect (bug)

Thanks for the patch @melchoyce but I tested it and it doesn't seems to fix anything.
/wp-admin/css/colors SCSS files are not meant to handle default WP Admin colors but only alternate color schemes.

I think we rather need to edit /wp-includes/css/buttons.css file.

In this file, we don't have access to SCSS variables and functions, we need to use hexadecimal colors.
I tried to replace the current #016087 value with #00405C and it works fine (screenshots coming), but I'm not sure we really want to add another new shade of blue.
Are we ok to replace the current #016087 with a darker blue for the whole admin?
If so, I can make a new patch.

Cheers,
Jb

@audrasjb
2 years ago

Before - hover color value = 016087

@audrasjb
2 years ago

Proposal - hover color value = 00405C

@audrasjb
2 years ago

Current - hover color value = 016087

@audrasjb
2 years ago

Proposed color - hover color value = 016087 - - it's very dark!

#10 @audrasjb
2 years ago

  • Keywords 2nd-opinion needs-design-feedback early added
  • Milestone changed from 5.3.1 to 5.4
  • Owner set to audrasjb
  • Status changed from assigned to reviewing

In the last screenshot above, the darker color applied everywhere in WP Admin to replace #016087… it's a big change! A way darker!
Not sure we want to do that in a point release. I'd suggest to continue this discussion in the next major milestone.

Feel free to move it back to WP 5.3.1 if you think we could get a quick decision on this ticket :-)

Last edited 2 years ago by audrasjb (previous) (diff)

#11 @audrasjb
2 years ago

The color schemes have the same problem, though, and the greys are easy to darken. Attaching a patch to do so.

Ah sorry I missed that comment @melchoyce. You patch was for alternative color schemes! 🙉
In any way, I'd propose to handle both default and alternates in the next major.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


2 years ago

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


2 years ago

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


2 years ago

#15 @nrqsnchz
2 years ago

hey @audrasjb!

I feel that the darkening on the primary button is a bit too much, but I do think that the darkening of secondary buttons feels good.

#16 @karmatosed
2 years ago

  • Keywords needs-design-feedback removed

As this now has design feedback, removing the keyword.

This ticket was mentioned in Slack in #accessibility by audrasjb. View the logs.


2 years ago

#18 @audrasjb
2 years ago

  • Milestone changed from 5.4 to Future Release

Moving to Future release as we are getting close to 5.4 beta 1 :)

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


7 months ago

#20 follow-up: @joedolson
7 months ago

With the updated WordPress color palette, this is probably worth re-addressing.

I feel that we don't have to change the button background color in order to achieve the goal: we need the state change to be more apparent, and we could also accomplish that by using a different button outline color in the :hover state.

If the :hover color on primary buttons shifted down a couple notches (Perhaps #02163a from https://codepen.io/ryelle/full/WNGVEjw), we'd have a more obvious hover state without an aggressive design change.

#21 @joedolson
7 months ago

  • Milestone changed from Future Release to 5.9

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


4 months ago

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


4 months ago

#24 @audrasjb
4 months ago

  • Keywords early removed

As per today's bug scrub, I'm removing the early keyword as we don't really need this early in the release cycle.

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


4 months ago

#26 in reply to: ↑ 20 @sourav926
3 months ago

Hi @joedolson
Did you mean this?
(Actually, I want to add a patch, so asking for confirmation...)

.wp-core-ui .button-primary:hover {
    border-color: #02163a;
}
Last edited 3 months ago by sourav926 (previous) (diff)

#27 @joedolson
3 months ago

Yes, that's what I was referring to.

@sourav926
3 months ago

Primary button hover border-color changed

This ticket was mentioned in PR #1784 on WordPress/wordpress-develop by AbmSourav.


3 months ago

  • Keywords has-patch added; needs-patch removed

@sourav926
3 months ago

I made a mistake, the previous patch was created from WordPress production repository. So, now I've make a new patch from development repository.

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


3 months ago

@audrasjb
3 months ago

Before 48590.3.diff

#30 @audrasjb
3 months ago

Thanks for the patch!

I’m not 100% sure I'm convinced by the border-color change, maybe it would be better to change the outline color instead?
But I don’t have a strong opinion about this design change. I covers the underlined issue, so theoretically it looks good to me.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


3 months ago

#32 @sabernhardt
3 months ago

  • Keywords needs-design-feedback added

The border color change could improve the hover state, but I don't like as much when combined with the focus outline.

Darkening the primary background and border one more shade to Blue 70 in hover and focus states could be another option. I'll add a patch for that, plus screenshots to compare both against the current styles (considering both options).

Current styling:

  • Primary buttons go from a background and border color of Blue 50 to Blue 60 on both hover and focus.
  • Secondary buttons change text and border color from Blue 50 to Blue 70 on hover (but focus can have a different border color).

@sabernhardt
2 months ago

option: Blue 70 on hover and focus, for background and border color

@sabernhardt
2 months ago

animated comparison of both patch options against the current styles, showing 3 different button-style links, going from default to hover and default to focus (Firefox/Windows)

This ticket was mentioned in Slack in #design by chaion07. View the logs.


2 months ago

This ticket was mentioned in Slack in #design by chaion07. View the logs.


2 months ago

This ticket was mentioned in Slack in #design by shaunandrews. View the logs.


2 months ago

#36 @sabernhardt
2 months ago

  • Focuses css added
  • Milestone changed from 5.9 to Future Release

Since we do not have consensus on either option, we can revisit this discussion for a later release.

#37 in reply to: ↑ 5 @shaunandrews
2 months ago

  • Focuses css removed
  • Keywords needs-design-feedback removed

I agree with afercia's comment above. I don't think the changes in this ticket necessarily _improve_ anything, and that the current hover state works well. It also occurs to me that the colors being used doesn't seem to take into consideration a user's chosen color scheme.

I'd suggest we close this issue.

#38 @shaunandrews
2 months ago

  • Focuses css added
Note: See TracTickets for help on using tickets.