fix(button): check if !disabled before rendering as an anchor if href is provided

[jendowns]

Closes #4479

Proposal to check if !disabled before rendering a Button as an anchor if href prop is provided. That way, you can ensure that you don't have a situation where a button-like anchor with disabled={true} can still be clicked and used to navigate.

Changelog

New

• {{new thing}}

Changed

• check !disabled before rendering a Button as an anchor if href is provided

Testing / Reviewing

Open the carbon-components-react deployment for this PR and then toggle the disabled state in the knobs while inspecting the second button in the examples ("Link"). Then try to interact with it while it is disabled={true}

Then compare to the current storybook: http://react.carbondesignsystem.com/iframe.html?id=buttons--default

[asudoh]

Hi 👋 thank you for jumping in! As I stated in the original issue, I'd prefer CSS-only solution.

[jendowns]

Thanks @asudoh!

My one hesitation with a css-only solution is accessibility 🤔
Since in this situation the component renders as an anchor, pointer-events: none would not give an indication to a screen reader that the element is truly non-interactive.

In VoiceOver, even if I add pointer-events: none to this element, I can still tab to it & VO still announces it as a link.

Whereas if I render it as a button element with a disabled state, it is ignored by VO.

(@dakahn what do you think?)

[netlify]

Deploy preview for the-carbon-components ready!

Built with commit 7fced60

https://deploy-preview-4480--the-carbon-components.netlify.com

[asudoh]

@jendowns What happens if we add aria-disabled to the <a>, too?

[netlify]

Deploy preview for carbon-elements ready!

Built with commit 7fced60

https://deploy-preview-4480--carbon-elements.netlify.com

[netlify]

Deploy preview for carbon-components-react ready!

Built with commit 7fced60

https://deploy-preview-4480--carbon-components-react.netlify.com

[jendowns]

Hey @asudoh, adding aria-disabled={true} makes VO read this as "Link, dimmed, button" (with "Link" of course representing the text of the element)

Good things about this approach

With aria-disabled="true", VoiceOver appears to correctly identify it as "dimmed".

With pointer-events="none" style rule, the element cannot be interacted with at all, so it can't be clicked.

So combining the 2 makes it so the anchor cannot be interacted with, and makes it so that VO reads it as "dimmed" (though note that you can still tab to the element, unlike a disabled state button)

Concerns about this approach

Even the Link component does not take this approach for a disabled state: http://react.carbondesignsystem.com/?path=/story/link--default

My concern about making an anchor non-interactive is that it subverts the purpose of the anchor... if an anchor isn't supposed to be clicked (and is, for example, disabled state) then it should be simple text, as is done for the Link component in this library.

The only reason this approach ends up working is due to pointer-events: none doing the heavy lifting -- aria-disabled does not actually modify the purpose of the link (nor would a disabled attribute), so it requires the pointer-events workaround to appear to make the link non-interactive.

You can still tab to this anchor though.
And now, with pointer-events: none, you get no visual feedback with cursor: not-allowed 🤔

I don't really care for this approach, as it seems to be getting around some rules and ignoring the purpose of a link. That's why, originally, I wanted to render it as a button element outright -- because you can set a button to a disabled state that makes it non-interactive without any workarounds. And it wouldn't go against the way the Link component is treated in disabled state, either.

I can be convinced otherwise, but I'm really hesitant to combine an aria with a css rule. I'm really curious what @dakahn thinks about this 🤔

[abbeyhrt]

This solution seems ideal to me and reads and works well with VoiceOver and JAWS, @asudoh what is the reason behind your preference for a CSS-only solution?

[dakahn]

Disabled links are bad. They subvert a core interaction on the web in a way that users of all kinds can find confusing and unexpected. I like Jen's proposed solution because it speaks well and communicates clearly in a screen reader and avoids aria relying instead on using the right elements in the right place. 👍

[asudoh]

OK fair enough, thank you for taking a look and explaining your thoughts @jendowns!