Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Button): replace disabled for a11y attribute aria-disabled #950

Merged
merged 4 commits into from
Jul 23, 2024

Conversation

LarryMatte
Copy link
Contributor

https://equisoft.atlassian.net/browse/DS-743

Remplacer l'attribut disabled pour l'attribut d'accessibilité aria-disabled="true".

@LarryMatte LarryMatte requested a review from a team as a code owner July 22, 2024 17:58
Copy link

Storybook for this build: https://ds.equisoft.io/pr-950/

Copy link

Webapp for this build: https://ds.equisoft.io/pr-950/webapp/

focusable={focusable}
onClick={onClick}
onClick={disabled ? undefined : onClick}
Copy link
Contributor

@savutsang savutsang Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ca me semble beau tout ca.

Sauf que maintenant, meme s'il n'y a plus de click, il va quand meme avoir le propagation du click (le disabled normal lui le block). Je pense on devrait bloquer cette propagation pour pas changer ce comportement, sinon ca risque d'affecter certains apps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Si je fais ca comme ça est-ce que c'est good pour toi?

    const handleClick = (event: MouseEvent<HTMLButtonElement>) => {
        if (disabled) {
            event.preventDefault();
            event.stopPropagation();
        } else if (onClick) {
            onClick(event);
        }
    };

    const handleKeyDown = (event: KeyboardEvent<HTMLButtonElement>) => {
        if (disabled) {
            event.preventDefault();
            event.stopPropagation();
        } else if (onKeyDown) {
            onKeyDown(event);
        }
    };

<StyledButton
            ...
            aria-disabled={disabled ? 'true' : undefined}
            ...
        >

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui c'est good de meme.

Copy link
Contributor

@pylafleur pylafleur Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pas certain de comprendre quel est le gain de remplacer l'utilisation du disabled natif si on finit par reproduire le même comportement avec du code de notre côté?

Edit: Ah, est-ce que c'est pour qu'il puisse recevoir le focus?

Copy link
Contributor Author

@LarryMatte LarryMatte Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quand un bouton est disabled nativement, il n'est pas focusable avec le clavier, ce qui pose des problèmes pour les utilisateurs de technologies d'assistance comme les lecteurs d'écran. Si un bouton est inaccessible via le clavier, ces utilisateurs ne pourront pas savoir qu'il existe sur la page, même s'il est seulement désactivé.

Exemple d'un formulaire. Si, pour une raison quelconque (champ required qui n'a pas été remplis ou il y a des erreurs dans la page) et que ces issues doivent être fixées avant que le bouton submit soit enabled et que le bouton submit est disabled nativement, l'utilisateur qui navigue avec le clavier pourrait ne jamais le trouver. Cela peut entraîner de la confusion pour les users car ils vont naviguer un formulaire sans submit buttons (selon leur perception de la chose).

En utilisant l'attribut aria-disabled, le bouton reste focusable et les lecteurs d'écran l'annoncent comme "dimmed". Ainsi, les utilisateurs de technologies d'assistance sont informés de sa présence et de son état désactivé.

@LarryMatte LarryMatte merged commit b724b3f into master Jul 23, 2024
23 checks passed
@LarryMatte LarryMatte deleted the dev/DS-743 branch July 23, 2024 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants