Link: Focus outline prone to clipping; also poor visibility when wrapping images

[BrendanMcK]

Environment Information

• Package version(s): 6.96.0
• Browser and OS versions: any

Please provide a reproduction of the bug in a codepen:

Can repro both issues with this codepen

Actual/Expected behavior:

There's two issue with the current focus outline styles here -they're closely related, so reporting as a single issue - unlikely that they can be fixed independently of each other.

• First issue is that the current focus outline style used by Fabric uses border and outline, both of which are outset from the original element. This means that if the element is flush with the edges of its parent (or ancestor) container, and that container has overflow: hidden|auto set, the focus outline gets clipped. The main case where we're hitting this is when we're using overflow along with text-overflow:ellipsis to get ellipsis-style truncation.

Some examples, using current Fabric outline:

• in the examples above, can see that sometimes only one or two edges of the focus outline are visible.

We've ended up using inset 1px box-shadow as our focus outline to avoid this clipping. It works great for us, but can be fragile in some cases: if a link contains DIV or other block-layout elements, we need to ensure that the link also has block-layout, otherwise the box-shadow won't appear. We can manage this in our own UI, but it's not clear that it's a robust solution for Fabric to adopt.

Worth noting that the default browser outline also has this clipping issue: we were originally steering clear of using a specific focus outline style at all, and sticking with a 'browser knows best' approach - but clipping of the browser outline in Chrome/Safari and Firefox (both of which got called out during a11y reviews) forced us to explore alternatives, so we ended up going with a box-shadow-based approach (with additional style rules for Windows High Contrast mode).

• Second issue is that the current focus outline isn't particularly visible when a link contains dar or very colorful images. In the codepen above, you can see the image receive focus - but it looks more as though it's just growing in size - the outline blends into the image. Likely this outline is also prone to the clipping issue described above - if the image was contained tightly in some scrollable container (say an image in a thumbnail picker), it might be even less visible.

The approach we've taken here is to add an option to our custom Link class to opt-in to a more visible focus outline as needed. This more visible outline also uses box-shadow, with multiple alternating shadows so that the resulting outline is a fully inset (to avoid clipping) white/black/white outline, guaranteeing good visibility even in confined layouts and with noisy images.

[dzearing]

@aneeshack4 Let's see what we can do here. We can sync on the area and I can help you :)

[dzearing]

Hey @BrendanMcK I see 2 problems here;

1. The link you're truncating is not stretching to the container. This makes it unpredictable to render a border. It needs to be display: block or inline-block, so that maxWidth can be specified.

2. The parent is clipping the link border, which is rendered outside of its "owned" layout space. The only fixes I see are:

a. Render the border on top of the content (this seems ugly because it covers some of it up.) b. Link itself adds inner padding to compensate for the focus rect (which seems bad because it can result in alignment problem.) c. Just adding padding to the container where it makes sense. I think this is the best approach.

I've updated your example with options 1 and "c", here:

https://codepen.io/micahgodbolt/pen/EGNqYm (new codepen with Stack)

Which renders the focus rectangle correctly. Thoughts?

[BrendanMcK]

Just adding padding

Back when I was initially looking at the focus outline clipping issue, I did briefly explore the option of adding padding to avoid this; but our CSS back then was sufficiently complex that I quickly realized it was not a viable approach: it's often not an immediate parent that's doing the clipping, but some higher-up ancestor (or, often, multiple ancestors). And adding padding somewhere also means adjusting some other property to keep the layout otherwise as-is, so there's all sorts of knock-on affects. It just didn't seem to be at all a robust or maintainable approach. While box-shadow has it's drawbacks (it's essentially a variation of option (a)), it's considerably more manageable.

It might be interesting to revisit this now that we're more Fabric based: what would this approach actually mean in practice? Would we need to add an addPadding or mayContainFocusableElements or similar property to some layout components? Would fixing outline clipping on a link be a relatively localized, or require a bunch of tweaks to many components?

[BrendanMcK]

At a first glance, adding padding might be more viable than it was for us in the past:

• Previously, we had a lot of components that used overflow: auto or similar to 'fit-to-content'. This was the main issue that ended up causing clipping to happen at multiple ancestors in the tree, making adding padding non-trivial. However, most of these have now been replaced by flexbox or similar, which doesn't have this side-effect of clipping content.

• We've also mostly centralized our CSS that provides ellipsis to a couple of components. These still use overflow: hidden to trigger ellipsis; but they could also add say padding: 1px along with margin: -1px to add padding without affecting layout.

So this might provide an approach for dealing with outline rect clipping; will investigate more over the next few weeks and report back. IIRC, there were some other cases where we had link clipping - mostly lists of items in scrollable containers, which also have the side-effect of clipping.

--

Seems like we'll still need an option for higher-visibility focus on an as-needed basis. Any thoughts on that issue? At least that option is much easier to address via an option on Link.

[dzearing]

I'd seriously worry about adding padding. Users will not anticipate it and you end up with weird side effects.

First padding doesn't work on inline. Margin does, but then... same unpredictability issue.

Other than the overlapping border, I don't see a way to avoid clipping.

The only fix I see is overlapping focus rectangles. Which is a little sad. And if we want to do the white/black 2px treatment we have on buttons, it would be 2px of inset overlap. Which makes it worse.

Codepen to illustrate the 2 approaches:

https://codepen.io/dzearing/pen/ZmewZB

[dzearing]

Talking offline in a thread on the right approach here;

[dzearing]

The outcome here was:

For Link, render focus within the rendered boundary of the link.

To do this the rect must use something that wraps. Outline renders outside of the boundary. Borders or inset box-shadow would work.

Neither of those work with high contrast. So, for high contrast media queries we can fall back to outline, but it means we can't solve the clipping issue in that case.

[BrendanMcK]

Neither of those work with high contrast

I don't understand this bit - why don't outline / borders / inset box-shadow work in HC mode? Can't you just re-state the style in terms of Window/WindowText within a HC media query?

[aneeshack4]

@BrendanMcK the border & box-shadow disappear in high contrast mode so I believe in HC mode only, we'll have to switch to outline. David & I spoke about this early last week - the approach he suggested is to not use OS HC support & instead to rely on theming to solve HC. I will try the HC media query approach to restate the style in terms of Window/WindowText though. My plate was full til now with another big PR but I finished that so I'm starting to work on this now. I'm new to all of this so I'm still digging into the code trying to understand the context - might reach out to you with questions as David's oof atm.

[BrendanMcK]

the border & box-shadow disappear in high contrast mode so I believe in HC mode only, we'll have to switch to outline.

Restating the styles within a HC media selector (using the HC theme colors) is exactly the way to address this.

Having clipping in HC mode would be continued and accessibility bug in its own right. HC users want good focus outlines too!

to not use OS HC support & instead to rely on theming to solve HC

My take is that this is non-trivial: the problem is that HC mode's default processing essentially ends up ignoring all of your theming styles. The only way to get this to work, as I understand it, would be to use the -ms-high-contrast-adjust: none style somewhere near the root to turn off the default HC processing so that your theme colors show through instead. But then you are 100% on the hook for all HC work; and this may also cause issues for other hosted HTML fragments (say custom renderers) that are relying on the default HC processing to do the HC stuff for them. It's quite an all-or-nothing approach.

[dzearing]

If it is as simple as restating the border to use an hc color in the hc query, then cool! We could solve the clipping in hc then too.

[aneeshack4]

@BrendanMcK I'm trying to repro this locally so I can test the fix against it but I don't know how to do this on the Fabric site - do you know how I can repro this clipping on https://developer.microsoft.com/en-us/fabric#/components/link? I've also been trying to use the Codepen you provided but nothing is rendered on there - any idea why?

[BrendanMcK]

It seems that VerticalStack is no longer available (or has moved), so the codepen is no longer working.

While box-shadow appears to work in this case, I'm not sure it's the right approach for Fabric to take, because, as I mentioned in the original post above, it's somewhat fragile.

The problem with focus outlines is that there's no one-size-fits-all solution. I think Fabric will have to decide which default makes the most sense for its use cases, and then provide options to use the other approaches. The default that we've chosen in Yammer makes sense for us (we have our own a11y QA process, so can find and fix broken focus issues), but may not be the right fit for Fabric.

Here's a summary of the different approaches, and their pros and cons. Note that any approach can be made work in HC mode, so that's not a factor in these decisions.

• outline

  • Pro: No impact on layout
  • Pro: Just works on wrapped inline elements and block elements
  • Con: Tends to be outset, so prone to clipping where text truncation is used
  • Con: Not very visible around tightly-wrapped images

• border

  • Pro: Just works on wrapped inline elements and block elements
  • Con: affects layout
  • Con: usually outset, so has same clipping issue

• inset box-shadow

  • Pro: No impact on layout
  • Pro: Works great for text links; inset means it avoids clipping
  • Con: fragile when containing block content; parent must also be made explicitly display: block
  • Con: disappears under child content - eg tightly-wrapped images

• Floating ::after element

  • Pro: minimal impact on layout (though parent needs to have position set)
  • Pro: can be made appear on top of children, so best approach when element contains tightly-wrapped content
  • Con: doesn't work with wrapped inline elements

With clickable content (whether hyperlink or button), it seems we have at least three different use-cases that have different - and in some cases, non-overlapping - requirements:

• Plain text. This may wrap, and/or may be truncated. The box-shadow inset approach works well here, but breaks in other scenarios.

• Tightly-wrapped images: eg. an IMG or other content that fully covers the surface of the parent A/BUTTON element. A Floating ::after element works well here. Typically needs higher visibility than a single box; we use a white/black/white to ensure good visibility against the contained content

• Loosely-wrapped block content: This is pretty much a catch-all for anything else - where a hyperlink/button contains mixed content that's not completely covering the surface, and where the hyperlink/button isn't itself clipped by parent. Regular outline works just fine here.

We've gone with the box-shadow approach as our default because we have a lot of text links that can wrap or get truncated with ellipsis. But doing this means we have to keep an eye out for places where the link contains block content, and add a block=true option there to avoid the outline disappearing. We also need to find any tightly-wrapped images and use another option there to get a higher-visibility focus outline.

Maybe the best fix for Fabric is to instead provide a focusOutline=inset|strong|default option on Link?

[aneeshack4]

Thank you @BrendanMcK for providing this detailed breakdown - really appreciate it! Just wanted to update you - I'm working on trying out the focusOutline=inset|strong|default option on Link. If it works, I'll send out a PR.

[aneeshack4]

Another update, discussed this a bit with @micahgodbolt: we're still trying to figure out what the right solution is for Fabric. Right now, I'm considering the different contexts Link's used in because defining focusOutline=inset|strong|default would mean allowing a top level prop to directly control CSS styles which we're not sure is ok for scenarios that Link's used differently in.

[BrendanMcK]

we're still trying to figure out what the right solution is for Fabric

Great; this is really more of a DCR or similar rather than a simple bug-fix, so needs plenty of due diligence. Once we add an option to our API, we'll be stuck with it for some time.

[msft-github-bot]

:tada:This issue was addressed in #7793, which has now been successfully released as office-ui-fabric-react@v6.135.0.:tada:

Handy links:

• Release Notes
• Official NPM Release
• Test the release with Developer Demo Site