WordPress.org

Make WordPress Core

Opened 14 months ago

Closed 9 months ago

#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 14 months ago.
38932.patch (2.1 KB) - added by stormrockwell 14 months ago.
replaced 2000 x 1200 format to the confirmed 2000 by 1200 pixels format
38932.2.patch (2.8 KB) - added by stormrockwell 14 months ago.
Fixed hard-coded text
38932.3.patch (2.7 KB) - added by stormrockwell 14 months ago.
38932.4.patch (2.9 KB) - added by stormrockwell 14 months ago.
added translators comments
38932.diff (2.9 KB) - added by afercia 9 months ago.

Download all attachments as: .zip

Change History (24)

#1 @afercia
14 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
14 months ago

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

#3 @afercia
14 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 14 months ago by afercia (previous) (diff)

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


14 months ago

#5 @rianrietveld
14 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
14 months ago

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

#6 @stormrockwell
14 months ago

  • Keywords has-patch added

#7 @westonruter
14 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
14 months ago

Fixed hard-coded text

#8 @stormrockwell
14 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.


14 months ago

#10 @westonruter
14 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
14 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
14 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
14 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
14 months ago

added translators comments

#14 @westonruter
14 months ago

  • Milestone changed from Future Release to 4.8

#15 @Presskopp
12 months ago

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

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

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


10 months ago

@afercia
9 months ago

#17 @afercia
9 months ago

  • Keywords has-screenshots added

Refreshed patch.

#18 @afercia
9 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.