Carbon: [Link] cannot have `block` Link due to usage of `box-shadow` for interactive states

[jendowns]

What package(s) are you using?

    "@carbon/colors": "^10.1.0",
    "@carbon/grid": "^10.3.0",
    "@carbon/icons-react": "^10.3.0",
    "@carbon/import-once": "^10.1.0",
    "@carbon/layout": "^10.1.0",
    "@carbon/themes": "^10.3.0",
    "@carbon/type": "^10.2.0",
    "carbon-components": "^10.3.1",
    "carbon-components-react": "^7.3.1",

Detailed description

If you ever apply display: block to the Link component (vanilla, React, or otherwise), the box-shadow being used to create the underline on :hover/:active/:focus states will extend to the entire length of the block, way past the text of the link in some cases.

For example, go to the Carbon website (https://www.carbondesignsystem.com/components/link/code), add display: block to an example Link, and you will see the following:

Even though this is expected with the box model, this "visual defect" is very noticeable. And I don't think we should expect Link to always remain inline in various applications / products. On the contrary, a block-style Link seems like a reasonable customization / deviation to expect, right? 🤔

🚨 UPDATE: Other concerns with AVT 🚨

Please see this comment for additional a11y concerns with using box-shadow for interactive link styling: https://github.com/carbon-design-system/carbon/issues/3059#issuecomment-501997246

Proposed solution

So text-decoration: underline would work great here & passes our AVT cases!

If you used text-decoration: underline instead of the box-shadow for the :hover/:active/:focus states, then it wouldn't matter if the Link was inline or block -- that text decoration would only extend as far as the link's text:

[emyarod]

the link component has gone through quite a number of design changes in the last few months (https://github.com/carbon-design-system/carbon/issues/1263 https://github.com/carbon-design-system/carbon/issues/1801 https://github.com/carbon-design-system/carbon/issues/1900 https://github.com/carbon-design-system/carbon/issues/1995) but (to my knowledge) this is the current spec:

our box-shadow usage to create the underline is due to the underline width increase for the focus state based on the spec

need confirmation from design about if there will be any spec changes but I guess a temporary workaround would be to wrap the link in a div if you want a block level element

cc @shixiedesign? (not sure who to cc, just using the person that last updated the specfile)

[shixiedesign]

Hi @jendowns ! Link is styled with box because text-decoration: underline uses an underline that's not low enough to clear the descenders in the type and also to accommodate the focus state styling. I believe Diego from security first came up with this solution 😅Lemme know if Andrew's proposal of wrapping link in a div is an acceptable solution...I know it's not elegant. Do you have other ideas on customizing type underline's looks?

If div fails and no other underline methods exist, we can compromise some visual details to ensure better usability. Let me know your thoughts. Thanks!

[jendowns]

Thanks @emyarod & @shixiedesign!

I suggested text-decoration: underline because it's already default/standard for links, but I'm open to other solutions. A wrapping div could definitely work -- though it seems like a lot for Link styling? 🤔 (EDIT: just to add, I don't currently have any other idea aside from the text-decoration or the wrapping div but I'll let you know if something comes to mind)

I realize there's a lot of Design context for the Link refactor, but my concern is that the functionality and usage of a Link has been reduced as a result of this decision.

To that point, this issue bubbled up to me from a Security product (non production situation), where we were previously using a Link in a block-level context. (And I imagine other teams could have a similar issue? Just to reiterate: I think a block styled Link is a reasonable expectation / usage for that component 😅 )

[emyarod]

@jendowns the suggestion to use the wrapper div was only to make it a block level element, not for any additional styling (since the bug was reported when the display: block is set on the links). the link itself would still be an inline element and the styles would remain unchanged. I imagined it would be a workaround on the consumer's end not on the library side

so rather than

<!-- index.html -->
<a href="/" class="bx--link">Example link</a>

/* style.css */
.bx--link {
  display: block;
}

doing something like:

<div>
  <a href="/" class="bx--link">Example link</a>
</div>

/* style.css */
// no css changes

unless I am misunderstanding the issue, but I hope that can help address the problem

[jendowns]

Thanks -- Yes I'm familiar with this approach & the box model. It is what we've already suggested as a work-around for our product team. My comment above was specifically in regards to wrapping things here in the library, which seems like overkill to me (looks like we agree). And my point being: wrapping or using text-decoration are the one ways I'm aware of to get around this specific issue.

Just to add --
You aren't misunderstanding the issue, but I think you are misunderstanding the intent behind it. That is: I believe it is reasonable for a developer to have a block styled Link, and this library no longer supports that. Can that be changed?

[shixiedesign]

@jendowns Let me bring it up in our design crit and see if the look of underline is what we would want to go with. Thanks for pointing out the issue!

[jendowns]

Thanks! Yeah happy to talk about this more if you need. I realize you all don't necessarily encourage style overrides to begin with, but this just seems like such a common thing? 😅

[emyarod]

yeah I'm not sure we can make Carbon links block level without reexamining the hover/active/focus styles, which is why I am leaning towards this being something addressed on the consumer's side for now. by default links are inline elements so I think that was our focus when styling Carbon links

open to any other ideas or suggestions on how to support block level links in Carbon but yeah if we change the design to open up text-decoration usage then that would be simpler

[jendowns]

Yeah totally understandable -- just thought I'd ask to make sure. 👍

[SimonFinney]

Thanks @jendowns for raising this! I appreciate the discussion.

I understand the reasoning behind the choices here, and maybe where this issue was raised on our side is a 1-in-a-1000 edge case and this may not come up again, but it seems counter-intuitive to me that if someone wanted to make a link block-level it would need to be wrapped in a div, especially for any existing consumers.

Another thing to note here about using box-shadow as a visual indicator, and we get called out on this quite a lot, is that for AVT cases where color is removed from the browser, the box-shadow for focus states is no longer displayed -

[emyarod]

yes good points and agreed. it's not ideal, just a possible workaround while the component is reexamined. links have gone through quite a few refactors in the last several months as I stated, so I'm hopeful the next iteration will resolve these issues

[SimonFinney]

@emyarod That sounds good to me! Thanks for the insight

[shixiedesign]

Things to think about: hover & focus state will be affected.

[shixiedesign]

After discussing with group, we agree that it's worth sacrificing some design wishes here. Let's go with text-decoration: underline; ! FYI @jendowns @emyarod

[jendowns]

Thanks @shixiedesign! I'm happy to create a PR for that 🎉

Just wanted to double-check on something --
So you'd like text-decoration: underline for hover, focus, and active?

[shixiedesign]

@jendowns spec below! Would like to have 1px border all around for focus if possible:

[jendowns]

Thanks @shixiedesign! Do you mind if we use an outline for that :focus state? Like outline: 1px solid $link-01?

[shixiedesign]

@jendowns Of course! I'm just pretending I know CSS so border / outline whatever works 😂

[alisonjoseph]

Just jumping in to comment on the cool stuff that's coming soon in css!! So in the future, we can get the thickness/offset back in all browsers. 😉

Here is a codepen Jen Simmons made showing what works today. https://codepen.io/jensimmons/pen/wLrjGG?editors=1100

https://drafts.csswg.org/css-text-decor-3/
https://drafts.csswg.org/css-text-decor-4/

[shixiedesign]

@jendowns Hey just realize one thing about focus state you are mentioning above: outline is totally cool but we need the focus outline color to use $focus token instead of $link-01 because focus outline in Dark themes is actually white.

In case you didn't see my comment here: https://github.com/carbon-design-system/carbon/pull/3225#pullrequestreview-255901068