WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 4 months ago

#47303 assigned defect (bug)

Consider removing extra trailing spaces from links with the `dashicons-external` icon

Reported by: SergeyBiryukov Owned by: isabel_brison
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: I18N Keywords: good-first-bug has-screenshots has-patch needs-design-feedback needs-testing
Focuses: Cc:
PR Number:

Description (last modified by SergeyBiryukov)

The handbook link added in [45170] has an extra space before the screen reader text and the dashicons-external icon:

The WordPress Hosting Team maintains a list of those modules, both recommended and required, in <a href=..." ...>the team handbook <span class="screen-reader-text">(opens in a new tab)</span><span class="dashicons dashicons-external" aria-hidden="true"></span></a>.

In #47300 it was brought up that the screen reader text is a hidden span, so it should NOT be preceded by any space.

Looking at other strings with the dashicons-external icon, they all have the space before the screen reader text and the icon:

  • Learn more about updating PHP
  • Read more about what WordPress requires to run.
  • Get help resolving this issue.
  • Read about debugging in WordPress.
  • Read more about why you should use HTTPS

Same for the strings added earlier in wp_dashboard_events_news(), wp_dashboard_php_nag(), and wp_direct_php_update_button():

  • Meetups
  • WordCamps
  • News
  • Learn more about updating PHP
  • Update PHP

Some of these links don't have an underline and the extra space looks OK (to me it actually looks better with the space before the icon, as is). Others, however, do have an underline and the extra space looks a bit weird.

For the links with an underline, we should either remove the trailing spaces or (preferably) split them so that the icon is linked separately.

Attachments (4)

link space underlined.png (109.5 KB) - added by afercia 5 months ago.
link in additional css .png (32.1 KB) - added by afercia 5 months ago.
Example in the Customizer (Additional CSS panel) where the space is within the visually hidden text. It just uses a wrong icon, but that's a separate issue.
47303.diff (0 bytes) - added by isabel_brison 4 months ago.
47303.2.diff (8.4 KB) - added by isabel_brison 4 months ago.

Download all attachments as: .zip

Change History (9)

#1 @SergeyBiryukov
5 months ago

  • Description modified (diff)

#2 @desrosj
5 months ago

  • Keywords needs-patch good-first-bug added

#3 @afercia
5 months ago

  • Keywords has-screenshots added

I was looking at this a few days ago (see also screenshot below) because the "underlined space" doesn't look so pretty.

Ideally, the actual printed out text should have a space like if it was normal text. This way, screen readers can read it out respecting the separation between words. For example, this text without the space:

Read more about why you should use HTTPS(opens in a new tab)

Wouldn't be ideal, while this other one:

Read more about why you should use HTTPS (opens in a new tab)

is announced in an appropriate way. Seems to me the simplest option would be using a space within the visually hidden text. For example, this is what the bundled themes generally do:

__( 'Continue reading<span class="screen-reader-text"> "%s"</span>', 'twentynineteen' )

Then, the external dashicon could use some left margin.

@afercia
5 months ago

Example in the Customizer (Additional CSS panel) where the space is within the visually hidden text. It just uses a wrong icon, but that's a separate issue.

@isabel_brison
4 months ago

#4 @isabel_brison
4 months ago

  • Keywords has-patch needs-design-feedback needs-testing added; needs-patch removed

First time contributor here! I have uploaded a patch for this issue taking into account @afercia's recommendations. I also noticed the icon was not quite aligned with the text so added an extra margin to align it. Would be great to get some feedback on it, code and design-wise :-)

#5 @talldanwp
4 months ago

  • Owner set to isabel_brison
  • Status changed from new to assigned
Note: See TracTickets for help on using tickets.