Support link underline preference #3848

joshblack · 2023-10-20T17:12:58Z

This is a follow-up to #3720 after it was reverted for the latest release. Original description included below.

This was initiated by our need for underlined links in blocks of text, and supports dotcom's new user setting to render links with underlines. The link underline setting is currently being tested on dotcom behind a feature flag.

A new setting has been added to the Storybook toolbar that allows us to preview what a component would look like when the user has specified that they prefer link underlines.

Closes https://github.com/github/primer/issues/2536

Screenshots

User who turned link underlines on in their GitHub settings

Additional context about link underline settings

Issue: https://github.com/github/releases/issues/3340
Dotcom PR: https://github.com/github/github/pull/278701

Merge checklist

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

* adds Prose component and stories * adds tests and makes some minor changes * Create happy-rivers-attend.md * handle link underline preferences * adds visual regression testing * fixes storybook ID in Prose docs data * updates changeset with changed components data * rm Prose component * test(vrt): update snapshots * test(e2e): separate storybook global args by semicolon instead of ampersand * test(vrt): update snapshots * update snapshots --------- Co-authored-by: joshblack <joshblack@users.noreply.github.com>

changeset-bot · 2023-10-20T17:13:01Z

🦋 Changeset detected

Latest commit: 08f8153

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-10-20T17:13:18Z

Uh oh! @joshblack, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

github-actions · 2023-10-20T17:13:25Z

Uh oh! @joshblack, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

joshblack · 2023-10-20T17:16:11Z

Quick rollout plan notes from @mperrotti:

make sure BaseStyles is being imported or that --prefers-link-underlines is being set
merge fix(Link): update underline={false} to disable underline #3838 that updates how underline works and cut a PRC release with those changes
upgrade PRC in dotcom and request a review lab deployment to test things out

The postmortem for this incident was created over at: https://github.com/github/primer/discussions/2788

github-actions · 2023-10-20T17:16:11Z

size-limit report 📦

Path	Size
dist/browser.esm.js	104.77 KB (+0.15% 🔺)
dist/browser.umd.js	105.36 KB (+0.15% 🔺)

pksjce · 2023-10-24T02:09:01Z

src/BaseStyles.tsx

@@ -26,6 +26,13 @@ const GlobalStyle = createGlobalStyle<{colorScheme?: 'light' | 'dark'}>`
  details-dialog:focus:not(:focus-visible):not(.focus-visible) {
    outline: none;
  }
+
+  /* Used to fake conditional styles using a technique by Lea Verou: https://lea.verou.me/blog/2020/10/the-var-space-hack-to-toggle-multiple-values-with-one-custom-property/ */


omg so clever!

pksjce · 2023-10-24T02:09:53Z

src/BaseStyles.tsx

+  /* We have to use a zero-width space character (\u200B) as the value instead of a regular whitespace character because styled-components strips out properties that just have a whitespace value. */
+  :root {--prefers-link-underlines: \u200B;}
+  [data-a11y-link-underlines='true'] {
+    --prefers-link-underlines: initial;


what do you think of using the off/on variables like Lea suggested in her blogpost for a clearer understanding of this variable assignment?

siddharthkp · 2023-10-25T10:47:01Z

src/Link/Link.tsx

@@ -22,9 +22,9 @@ const hoverColor = system({

 const StyledLink = styled.a<StyledLinkProps>`
  color: ${props => (props.muted ? get('colors.fg.muted')(props) : get('colors.accent.fg')(props))};
-  text-decoration: ${props => (props.underline ? 'underline' : 'none')};
+  text-decoration: ${props => (props.underline ? 'underline' : 'var(--prefers-link-underlines, underline)')};


Assuming BaseStyles is an optional wrapper... (correct me if it's not!)

Instead of changing the default, should we keep that default and let folks change the default by switching-on --prefers-link-underlines?

text-decoration: props.underline ? 'underline' : 'var(--prefers-link-underlines, none)'

If --prefers-link-underlines is not defined, for example when BaseStyles is not imported, the fallback would be none

@joshblack @pksjce Does this sound right or am I missing the goal here?

siddharthkp · 2023-10-25T10:53:27Z

src/BaseStyles.tsx

+
+  /* Used to fake conditional styles using a technique by Lea Verou: https://lea.verou.me/blog/2020/10/the-var-space-hack-to-toggle-multiple-values-with-one-custom-property/ */
+  /* We have to use a zero-width space character (\u200B) as the value instead of a regular whitespace character because styled-components strips out properties that just have a whitespace value. */
+  :root {--prefers-link-underlines: \u200B;}


Very cool hack! But we're only using this for one property: text-decoration so I'm not sure if we should prefer the explicit none here?

joshblack · 2023-11-30T22:19:42Z

Superseded by: #3946

joshblack changed the title ~~Support link underline preference (#3720)~~ Support link underline preference Oct 20, 2023

Merge branch 'main' into feat/add-link-underline-preference-setting

08f8153

github-actions bot temporarily deployed to storybook-preview-3848 October 20, 2023 17:19 Inactive

joshblack deployed to github-pages October 20, 2023 17:20 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-3848 October 20, 2023 17:21 View deployment

pksjce reviewed Oct 24, 2023

View reviewed changes

siddharthkp assigned mperrotti Oct 25, 2023

siddharthkp reviewed Oct 25, 2023

View reviewed changes

siddharthkp assigned siddharthkp and unassigned siddharthkp Oct 30, 2023

joshblack closed this Nov 30, 2023

joshblack deleted the feat/add-link-underline-preference-setting branch November 30, 2023 22:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support link underline preference #3848

Support link underline preference #3848

joshblack commented Oct 20, 2023 •

edited

Loading

changeset-bot bot commented Oct 20, 2023 •

edited

Loading

github-actions bot commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

joshblack commented Oct 20, 2023 •

edited

Loading

github-actions bot commented Oct 20, 2023 •

edited

Loading

pksjce Oct 24, 2023

pksjce Oct 24, 2023

siddharthkp Oct 25, 2023

siddharthkp Nov 2, 2023

siddharthkp Oct 25, 2023

joshblack commented Nov 30, 2023

Support link underline preference #3848

Support link underline preference #3848

Conversation

joshblack commented Oct 20, 2023 • edited Loading

Screenshots

User who turned link underlines on in their GitHub settings

Additional context about link underline settings

Merge checklist

changeset-bot bot commented Oct 20, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

joshblack commented Oct 20, 2023 • edited Loading

github-actions bot commented Oct 20, 2023 • edited Loading

size-limit report 📦

pksjce Oct 24, 2023

Choose a reason for hiding this comment

pksjce Oct 24, 2023

Choose a reason for hiding this comment

siddharthkp Oct 25, 2023

Choose a reason for hiding this comment

siddharthkp Nov 2, 2023

Choose a reason for hiding this comment

siddharthkp Oct 25, 2023

Choose a reason for hiding this comment

joshblack commented Nov 30, 2023

joshblack commented Oct 20, 2023 •

edited

Loading

changeset-bot bot commented Oct 20, 2023 •

edited

Loading

joshblack commented Oct 20, 2023 •

edited

Loading

github-actions bot commented Oct 20, 2023 •

edited

Loading