canonical / react-components

A set of components based on Vanilla Framework
https://canonical.github.io/react-components
82 stars 49 forks source link

feat: allow focusing disabled buttons and displaying tooltips #1082

Closed petermakowski closed 2 months ago

petermakowski commented 2 months ago

Done

Rationale

Previously, disabled attribute was used which prevents focusing the button entirely. This meant that users navigating by keyboard could not focus the button and any associated tooltip would not be displayed.

By switching to aria-disabled, the button can still be focused while conveying its disabled state to assistive technologies. Submit action is prevented by using .preventDefault.

https://css-tricks.com/making-disabled-buttons-more-inclusive/

QA

Storybook

To see rendered examples of all react-components, run:

yarn start

QA in your project

from react-components run:

yarn build
npm pack

Install the resulting tarball in your project with:

yarn add <path-to-tarball>

QA steps

Percy steps

Fixes

Fixes: https://warthogs.atlassian.net/browse/MAASENG-2122

Screenshots

After

Google Chrome screenshot 001879@2x

webteam-app commented 2 months ago

Demo

Jenkins

demos.haus

bartaz commented 2 months ago

We had some discussions around that in the past, with no exact conclusion from UX part what's the best approach: https://github.com/canonical/vanilla-framework/issues/4299 https://github.com/canonical/vanilla-framework/issues/4942

But seems like having tooltips on disabled buttons is something that we want, so this may be a way for implementing this.

Is there anything here that would be worth upstreaming to Vanilla? I guess there are no CSS changes needed, but I guess we could have a similar example and functionality documented there.

petermakowski commented 2 months ago

@bartaz I don't think there is anything major that would need to be upstreamed to Vanilla as it already handles disabled state as expected without using :disabled through .is-disabled class.

We might want to update the Vanilla guidelines and suggest not using :disabled but aria-disabled with .is-disabled instead.

We could also allow applying disabled through aria-disabled selector as an alternative.

None of these changes are required for this PR to land, it stands on its own as is IMO.

github-actions[bot] commented 2 months ago

:tada: This PR is included in version 0.53.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: