-
Notifications
You must be signed in to change notification settings - Fork 83
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,11 @@ | ||
import React from 'react'; | ||
import texts from './locale/texts'; | ||
import ContextMessage from './ContextMessage'; | ||
import { ContextMessageIcon } from './ContextMessageIcon'; | ||
import { oneOf } from 'prop-types'; | ||
import acceptedLocales from './locale/accepted-locales'; | ||
|
||
const ContextSuccessMessage = props => { | ||
const ContextSuccessMessage = ({ locale, ...rest }) => { | ||
const checkIconSmall = | ||
'data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIGhlaWdodD0iMjAiIHZpZXdCb3g9IjAgLTk2MCA5NjAgOTYwIiB3aWR0aD0iMjAiPjxwYXRoIGQ9Im0zOTUtMzcyLjM4NCAyNzAuNTM5LTI2OS41MzhxNy45MjMtNy45MjMgMTcuNjkyLTcuNjE2IDkuNzY5LjMwOCAxOC4wNzYgOC42MTYgOC4zMDggOC4zMDcgOC4zMDggMTcuODg0IDAgOS41NzYtOC4zMDggMTcuODg0bC0yODMgMjgyLjk5OXEtOS44NDYgOS44NDYtMjIuODA3IDkuODQ2LTEyLjk2MSAwLTIyLjgwNy05Ljg0NmwtMTE0LTExMy45OTlxLTcuOTIzLTcuOTIzLTguMzA4LTE3LjY5Mi0uMzg0LTkuNzY5IDcuOTIzLTE4LjA3NiA4LjMwOC04LjMwOCAxNy44ODQtOC4zMDggOS41NzcgMCAxNy44ODQgOC4zMDhMMzk1LTM3Mi4zODRaIi8+PC9zdmc+'; | ||
|
||
|
@@ -11,19 +14,28 @@ const ContextSuccessMessage = props => { | |
|
||
return ( | ||
<ContextMessage | ||
{...props} | ||
{...rest} | ||
locale={locale} | ||
messageType="success" | ||
aria-label="Suksessmelding" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Her er vad jag var redd for iaf Knappen får ikke "send email send". Alltså send i knappen blir spist op, men det kanskej avhenger av elementer/context There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
role="group" | ||
aria-label={texts[locale].success.ariaLabel} | ||
icon={ | ||
<ContextMessageIcon | ||
smallIconUrl={checkIconSmall} | ||
largeIconUrl={checkIconLarge} | ||
ariaLabel="hake" | ||
/> | ||
} | ||
/> | ||
); | ||
}; | ||
|
||
ContextSuccessMessage.defaultProps = { | ||
locale: 'nb', | ||
}; | ||
|
||
ContextSuccessMessage.propTypes = { | ||
/** Decides the language */ | ||
locale: oneOf(acceptedLocales), | ||
}; | ||
|
||
export default ContextSuccessMessage; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,47 @@ | ||
const nb = {}; | ||
const nn = {}; | ||
const en = {}; | ||
|
||
nb.FFE_CONTEXT_MESSAGE_CLOSE = 'Lukk'; | ||
nn.FFE_CONTEXT_MESSAGE_CLOSE = 'Lukk'; | ||
en.FFE_CONTEXT_MESSAGE_CLOSE = 'Close'; | ||
const nb = { | ||
close: 'Lukk', | ||
error: { | ||
ariaLabel: 'Feilmelding', | ||
}, | ||
info: { | ||
ariaLabel: 'Infomelding', | ||
}, | ||
success: { | ||
ariaLabel: 'Suksessmelding', | ||
}, | ||
tip: { | ||
ariaLabel: 'Tipsmelding', | ||
}, | ||
}; | ||
const nn = { | ||
close: 'Lukk', | ||
error: { | ||
ariaLabel: 'Feilmelding', | ||
}, | ||
info: { | ||
ariaLabel: 'Infomelding', | ||
}, | ||
success: { | ||
ariaLabel: 'Suksessmelding', | ||
}, | ||
tip: { | ||
ariaLabel: 'Tipsmelding', | ||
}, | ||
}; | ||
const en = { | ||
close: 'Close', | ||
error: { | ||
ariaLabel: 'Error message', | ||
}, | ||
info: { | ||
ariaLabel: 'Info message', | ||
}, | ||
success: { | ||
ariaLabel: 'Success message', | ||
}, | ||
tip: { | ||
ariaLabel: 'Tip message', | ||
}, | ||
}; | ||
|
||
export default { nb, nn, en }; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ;)
There was a problem hiding this comment.
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