WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 21 months ago

#33061 assigned defect (bug)

List table: colour comment bubble according to the scheme

Reported by: iseulde Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.3
Component: Comments Keywords: has-patch close reporter-feedback
Focuses: Cc:
PR Number:

Description

On hover the bubble is dark blue, which looks a bit weird in different colour schemes.

See #32152.

Attachments (4)

33061.patch (1.1 KB) - added by iseulde 4 years ago.
33061.2.patch (1.2 KB) - added by iseulde 4 years ago.
33061.3.patch (1.2 KB) - added by iseulde 4 years ago.
33061.4.patch (1.8 KB) - added by afercia 4 years ago.

Download all attachments as: .zip

Change History (27)

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


4 years ago

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


4 years ago

#3 @samuelsidler
4 years ago

  • Milestone changed from Awaiting Review to 4.3

#4 @obenland
4 years ago

  • Owner set to samuelsidler
  • Status changed from new to assigned

#5 @iseulde
4 years ago

Maybe we could even consider colouring the grey too. Same colours as the view switch icons.

@iseulde
4 years ago

#6 @iseulde
4 years ago

  • Keywords has-patch added

@iseulde
4 years ago

#7 @iseulde
4 years ago

If we use the same variables for the default scheme though, it would be black and red on hover...

#8 @ocean90
4 years ago

#33115 was marked as a duplicate.

#9 @afercia
4 years ago

Sorry, forgot this ticket and created a duplicate :) Adding a screenshot for visual reference of what's happening right now (Sunrise color scheme):

  • when hovering the bubble with zero comments it becomes red
  • hovering/focusing the bubbles with links gives always the blue from the default color scheme

https://cldup.com/jrHQ466lDc.png

#10 @afercia
4 years ago

With patch applied. Everything is working :) Not sure about the actual result in "Light" where the bubbles maybe are too... light, and in "Sunrise" where both approved and pending are red.

https://cldup.com/e_UcqCKnnh.png

#11 follow-up: @iseulde
4 years ago

Thanks for making those screenshots. Yes, I don't think the colours are good.
If we used the same variables for the default scheme the default colour of the bubble would be black (same as the menu background).

#12 @iseulde
4 years ago

It's probably better to have a grey variant as the default bubble colour and then the "active" colour for hover.

@iseulde
4 years ago

#13 @iseulde
4 years ago

Above patch: keep grey bubble, $highlight-color on hover, $highlight-color for unapproved bubble.

#14 in reply to: ↑ 11 @afercia
4 years ago

Replying to iseulde:

Thanks for making those screenshots.

You're welcome :) Tested last patch and maybe just the Light color scheme would need the color text to be adjusted, maybe always white?

https://cldup.com/oJ7dg3Pxbw.png

@afercia
4 years ago

#15 @afercia
4 years ago

Override color: $text-color; with color: $menu-highlight-text; for the Light color scheme. Please double check, not so familiar with Sass here :)

#16 @markjaquith
4 years ago

The dark text on red background is really difficult for me to see.

#17 follow-up: @helen
4 years ago

I'm not really sure there's anything wrong with blue - links are blue, no matter the color scheme, and those are more directly surrounding and in context with the bubble than the menu chrome.

#18 in reply to: ↑ 17 @markjaquith
4 years ago

Replying to helen:

I'm not really sure there's anything wrong with blue - links are blue, no matter the color scheme

This is a good point.

I think we should punt this, as RC is imminent.

#19 @obenland
4 years ago

  • Milestone changed from 4.3 to Future Release

Let's look at it again in a future release.

#20 @iseulde
4 years ago

Then we still need to remove some remains.

#22 @afercia
2 years ago

  • Keywords close reporter-feedback added

@iseulde anything left here or this can be closed? :)

#23 @samuelsidler
21 months ago

  • Owner samuelsidler deleted
Note: See TracTickets for help on using tickets.