Fix a11y form component ids #2599
Conversation
|
🔭🐙🐈 Test this branch here: https://db-ui.github.io/mono/review/fix-a11y-form-component-ids |
…o fix-a11y-form-component-ids
…nent-ids # Conflicts: # showcases/vue-showcase/src/utils/navigation-items.ts
| { | ||
| from: 'props.value ?? _value', | ||
| to: 'props.value' |
There was a problem hiding this comment.
Can you perhaps also add an explanation here?
| { | ||
| from: /this.\$refs.ref\?.validationMessage/g, | ||
| to: 'this?.$refs.ref?.validationMessage' |
There was a problem hiding this comment.
Can you perhaps also add an explanation here?
| // This is a workaround for valid/invalidMessages resetting values | ||
| [ | ||
| 'HTMLSelectElement', | ||
| 'HTMLInputElement', | ||
| 'HTMLTextAreaElement' | ||
| ].forEach((element) => { | ||
| replacements.push({ | ||
| from: `handleInput(event: InputEvent<${element}>) {`, | ||
| to: | ||
| `handleInput(event: InputEvent<${element}>) {\n` + | ||
| 'this._value = (event.target as any).value;' | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Without further explanation, I don't think it's clear what's happening here.
| const validMessageId = state._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX; | ||
| const invalidMessageId = |
There was a problem hiding this comment.
These two values could be assigned directly to the states. Do you do this for reasons of consistency?
| const validMessageId = state._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX; | ||
| const invalidMessageId = |
There was a problem hiding this comment.
These two values could be assigned directly to the states. Do you do this for reasons of consistency?
| if (!ref?.validity.valid || props.customValidity === 'invalid') { | ||
| state._descByIds = state._invalidMessageId; | ||
| } else if ( | ||
| props.customValidity === 'valid' || | ||
| (ref?.validity.valid && | ||
| (props.required || | ||
| props.minLength || | ||
| props.maxLength || | ||
| props.pattern)) | ||
| ) { | ||
| state._descByIds = state._validMessageId; | ||
| } else if (props.message) { | ||
| state._descByIds = state._messageId; | ||
| } else { | ||
| state._descByIds = 'none'; | ||
| } |
There was a problem hiding this comment.
Perhaps it would be useful to briefly describe the benefits of this block...
| ...Array.from<Element>( | ||
| ref.querySelectorAll('& > .db-tab-panel') | ||
| ), | ||
| ...Array.from<Element>( | ||
| ref.querySelectorAll('& > dbtabpanel > .db-tab-panel') | ||
| ), | ||
| ...Array.from<Element>( | ||
| ref.querySelectorAll('& > db-tab-panel > .db-tab-panel') | ||
| ) |
There was a problem hiding this comment.
Do you select the tab panels for the various frameworks here (automatic wrapping of the components)? If so, two querySelectorAll would be unnecessary at runtime - right?
| ); | ||
| } | ||
| tabPanels.forEach((panel: Element, index: number) => { | ||
| if (!panel.id) { |
There was a problem hiding this comment.
if (panel.id) return;
...
A guard clause would avoid indentation and possibly increase readability.
| </DBTabList> | ||
| </db-tab-item> | ||
| <db-tab-item icon="airplane" [noText]="true"></db-tab-item> | ||
| </db-tab-list> | ||
| <DBTabPanel> |
There was a problem hiding this comment.
Also write in kebap-case?
| </DBTabPanel> | ||
| <DBTabPanel>Tab Panel 5</DBTabPanel> |
There was a problem hiding this comment.
Also write in kebap-case?
Proposed changes
closes #2588
aria-describedbyand messagesTypes of changes
Further comments