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

chore(inputs): update aria labels and description #782

Closed
wants to merge 18 commits into from

Conversation

hebernardEquisoft
Copy link
Contributor

@hebernardEquisoft hebernardEquisoft commented Mar 25, 2024

DS-924

Description

Ici on vient mettre à jour le processus de définition des labels et descriptions pour les composantes d'inputs.
J'ai tenté de respecter le plus possible les contraintes dans le ticket.

Vraiment pas certain de celle-là!

Tests fonctionnels

  • Valider le bon fonctionnement de chacun des inputs affectés

@hebernardEquisoft hebernardEquisoft requested a review from a team as a code owner March 25, 2024 15:34
.map(({ id }) => id!)
.join(' ') || undefined;

export const useAriaLabels = ({
Copy link
Contributor

@savutsang savutsang Mar 27, 2024

Choose a reason for hiding this comment

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

Le nom n'est pas bon, ca suggere un usage generalise, mais il n'est fait que pour etre utilise avec un control qui est dans un FieldContainer (ex: hint et invalid ids qui sont hardcode dedans).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ouais, je suis à la recherche d'une suggestion concrète mais à première vue, je pense qu'on devrait gérer les labels et les descriptions séparément.

Il pourrait y avoir une interface pour les props liés aux labels et une autre spécifique aux props destinées au FieldContainer.

* Aria label for the input (used when no visual label is present)
*/
ariaLabel?: string;
interface ComboboxProps extends AriaLabelsProps {
Copy link
Contributor

@savutsang savutsang Mar 27, 2024

Choose a reason for hiding this comment

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

AriaLabelsProps ajoute des props dans le components qu'on ne veut pas (ex: hasHint, inputId, isValid).

@@ -259,6 +259,8 @@ input + .c2 {
aria-controls="uuid1_listbox"
aria-expanded="true"
aria-invalid="false"
aria-label="Select an option"
aria-labelledby="uuid1_label"
Copy link
Contributor

Choose a reason for hiding this comment

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

Ici c'est contradictoire, on peut pas avoir les 2 attributs en même temps

if (label && !ariaLabel) {
processedAriaLabel = processedLabel;
} else if (ariaLabel && !label) {
processedLabel = processedAriaLabelledBy;
Copy link
Contributor

@savutsang savutsang Mar 28, 2024

Choose a reason for hiding this comment

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

Pourquoi? C'est pourtant un cas valide d'avoir juste aria-label et que le label reste undefined. Genre on ne veut pas afficher de label et qu'on n'a pas de id pour le labelledBy, dans ce cas on lui fourni explicitement un aria-label.

fieldId={id}
label={label}
fieldId={fieldId}
label={processedLabels.label}
Copy link
Contributor

@savutsang savutsang Mar 28, 2024

Choose a reason for hiding this comment

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

Suite a l'autre commentaire, c'est un probleme que le hook fait en sorte que ariaLabel pourrait aboutir ici meme lorsque le prop label est undefined. Alors que le comportement expected du prop "label" c'est que si le dev ne met rien, c'est qu'il ne veut pas afficher de Label du tout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants