WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 16 hours ago

#47138 reopened defect (bug)

Visible labelling only done via placeholder attribute

Reported by: anevins Owned by: audrasjb
Milestone: 5.3 Priority: normal
Severity: minor Version:
Component: Media Keywords: has-screenshots wpcampus-report form-controls has-patch
Focuses: ui, accessibility Cc:
PR Number:

Description

Moved from the WPCampus accessibility report issues on GitHub, see https://github.com/WordPress/gutenberg/issues/15301

  • Severity:
    • Low
  • Affected Populations:
    • Low-Vision
    • Motor Impaired
    • Cognitively Impaired
  • Platform(s):
    • All / Universal
  • Components affected:
    • Media Dialog

Issue description
In the "Media Library" panel of the "Featured Image" modal dialog, a
live search field is available to filter uploaded images and narrow
results.

While there is a label element present, it is rendered visually-hidden,
while a placeholder with similar text is visually present in the input.

Users who've entered any values must empty the field to see the
placeholder text, or otherwise memorise it. Users with cognitive
disabilities may have trouble remembering what the filled field does,
while speech recognition users cannot see the name they can speak to set
focus on the field. Low-vision users with high text-size may not be able
to see the whole placeholder even when it's visible, if its value is
clipped by the edge of the input.

Issue Code

    <div class="media-toolbar-primary search-form">


        <label for="media-search-input" class="screen-reader-text">Search Media</label>


        <input type="search" placeholder="Search media items..." id="media-search-input" class="search">


    </div>


    ...


    .screen-reader-text  {


        border: 0;


        clip: rect(1px,1px,1px,1px);


        -webkit-clip-path: inset(50%);


        clip-path: inset(50%);


        height: 1px;


        margin: -1px;


        overflow: hidden;


        padding: 0;


        position: absolute;


        width: 1px;


        word-wrap: normal!important;


    }

Remediation Guidance
Make labels visible instead of using placeholders, as much as possible.

Relevant standards
N/A

Note: This issue may be a duplicate with other existing accessibility-related bugs in this project. This issue comes from the Gutenberg accessibility audit, performed by Tenon and funded by WP Campus. This issue is GUT-64 in Tenon's report

See also https://core.trac.wordpress.org/ticket/40331

Attachments (31)

57185908-ef8bc400-6e89-11e9-8c12-de9e78994d6e.png (219.5 KB) - added by anevins 5 months ago.
modal-search-label.png (55.5 KB) - added by audrasjb 4 months ago.
Modal search
library-search-label.png (55.2 KB) - added by audrasjb 4 months ago.
Library search
library-search-label-mobile.png (43.1 KB) - added by audrasjb 4 months ago.
Library search on mobile
Capture d’écran 2019-09-28 à 14.45.52.png (579.4 KB) - added by audrasjb 2 weeks ago.
Search label
Capture d’écran 2019-09-28 à 14.46.01.png (632.9 KB) - added by audrasjb 2 weeks ago.
Search label focused
Capture d’écran 2019-09-28 à 14.49.43.png (48.2 KB) - added by audrasjb 2 weeks ago.
Search label on mobile
47138.diff (3.5 KB) - added by audrasjb 2 weeks ago.
visible search label instead of placeholder in media view
01 at 1024.png (106.9 KB) - added by afercia 2 weeks ago.
Media Library at 1024
02 media modal.png (228.8 KB) - added by afercia 2 weeks ago.
Media modal dialog
03 current 5.2.png (46.0 KB) - added by afercia 2 weeks ago.
WordPress 5.2: pre-existing issue with elements that disappear
47138.2.diff (7.7 KB) - added by afercia 12 days ago.
47138 01.png (249.7 KB) - added by afercia 12 days ago.
47138 02.png (175.9 KB) - added by afercia 12 days ago.
47138 03.png (161.2 KB) - added by afercia 12 days ago.
list-layout-large.png (166.3 KB) - added by audrasjb 10 days ago.
List layout large
list-layout-intermediate-1.png (119.5 KB) - added by audrasjb 10 days ago.
List layout intermediate 1
list-layout-intermediate-2.png (60.3 KB) - added by audrasjb 10 days ago.
List layout intermediate 2
list-layout-small.png (45.3 KB) - added by audrasjb 10 days ago.
List layout small
grid-layout-large.png (409.0 KB) - added by audrasjb 10 days ago.
Grid layout large
grid-layout-intermediate-1.png (354.4 KB) - added by audrasjb 10 days ago.
Grid layout intermediate 1
grid-layout-intermediate-2.png (263.8 KB) - added by audrasjb 10 days ago.
Grid layout intermediate 2
grid-layout-small.png (144.4 KB) - added by audrasjb 10 days ago.
Grid layout small
47138.3.diff (7.7 KB) - added by afercia 8 days ago.
47138.4.diff (7.9 KB) - added by afercia 8 days ago.
47138 margin.png (9.3 KB) - added by afercia 8 days ago.
1 pixel margin not allowing correct horizontal alignment
47138 spinner default.png (108.1 KB) - added by afercia 16 hours ago.
In most of the cases the spinner is the last element within the toolbar
47138 suggested image dimensions customizer.png (420.5 KB) - added by afercia 16 hours ago.
Customizer - Select logo - dimensions instructions: the spinner is followed by the instructions
47138 suggested image dimensions spinner.png (119.5 KB) - added by afercia 16 hours ago.
When the spinner appears, it is also misplaced
47138 edit gallery.png (55.1 KB) - added by afercia 16 hours ago.
Same for the Edit Gallery drag and drop instructions
47138.5.diff (1017 bytes) - added by afercia 16 hours ago.

Change History (72)

#1 @afercia
5 months ago

  • Keywords form-controls added

#2 @audrasjb
5 months ago

  • Keywords needs-design added

#3 @afercia
5 months ago

  • Milestone changed from Awaiting Review to 5.3

#4 @audrasjb
5 months ago

  • Keywords good-first-bug added; needs-design removed

I'd suggest to remove the screen-reader-text class to the label and to display it on the left of the field (do not forget to handle RTL languages).

Adding good- first-bug keyword as the issue looks like a nice one for new contributors.

#5 @afercia
5 months ago

Worth reminding the WPCampus accessibility report focused on user interface parts directly visible from the block editor. However, this is a broader issue across the whole WordPress admin pages, see #40331.

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


5 months ago

#7 @afercia
5 months ago

  • Keywords good-first-bug removed

#8 @karmatosed
4 months ago

  • Keywords needs-design-feedback added

@audrasjb
4 months ago

Modal search

@audrasjb
4 months ago

Library search

@audrasjb
4 months ago

Library search on mobile

#9 @audrasjb
4 months ago

Hi there, the 3 screenshots above are a first workaround for this issue.

#10 @karmatosed
3 months ago

Would it be possible to shorten this to 'search' or would it be preferable to have the context? I suggest this as you are in media so expecting it to search for media feels the right choice.

#11 @audrasjb
3 months ago

  • Owner set to audrasjb
  • Status changed from new to accepted

@karmatosed good point.
I see no objection on my side :)

#12 @afercia
3 months ago

No strong objections either, especially now that the modal dialog is correctly announced and it's the only perceivable content within the page.

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


8 weeks ago

#14 @karmatosed
8 weeks ago

  • Keywords needs-design-feedback removed

As this has design feedback, going to remove the keyword.

#15 @afercia
7 weeks ago

Definitely worth fixing this single instance within the media modal dialog.

However, worth reminding again this is a broader issue. There are several instances in core and Gutenberg where visible labeling is done only via the placeholder attribute. This is just to say that even if this search input field gets fixed, core and Gutenberg won't be "OK" on this matter.

Last edited 6 weeks ago by afercia (previous) (diff)

#16 @afercia
6 weeks ago

  • Type changed from defect (bug) to task (blessed)

Based on the previous comment, I'd like to propose to make this ticket a "blessed" task for the 5.3 release cycle. This way, multiple commits can reference this ticket in the attempt to address as many instances as possible in core and Gutenberg.

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


4 weeks ago

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


3 weeks ago

@audrasjb
2 weeks ago

Search label focused

@audrasjb
2 weeks ago

Search label on mobile

#19 @audrasjb
2 weeks ago

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

In 47138.diff, on both List and Grid media views:

  • Remove placeholders for media search input
  • Remove screen-reader-textclass for media search label
  • Add media-search-input-label class on media search label
  • Add CSS styles to align label vertically

Works fine on my side. Would love to get some feedback, especially concerning JS and CSS changes :-)

Cheers,
Jb

Last edited 2 weeks ago by afercia (previous) (diff)

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


2 weeks ago

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


2 weeks ago

@audrasjb
2 weeks ago

visible search label instead of placeholder in media view

#22 follow-up: @afercia
2 weeks ago

Thanks @audrasjb. Quickly reviewed, spotted a few things:

  • There are a couple of JS errors: run grunt jshint or grunt jshint:media to see them (unused l10n variable and a comma to remove).
  • Not sure we should remove the existing strings searchMediaLabel and searchMediaPlaceholder: there's a chance plugins are using them for any custom things they add to the media views. I'd rather introduce a new string.
  • On smaller viewports and in the media modal (test with the Classic Editor) there's not enough room for the visible label (see screenshots below). The CSS should be adjusted a bit. Note that the current CSS is a bit tricky especially for the modal :(

Worth noting the media modal responsiveness is far from ideal even on current WordPress 5.2 and in all previous versions. At some point on smaller viewports some elements get completely hidden (float drop). However, this is a pre-existing issue that is out of the scope of this ticket.

@afercia
2 weeks ago

Media Library at 1024

@afercia
2 weeks ago

Media modal dialog

@afercia
2 weeks ago

WordPress 5.2: pre-existing issue with elements that disappear

#23 @azaozz
2 weeks ago

Having a visible label on the search input field is nice but it currently introduces several visual regressions. It seems the whole layout CSS will need a revision in order to accomplish this.

I'm not sure if there is enough time to refactor the CSS and find and fix all regressions for 5.3. Beta-2 is tomorrow!

@audrasjb @afercia @karmatosed please consider moving this enhancement to 5.4. In its current state it can reduce accessibility and usability for all users.

Last edited 2 weeks ago by azaozz (previous) (diff)

#24 follow-up: @audrasjb
2 weeks ago

@azaozz thanks for your feedback on this ticket.

I'm also considering moving this ticket to 5.4, but not for the same reason :D
I'm simply wondering about inconsistencies introduced if we fix the placeholder/label issue on Media screen but not on search fields taking place on other screens (posts, users, etc.). We'll probably move this ticket to the next release during our next bug scrub, as it will probably not land in beta 2 tomorrow (still no validated patch).

Could you please add details on the specific visual regressions you found when you tested this patch? Would be useful for future works on this ticket :-)

#25 in reply to: ↑ 24 @azaozz
2 weeks ago

Replying to audrasjb:

Generally the same things @afercia reported above.

The place there is... not designed (technically) very well. It has two parts, primary and secondary "toolbar" and I think plugins can add items in both. Changing the height of the primary part will affect the rest of the layout. Alternatively changing the current 33% max-width would affect items added by plugins...

Thinking best would be to partially refactor that bit (at least) so the label can be shown properly and not affect the surroundings. The initial idea was that the two parts will be horizontally if there is enough space, and switch to vertically when there's not.

#26 @afercia
2 weeks ago

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

Yep I'd agree with @azaozz that the media modal would need some refactoring. This UI has been a bit neglected for years: fixed heights and absolute positioning were a few years ago maybe the only option to build a decent layout. They don't scale when adding content though, the responsiveness is not ideal to be fair, and today there are better CSS techniques.

Worth noting though that in #47610 I already partially refactored the height of the toolbar following the feedback provided by the design team. That is to make room for the new headings. Changing the height doesn't alter the widths :) and basically keeps the current responsiveness unaltered (with all its existing bugs and breakages).

Coincidentally, now there's enough room above the search field to place the visible label there. See the final design in the screenshots after comment https://core.trac.wordpress.org/ticket/47610#comment:30

Seems to me this ticket needs only a quick patch refresh to adjust the CSS and move the visible label above the search field.

#27 @mikeschroder
12 days ago

Does anyone have the chance to make the change that @afercia is recommending to allow folks to test?

@azaozz @audrasjb Do you think that would be sufficient to resolve your concerns?

#28 @afercia
12 days ago

@mikeschroder hello. I'm having a look at this right now. Leaning towards minimal CSS changes.

I've also experimented a bit with flexbox and for the future I'd recommend refactoring the toolbar using flexbox, which allows to entirely remove the weird max-widths and floats. In the Media Library this would solve all layout issues. In the media modal, the fixed height should be removed, which complicates things a bit but I think it's doable. Not something to try now though :)

@afercia
12 days ago

#29 @afercia
12 days ago

  • Keywords has-patch commit added; needs-patch removed

47138.2.diff refreshes the patch:

  • uses flexbox only for the search form
  • keeps the old strings, adds a new one
  • in the media modal: the visible label is now above the input field: this way, it doesn't alter in any way the existing widths / floats, in teh responsive view all the existings drop-float and breakages are unchanged :)
  • in the media library (the grid mode): flexbox helps a bit: even if the label is horizontally aligned and reduces the available space for the form controls, the responsiveness is basically unchanged

Looks good to me, I'd greatly appreciate a final review.

Screenshots attached below.

@afercia
12 days ago

@afercia
12 days ago

@afercia
12 days ago

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


11 days ago

@audrasjb
10 days ago

List layout large

@audrasjb
10 days ago

List layout intermediate 1

@audrasjb
10 days ago

List layout intermediate 2

@audrasjb
10 days ago

List layout small

@audrasjb
10 days ago

Grid layout large

@audrasjb
10 days ago

Grid layout intermediate 1

@audrasjb
10 days ago

Grid layout intermediate 2

@audrasjb
10 days ago

Grid layout small

#31 @audrasjb
10 days ago

Tested the last patch on my side and everything is looking good.
Ready to go :)

#32 in reply to: ↑ 22 @SergeyBiryukov
9 days ago

Replying to afercia:

Not sure we should remove the existing strings searchMediaLabel and searchMediaPlaceholder: there's a chance plugins are using them for any custom things they add to the media views. I'd rather introduce a new string.

In the plugin directory, there are 3 matches for searchMediaLabel and 1 match for searchMediaPlaceholder, though only Enhanced Media Library appears to be affected.

If we're keeping the strings, then the // placeholder (no ellipsis) comment should also be kept, as per [34233].

@afercia
8 days ago

#33 @afercia
8 days ago

47138.3.diff keeps the // placeholder (no ellipsis) comment.

Last edited 8 days ago by afercia (previous) (diff)

@afercia
8 days ago

#34 @afercia
8 days ago

47138.4.diff resets the search input field 1 pixel margin to improve alignment with its label. See screenshot below.

Note: there are probably historical reasons for this 1 pixel margin applied to all the input and select elements in forms.css but it's extremely annoying when it comes to properly align elements. I'm not sure there are good reasons to keep it and I'd like to propose to audit it for the next release cycle.

@afercia
8 days ago

1 pixel margin not allowing correct horizontal alignment

#35 @afercia
8 days ago

  • Type changed from task (blessed) to defect (bug)

Regarding:

Based on the previous comment, I'd like to propose to make this ticket a "blessed" task for the 5.3 release cycle. This way, multiple commits can reference this ticket in the attempt to address as many instances as possible ...

Unfortunately it didn't happen for lack of resources and prioritization. This ticket addresses only 2 instances: in the Media Library and in the Media modal. A follow-up ticket should be created to audit al the other instances in core. Changing the type of this ticket back to "bug".

#36 @afercia
8 days ago

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

In 46418:

Accessibility: Media: Improve the search media field labelling.

Visible <label> elements benefit all users. The placeholder attribute should not be used as a replacement for visible labels.
Instead, it's supposed to be used only for a short hint to aid users with data entry e.g. a sample value or a brief description of the expected format.

Screen readers may not announce a placeholder attribute at all. Other users may suffer from the lack of a visible label and a placeholder used as replacement, for example:

  • users with cognitive disabilities may have trouble remembering what the filled field does
  • speech recognition users cannot see the name they can speak to set focus on the field
  • low-vision users with high text-size may not be able to see the whole placeholder even when it's visible, if its value is clipped by the edge of the input

Props anevins, audrasjb, karmatosed, azaozz, SergeyBiryukov, afercia.
See #40331.
Fixes #47138.

#37 @afercia
8 days ago

  • Keywords commit removed

#38 @afercia
4 days ago

  • Keywords needs-patch added; has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

I noticed a couple edge cases where the spinner in the toolbar is misplaced. Will add a patch and instructions to test soon.

Last edited 17 hours ago by afercia (previous) (diff)

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


4 days ago

#40 @afercia
16 hours ago

  • Keywords has-patch added; needs-patch removed

On a fast network and with powerful hardware the spinner may not appear at all. That's because it appears only after a brief delay and if the attachments fetch is fast enough, the spinner is not displayed.

To test, the easier way is to throttle the network speed to "Slow 3G" from the Chrome dev tools > Network tab.

[46418] added float: none to the spinner CSS. This works in most of the cases in the main views. The problem the latest patch 47138.5.diff seeks to solve is that while normally the spinner is the last item in the toolbar, in other views there are other elements before it. For example when there are "instructions" for the image dimensions or the drag and drop instructions. See screenshots attached below.

In these cases, whether the spinner appears or not, it still takes space and the text is shifted to the right.

Seems to me the easiest solution is making the spinner always be the last element within the toolbar.

Some testing would be nice :)

@afercia
16 hours ago

In most of the cases the spinner is the last element within the toolbar

@afercia
16 hours ago

Customizer - Select logo - dimensions instructions: the spinner is followed by the instructions

@afercia
16 hours ago

When the spinner appears, it is also misplaced

@afercia
16 hours ago

Same for the Edit Gallery drag and drop instructions

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


16 hours ago

@afercia
16 hours ago

Note: See TracTickets for help on using tickets.