WordPress.org

Make WordPress Core

#38932 closed defect (bug) (fixed)

Add unit (px) to suggested image size in customizer

Reported by: Presskopp Owned by: afercia
Milestone: 4.8 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch has-screenshots
Focuses: accessibility Cc:

Description

When you go to Customizer > Header Media > Add new image

you see a notice about image size like:

Suggested image dimensions: 2000 × 1200

It should have a unit

Suggested image dimensions: 2000 × 1200 px

Attachments (6)

missing_px.jpg (37.7 KB) - added by Presskopp 18 months ago.
38932.patch (2.1 KB) - added by stormrockwell 18 months ago.
replaced 2000 x 1200 format to the confirmed 2000 by 1200 pixels format
38932.2.patch (2.8 KB) - added by stormrockwell 18 months ago.
Fixed hard-coded text
38932.3.patch (2.7 KB) - added by stormrockwell 18 months ago.
38932.4.patch (2.9 KB) - added by stormrockwell 18 months ago.
added translators comments
38932.diff (2.9 KB) - added by afercia 13 months ago.

Download all attachments as: .zip

Change History (24)

#1 @afercia
18 months ago

  • Focuses accessibility added
  • Version trunk deleted

From an accessibility perspective, worth noting all similar abbreviations should be avoided or expanded with the full form. For example, the suggested dimensions (including the missing "px") gets read out by a screen reader as: one two zero zero times two eight zero pee ex Numbers should be properly formatted too. Recently, in another project, I've changed a similar string in "2000 by 1200 pixels" which is slightly better.

Same for the file size: one gee bee.

By the way, this applies to all the similar cases in core.

#2 @dd32
18 months ago

@afercia curiously, would <abbr title="pixels">px</abbr> work? (maybe with an aria-* attribute?)

#3 @afercia
18 months ago

@dd32 the title attribute works like a description and gets announced in addition to the visible text. So:

<abbr title="pixels">px</abbr> = pee ex (short pause here) pixels

Also, there's no guarantee the title attribute will be announced, it depends on the verbosity settings users can tweak in their screen readers.

The aria-label attribute replaces the content instead:

<span aria-label="pixels">px</span> = pixels

By the way, it tends to work best on interactive elements. Thinking also at the resulting translatable string, it would be simpler to use just plain text.

Last edited 18 months ago by afercia (previous) (diff)

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


18 months ago

#5 @rianrietveld
18 months ago

  • Milestone changed from Awaiting Review to Future Release

We discussed this in the accessibility bug scrub. Thank you for this ticket @Presskopp

We concluded: just use plain simple 'pixels', adding extra HTML to explain px seems a unnecessary complication. Also the x can be replaced by the word 'by' for a clear pronunciation by screen readers. So the sentence will be:

2000 by 1200 pixels

Note by @celloexpressions that the custom header controls currently say 2000 &times; 1200 pixels, so that should be unified at some point.

@stormrockwell
18 months ago

replaced 2000 x 1200 format to the confirmed 2000 by 1200 pixels format

#6 @stormrockwell
18 months ago

  • Keywords has-patch added

#7 @westonruter
18 months ago

  • Keywords needs-patch added; has-patch removed

@stormrockwell thanks for the patch, but you have hard-coded English text in there. It needs to be translatable.

@stormrockwell
18 months ago

Fixed hard-coded text

#8 @stormrockwell
18 months ago

Could someone confirm that I am adding the translation functions correctly?

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


18 months ago

#10 @westonruter
18 months ago

@stormrockwell there should probably be a single string that gets used with substitutions. For example suggestedDimensions could look like: __( '%1$s by %2$s pixels' ) and then in JS you can replace those placeholders with the values for width and height respectively.

#11 @stormrockwell
18 months ago

  • Keywords has-patch added; needs-patch removed

Thank you very much @westonruter for the help. I reformatted 38932.3.patch following those tips.

#12 @westonruter
18 months ago

Patch looks good. The question of the hour then is how this gets milestoned. Does this go to 4.7.1 or 4.8?

#13 @ocean90
18 months ago

38932.3.patch is missing translator comments for the placeholders. /* translators: 1: ...


Does this go to 4.7.1 or 4.8?

I vote for 4.8

@stormrockwell
18 months ago

added translators comments

#14 @westonruter
18 months ago

  • Milestone changed from Future Release to 4.8

#15 @Presskopp
16 months ago

I made a new related ticket about adding units in media view: #39667

Last edited 16 months ago by Presskopp (previous) (diff)

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


14 months ago

@afercia
13 months ago

#17 @afercia
13 months ago

  • Keywords has-screenshots added

Refreshed patch.

#18 @afercia
13 months ago

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

In 40573:

Accessibility: Improve the suggested image size text in the media views.

Adds pixels as unit, avoiding abbreviation for a clearer pronunciation by screen
readers. Also, adds the word by instead of the special character times.

Props Presskopp, stormrockwell.
Fixes #38932.

Note: See TracTickets for help on using tickets.