Enkele a11y aanpassingen om de links leesbaar te houden en focusable te maken. #1
Conversation
…<button> element. Also disabling the disabled theming.
…ual HTML Checkbox.
|
Ik heb nog nooit typescript geschreven, dus wees gerust kritisch :) |
| @@ -109,7 +109,28 @@ export const CheckListIconButton = ({ | |||
| }} | |||
| disabled={isDisabled} | |||
| > | |||
| <ButtonIcon iconButtonType={iconButtonType} /> | |||
| <CheckListItemDescription | |||
There was a problem hiding this comment.
@mathieuspil De checklistitem description in de iconbutton stoppen zou ik niet doen, anders het component zijn doel voorbij (een buttonicon voor een checklist). Je zou een nieuwe component kunnen maken (bijv. CheckListItemContent, wat dan een checklistem prop neemt en de description + knopje aan de rechterkant rendert). Klopt het dat het knopje aan de rechterkant dus niet meer dient als een knop, maar eerder als een herkenbaar symbool?
There was a problem hiding this comment.
checklistitem description in de iconbutton stoppen zou ik niet doen
=> Ik bekijk het zo: de button heeft nu een descriptive tekst. De <button> is nu alles aan de rechterkant van de checklistitem.
Het principe van het knopje aan de rechterkant bestaat niet meer: alles is nu een beter accessible knop (met hover en focus mogelijkheden)
Qua benaming van componenten binnenin React heb ik absoluut geen voorkeur, maar ik had niet het gevoel dat de structuur een nieuwe laag nodig had omdat ik enkel tekst verplaatst heb?
laat maar weten hoe hard ik de bal misslaan heb :)
| checked={isDisabled} | ||
| /> | ||
|
|
||
| <div |
There was a problem hiding this comment.
Hier zou ik een styled component van maken
There was a problem hiding this comment.
Ik denk dat ik weet wat je wilt zeggen :)
Bedoel je op deze manier dan: mathieuspil@05416d4 ?
| @@ -33,13 +35,15 @@ export const CheckListSideBarButton = ({ | |||
| iconButtonType, | |||
| value, | |||
| disabled = false, | |||
| checkListItem, | |||
| }: ICheckListSideBarButtonProp) => { | |||
| const { openSideBar } = useEduSideBar() | |||
| return ( | |||
| <CheckListIconButton | |||
| onClick={() => openSideBar(eduSidebarType, value)} | |||
There was a problem hiding this comment.
ChecklistItems met een description (wat een modal triggert), opent nu tegelijkertijd een sidebar en een modal.
There was a problem hiding this comment.
Aha, dus er was een use-case waarbij dat die tekst ook geklikt kon worden! Dat wist ik niet.
Hoe lang kan die description worden? Want hoe ik het met de ux-pet zie: Als je klikt op een tekst dan zou hetgeen moeten openen dat de tekst beschrijft, niet een modal met meer info.
Zou je suggesties aanvaarden die de description niet langer in een modal toont?
Hoe lang kan deze description worden? Is dat altijd maar 1 zinnetje? En op welke soort checklistitems is een description mogelijk?
Kan je het achterliggend idee van een description bij een checklistitem even schetsen?
There was a problem hiding this comment.
@timdpaep Dit is nu aangepast en dit triggert niet langer een Modal, maar de tekst wordt nu zichtbaar
There was a problem hiding this comment.
Aha, dus er was een use-case waarbij dat die tekst ook geklikt kon worden! Dat wist ik niet.
Hoe lang kan die description worden? Want hoe ik het met de ux-pet zie: Als je klikt op een tekst dan zou hetgeen moeten openen dat de tekst beschrijft, niet een modal met meer info.Zou je suggesties aanvaarden die de description niet langer in een modal toont?
Hoe lang kan deze description worden? Is dat altijd maar 1 zinnetje? En op welke soort checklistitems is een description mogelijk?
Kan je het achterliggend idee van een description bij een checklistitem even schetsen?
Achterliggend idee is dat je soms wat extra uitleg moet geven bij een checklist item (bijv. dat er voor "het maken van oefening X" er eerst "node modules" moeten worden geïnstalleerd, ik zeg maar iets)
No description provided.