WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 6 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-testing
Focuses: Cc:

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 (5)

link space underlined.png (109.5 KB) - added by afercia 17 months ago.
link in additional css .png (32.1 KB) - added by afercia 17 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 16 months ago.
47303.2.diff (8.4 KB) - added by isabel_brison 16 months ago.
47303.3.diff (8.4 KB) - added by isabel_brison 6 months ago.

Download all attachments as: .zip

Change History (15)

#1 @SergeyBiryukov
17 months ago

  • Description modified (diff)

#2 @desrosj
17 months ago

  • Keywords needs-patch good-first-bug added

#3 @afercia
17 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
17 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.

#4 @isabel_brison
16 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
16 months ago

  • Owner set to isabel_brison
  • Status changed from new to assigned

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


6 months ago

#7 @mukesh27
6 months ago

  • Keywords needs-design-feedback removed

Hi there, Remove the "needs-design-feedback" as decide in today's design weekly triage.

#8 @mukesh27
6 months ago

  • Keywords needs-refresh added

Hi there!

The patch needs an update so can @isabel_brison you please update it?

On small suggestion from my end.

Use -1px top margin instead of -2px.

.screen-reader-text + .dashicons-external {
   margin-top: -1px;
   margin-left: 4px;
}

#9 @mukesh27
6 months ago

  • Keywords needs-refresh removed

#10 @isabel_brison
6 months ago

Patch updated!

Note: See TracTickets for help on using tickets.