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(ffe-context-message-react): translations of aria labels #1818

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

pethel
Copy link
Contributor

@pethel pethel commented Mar 4, 2024

Updagede dette da jag implmenterte ikonerna. aria labels blir ikke oversett til riktig locale.

Sedan lurer jag på om locale forsatt skall vare optional?

@pethel pethel requested a review from a team as a code owner March 4, 2024 11:37
messageType="error"
aria-label="Feilmelding"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Disse har jag fjernet konsekvent i komponenterna for dom er ikke i bruk. En alert skall vel heller kanske ikke ha en aria label?

Copy link
Contributor

Choose a reason for hiding this comment

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

Det er jeg ikke sikker på stemmer? Vi har UU testet disse komponentene og da funket det bra å ha label på gruppe/alert nivået. Så per nå er jeg litt uenig i at aria-labelene settes på ikonene istedenfor. Her kan man kanskje høre med Sindre om best practice

Copy link
Contributor

Choose a reason for hiding this comment

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

Kort svar:
Det kommer an på konteksten man implementerer elementet i. Om ikonet står for seg selv vil man måtte legge til informasjon i koden, men om det er supplerende skriftlig informasjon sammen med ikonet kan man like gjerne klassifisere ikonet som dekorativt.
Å gi alle brukere jevnverdig god informasjon på en brukervennlig måte er det viktigste.

Langt svar:
Aria-label skal som første regel kun benyttes når det ikke er mulig å bruke HTML-attributter for å gi ekstra kontekstuell informasjon. Så å bruke aria-label i stedet for HTML-attributter for disse elementene vil da være riktig.

Når det kommer til uu-hensyn kommer det i dette tilfellet an på kontekst for elementet.

Om ikonet presenteres med skriftlig beskjed om hva som har skjedd (som for eksempel "Betalingen ble registrert!") kan man argumentere for at ikonet er supplerende informasjon og derfor ikke trenger å bli plukket opp av skjermlesere i det hele tatt.

Om den tiltenkte konteksten er å benytte elementet for en alert er det viktig at dette gjøres korrekt. Om alerten krever å raskt kunne informere brukeren vil det være viktig å sørge for at dette ivaretas for alle brukere, om nødvendig med ARIA: alert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Når jag googlet så sa jag flere exempler med aria label på group men ikke direkt på alert. Jag tolkat koden slik att aria lablen aldrig ble brukt nedøver men ser nå den ligger på {...rest}. Har tagit tilbake den nå iaf. Hensikten var ikke or ersette med ikonet. Ikonet synes jag dermot er øverflødig ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Du har helt rett i at label kan brukes på gruppert informasjon :)

Som nevnt er det 100% avhengig av konteksten innholdet settes i, også for om ikonet skal ha label eller ikke

@pethel pethel force-pushed the develop_fix-locales-context-error-message branch from 19b6642 to ef7dea3 Compare March 4, 2024 11:40
Copy link

github-actions bot commented Mar 4, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-1818.westeurope.2.azurestaticapps.net

1 similar comment
Copy link

github-actions bot commented Mar 4, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-1818.westeurope.2.azurestaticapps.net

messageType="success"
aria-label="Suksessmelding"
Copy link
Contributor

Choose a reason for hiding this comment

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

Her er jeg ikke sikker på om jeg er enig at aria-label skal fjernes. Fordi når den ligger på group så leses den opp hele boksen istedenfor at brukerne må navigere seg inn til ikonet.

Copy link
Contributor Author

@pethel pethel Mar 5, 2024

Choose a reason for hiding this comment

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

Så ikke noen exempel på alert og aria label. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/alert_role

Er ikke risken med en aria-label på alerten att den gir blaffen i texten i selva alreten og bruker selva aria labeln som innehold? Alltså att selva beskjeden som den som bruker komponenten kommer med ikke blir lest up utan kunden får kun høra "Feilmedlning"?

Har ikke testet selv. Enig i aria-label gir mening på "role=group"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Snakket med Sindre og båda blir lest op så da er dette greit.

Copy link
Contributor Author

@pethel pethel Mar 5, 2024

Choose a reason for hiding this comment

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

Her er vad jag var redd for iaf
https://www.aditus.io/aria/aria-label/

Knappen får ikke "send email send". Alltså send i knappen blir spist op, men det kanskej avhenger av elementer/context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vi har blitt eniga om att beholde aria labels også på role alert

@pethel pethel force-pushed the develop_fix-locales-context-error-message branch from ef7dea3 to 532f90c Compare March 5, 2024 11:02
Copy link

github-actions bot commented Mar 5, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-1818.westeurope.2.azurestaticapps.net

@pethel pethel force-pushed the develop_fix-locales-context-error-message branch from 532f90c to e34c130 Compare March 5, 2024 11:18
Copy link

github-actions bot commented Mar 5, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-1818.westeurope.2.azurestaticapps.net

@pethel pethel force-pushed the develop_fix-locales-context-error-message branch from e34c130 to 6899495 Compare March 5, 2024 14:22
Copy link

github-actions bot commented Mar 5, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-1818.westeurope.2.azurestaticapps.net

@pethel pethel force-pushed the develop_fix-locales-context-error-message branch from 6899495 to b19e6be Compare March 6, 2024 09:16
Copy link

github-actions bot commented Mar 6, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-1818.westeurope.2.azurestaticapps.net

@pethel pethel merged commit fa54b9d into develop Mar 6, 2024
4 checks passed
@pethel pethel deleted the develop_fix-locales-context-error-message branch March 6, 2024 09:21
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