WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#29897 closed defect (bug) (duplicate)

Some focus styles are broken

Reported by: iseulde Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch
Focuses: ui, accessibility Cc:

Description

See screenshots below.

Attachments (16)

Screen Shot 2014-09-28 at 10.56.22.png (12.8 KB) - added by iseulde 6 years ago.
Screen Shot 2014-09-28 at 10.57.08.png (8.1 KB) - added by iseulde 6 years ago.
Screen Shot 2014-09-28 at 10.57.28.png (16.6 KB) - added by iseulde 6 years ago.
Screen Shot 2014-09-28 at 10.57.46.png (8.3 KB) - added by iseulde 6 years ago.
Screen Shot 2014-09-28 at 10.57.57.png (7.7 KB) - added by iseulde 6 years ago.
Screen Shot 2014-09-28 at 10.58.32.png (15.5 KB) - added by iseulde 6 years ago.
Screen Shot 2014-09-28 at 10.59.18.png (9.6 KB) - added by iseulde 6 years ago.
This is a disabled button…
Screen Shot 2014-09-28 at 10.59.54.png (10.1 KB) - added by iseulde 6 years ago.
Screen Shot 2014-09-28 at 10.59.41.png (9.6 KB) - added by iseulde 6 years ago.
Screen Shot 2014-09-28 at 11.03.01.png (19.2 KB) - added by iseulde 6 years ago.
Screen Shot 2014-09-28 at 11.03.22.png (7.2 KB) - added by iseulde 6 years ago.
29897.patch (1.2 KB) - added by iseulde 6 years ago.
29897.dashboard.list-table.patch (7.5 KB) - added by afercia 6 years ago.
29897.2.patch (7.5 KB) - added by afercia 6 years ago.
Refreshed simplified patch
29897.add-plugins.png (8.7 KB) - added by SergeyBiryukov 6 years ago.
29897.3.patch (7.7 KB) - added by afercia 5 years ago.
Fixes broken focus styles in the Dashboard, in list tables and in the Media Library/Add Themes/Add Plugins toolbars. Also fixes the media modal toolbars for IE 8.

Download all attachments as: .zip

Change History (39)

@iseulde
6 years ago

This is a disabled button...

@iseulde
6 years ago

#1 @iseulde
6 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.1

Above patch fixes the "Add New Theme" button.

#2 @afercia
6 years ago

Hi, proposed attached patch tries to fix dashboard and "list table" stuff:

  • removes overflow: hidden where possible so the focus box-shadow is no more "cut-off"
  • introduces a new "containing floats" CSS method, available in common.css which uses display: table, not supported by IE 7 and on IRC I've read something about IE 7 support going to be dropped soon
  • better positioning for the list/grid mode icons in the media library, please notice when using display: inline-block white space in the markup matters and list/grid modes differs about white space
  • outstanding issues on list/grid modes toolbars for small displays
  • needs testing :) especially in IE

#3 @afercia
6 years ago

I'm working on the pagination buttons:
https://core.trac.wordpress.org/raw-attachment/ticket/29897/Screen%20Shot%202014-09-28%20at%2010.59.18.png
to make the "disabled" buttons really disabled and refining my previous patch. I'd like to change them and use the standard buttons style, see screenshot below, before and after in normal and "focused" state:

https://cldup.com/n_zLqpG6ao.png

Any thoughts more than welcome.

#4 follow-up: @ocean90
6 years ago

  • Milestone changed from 4.1 to Future Release

Patches need some testing. Not sure I like the button style for the navigation.

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

Replying to ocean90:

Patches need some testing. Not sure I like the button style for the navigation.

I was thinking to split out the part about navigation buttons in a new ticket, and keep this ticket just for the broken focus styles. Thoughts?

@afercia
6 years ago

Refreshed simplified patch

#6 @afercia
6 years ago

Refreshed patch:

  • merges avryl's patch
  • takes care of focus styles in the dashboard and lists tables
  • strips out proposed changes to list table navigation, will open a new ticket for that
  • enlarges list table view-switch links clickable area
  • takes care of "dismiss" link, see also #28086

some examples:

https://cldup.com/-qek1a-Z_m.png

#7 @afercia
6 years ago

patch needs a refresh after r30813

#8 @SergeyBiryukov
6 years ago

Focus style for the tabs on Add Plugins screen could use some padding: 29897.add-plugins.png.

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

#9 @iseulde
5 years ago

  • Milestone changed from Future Release to 4.2

#10 @iseulde
5 years ago

#31203 was marked as a duplicate.

#11 @afercia
5 years ago

Will refresh the patch and check also focus style for add-new-theme. About the padding issue mentioned by @SergeyBiryukov, I'd propose to change the bottom black bar currently made with a border and use instead a pseudo element :after. Also, in responsive view, maybe reduce the vertical height:

https://cldup.com/l_LlGxrkqF.png

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

#12 @afercia
5 years ago

Related: #30725

#13 @afercia
5 years ago

Update:

  • agreed with @celloexpressions the Add New Theme link focus style will be handled in #31203
  • the only thing left here is the focus style for plugin install/theme install toolbars, see proposal below

UI feedback welcome. Some tickets should be probably fixed first, see #30725, #30766 and #31203

https://cldup.com/y9Y_uS04Yq.png

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


5 years ago

#15 @afercia
5 years ago

Refreshed patch takes care also of Add Themes/Add Plugins toolbars links. There are outstanding issues, for example the .tablenav links and also outstanding accessibility issues that need some improvements, will create new tickets for them.
For the main focus styles please refer to screenshot in comment 6. See also new screenshots below (please consider would be really hard to make screenshots for all the single focused elements...).
Includes fixes for IE 8, please notice given the curren selectors, touching the toolbars forces to fix also the media modal toolbar, see IE 8 screenshot.

Tested in latest Firefox, latest Chrome, IE 8.

Animated gif to compare the 4 toolbars: media library list/grid mode, add themes, add plugins:

https://cldup.com/NclD_BJRFA.gif

Chrome dev tools mobile emulator. Notice "Hello Dolly" breaks the header title :)

https://cldup.com/9tUKz3lwBo.png

IE 8 media modal:

https://cldup.com/M8chUaRq5v.png

@afercia
5 years ago

Fixes broken focus styles in the Dashboard, in list tables and in the Media Library/Add Themes/Add Plugins toolbars. Also fixes the media modal toolbars for IE 8.

#16 @DrewAPicture
5 years ago

  • Focuses accessibility added
  • Milestone changed from 4.2 to Future Release

After chatting with @iseulde about this a little bit, we've settled on punting this to future release for two specific reasons:

  1. The patch is too broad. It touches a lot of different focus styles and is tough to follow what needs testing.
  1. We need a lot of testing on a lot of devices. Each focus style exponentially increases the testing effort.

Best recommendation would be to handle the focus styles individually in separate patches and/or tickets.

#17 follow-up: @afercia
5 years ago

@drew fine with punting :) just as a reminder, the selects in the media toolbar are still totally broken in IE 8 both in Media Library -> Grid mode and in the Add Media modal. Without the IE 8 fixes WordPress 4.2 will ship a really bad experience for IE 8 users, see screenshots:

Media Library grid mode:

https://cldup.com/cCA8uhKJj9.png

Add Media modal:

https://cldup.com/1VtkVs6L_y.png

#18 in reply to: ↑ 17 @helen
5 years ago

Replying to afercia:

just as a reminder, the selects in the media toolbar are still totally broken in IE 8 both in Media Library -> Grid mode and in the Add Media modal. Without the IE 8 fixes WordPress 4.2 will ship a really bad experience for IE 8 users

That's #30725.

#19 @afercia
5 years ago

Most of the issues here are now split in separate tickets with the exception of the Plugins filter links, see:

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


5 years ago

#21 @afercia
5 years ago

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

Discussed a bit in the last accessibility meeting on Slack, we would like to close this ticket to clean up the accessibility tickets list. The only outstanding part is about the Plugins (and Themes) padding, see comment:8. I'd say it's a minor issue: the focus style there maybe is not so nice to see but it's functional. Feel free to reopen if I'm missing something.

#22 @netweb
5 years ago

  • Milestone Future Release deleted

With no commits on this ticket and issues moved to separate tickets per comment:19 I'm removing the milestone.

#23 @ocean90
5 years ago

  • Resolution changed from fixed to duplicate
Note: See TracTickets for help on using tickets.