Twenty Nineteen: Consider a custom hover/underline style

[allancole]

Originally reported by ​@afercia in the Twenty Nineteen GitHub repo:

The underline style on links using the Hoefler Text font is very thin in the Firefox browser and almost not readable, compared to other browsers (see attachments).

We should consider replacing the underline with a custom border that can be more consistent across browsers and font stacks.

More details here: ​https://github.com/WordPress/twentynineteen/pull/186#issuecomment-431418217

Original ticket here: ​https://github.com/WordPress/twentynineteen/issues/278

[allancole]

Hoefler Text underline in Safari

[allancole]

Hoefler Text underline in Firefox

[sabernhardt]

I think it's probably too late to switch to a border or similar styling.

However, both Firefox and Safari added support for ​text-decoration-thickness last year. Perhaps that could be set to 2 pixels (or an em-based measurement) for more consistency between fonts.

Maybe we also could include ​text-decoration-skip-ink for any browsers that support the property.

[danfarrow]

Add text-decoration-thickness: 2px and text-decoration-skip-ink: auto to declaration blocks containing text-decoration: underline. Rebuild style-rtl.css and style.css

[danfarrow]

While looking into this ticket I soon realised that this is an issue on Mac Firefox only.

I have uploaded a patch (my first!) but as I don't have access to a Mac I'm unable to test it.

Also I rebuilt style.css & style-rtl.css as part of the patch. I'm not sure if that was the right approach - if anybody could advise I'd be very grateful.

[sabernhardt]

@danfarrow Thanks for the patch!

As @desrosj ​noted on Slack, committers can compile the stylesheets if they are not included within the patch. Including those still can help people test patches, though.

I haven't tested (on Mac), but I see the compiled style.css removed "block-patterns" from the Tags list.

[danfarrow]

@sabernhardt That's very helpful, thank you!

I didn't spot the tag change (although it is obvious now you mention it), maybe my local repo needs updating?

I'll figure it out tomorrow

[danfarrow]

Add text-decoration-thickness: 2px and text-decoration-skip-ink: auto to declaration blocks containing text-decoration: underline. Rebuild style-rtl.css. Rebuild style.css, retaining the block-patterns tag

[danfarrow]

[Previous diff was not fixed - this one is the fixed diff!] Add text-decoration-thickness: 2px and text-decoration-skip-ink: auto to declaration blocks containing text-decoration: underline. Rebuild style-rtl.css. Rebuild style.css, retaining the block-patterns tag

[danfarrow]

@sabernhardt It appears that the block-patterns tag is missing from the repo's style.scss so I will create have created a new ticket for that #52159

I have manually removed that section from the diff and have uploaded a new version ready for testing - if anybody on a Mac is able to have a look that would be great, thank you!

[sabernhardt]

Somehow the RTL stylesheet has a thickness of 20px instead of 2.

[danfarrow]

Oh whoops somehow I failed to notice that, sorry! I will try again

[audrasjb]

Moving to Future release pending a refreshed patch.

Reminder: we simply need to replace the 20px thickness to 2px.

The original diff submitted by ​@danfarrow did not work for me, so I recreated the diff while fixing the 20px in style-rtl.

Trac ticket: https://core.trac.wordpress.org/ticket/45925

[neychok]

Hey @danfarrow , your diff did not work for me, so I just applied the changes again and created a PR while also changing the 20px to 2px in style-rtl.css

[danfarrow]

@neychok Wonderful, thank you! Where did those last 3 months go?!

[audrasjb]

Thanks @neychok! I added a small change request in your PR :)

[neychok]

Thank you @audrasjb for the review, I did the change you requested

[sabernhardt]

The PR is updated (Thanks!). When testing, check the focus style in the entry content area, comments section, the calendar widget, and another place such as the top navigation.

[multidots1896]

Added patch

[audrasjb]

@multidots1896 thanks for the new patch, however could you please also explain why do you think it would be better to use your proposal instead of the pull request proposed a few months ago? The implementation is slightly different, but it's hard to understand the purpose of this new patch while we already have a PR for this issue. Thanks :-)

[multidots1896]

Hi @audrasjb

Yes, I have alternate solution

text-underline-offset: auto;

is different property and it is usable. but

text-decoration-thickness: 2px;

is remain same in your PR.so that's why i have applied patch.

Thanks

[sabernhardt]

Perhaps we should simplify this and only commit the change common to both patches, the text-decoration-thickness property.

During an ​Accessibility bug scrub, @joedolson said "I don't see how setting a property to the default value fixes anything" in regard to text-underline-offset: auto in 45925.patch. However, auto is also the default value for text-decoration-skip-ink. Setting that might override either none or all if someone customizes the skip-ink style (in a browser that supports all).

[audrasjb]

@sabernhardt so if I understand correctly, the ideal patch would be ​https://github.com/WordPress/wordpress-develop/pull/2309/files, without text-decoration-skip-ink: auto;, right?

[maartenj]

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/45925/45925.patch

Environment

• OS: macOS 12.6
• Web Server: Nginx
• PHP: 7.4.30
• WordPress: 6.1-alpha-53344-src
• Browser: Firefox 104.0.2
• Theme: Twenty Nineteen

Actual Results

• ✅ Issue resolved with patch.

[sabernhardt]

Yes, I think removing text-decoration-skip-ink from PR 2309 would improve it.

[neychok]

Should I go ahead and update my PR by removing text-decoration-skip-ink?

[audrasjb]

@sabernhardt thanks!
@neychok I already updated your PR. Waiting for unit tests to pass, then I'll commit the changeset.

[audrasjb]

In 54171:

Twenty Nineteen: Define underline thickness for links.

This changeset adds text-decoration-thickness: 2px; to all underlined link to ensure that underline thickness stay consistent across browsers. This fixes an issue where the underline style on links using the Hoefler Text font was too thin in Firefox.

Props allancole, sabernhardt, danfarrow, audrasjb, neychok, multidots1896, maartenj.
Fixes #45925.

Committed in https://core.trac.wordpress.org/changeset/54171