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 a11y form component ids #2599

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5fcc7b0
fix: a11y issue for all form components
nmerget Apr 30, 2024
3514dd4
Merge branch 'main' of github.com:db-ui/mono into fix-a11y-form-compo…
nmerget Apr 30, 2024
51c14bf
fix: a11y issue for switch
nmerget Apr 30, 2024
dabcf94
fix: issue from PR
nmerget May 8, 2024
2b04630
Merge branch 'main' into fix-a11y-form-component-ids
nmerget May 8, 2024
28d00c9
fix: issue with vue
nmerget May 8, 2024
fa45de4
Merge remote-tracking branch 'origin/fix-a11y-form-component-ids' int…
nmerget May 8, 2024
55ddd0d
Merge branch 'main' into fix-a11y-form-component-ids
mfranzke May 10, 2024
91b2e60
Update packages/components/src/components/select/select.lite.tsx
nmerget May 10, 2024
ded12f9
Update checkbox.lite.tsx
mfranzke May 10, 2024
286eae3
Update input.lite.tsx
mfranzke May 10, 2024
524e892
Update textarea.lite.tsx
mfranzke May 10, 2024
4ad533c
Update input.lite.tsx
mfranzke May 10, 2024
613e971
fix: issues from PR
nmerget May 10, 2024
2f92ef4
Merge remote-tracking branch 'origin/fix-a11y-form-component-ids' int…
nmerget May 10, 2024
ad99509
Merge branch 'main' into fix-a11y-form-component-ids
nmerget May 10, 2024
99da69a
Merge branch 'main' into fix-a11y-form-component-ids
nmerget May 10, 2024
315e2e0
fix: issues with form-components
nmerget May 10, 2024
ea3abae
fix: removed unused dependency
nmerget May 10, 2024
3fd2d65
feat: add new tests for form-components for react- and vue-showcase
nmerget May 13, 2024
809fd76
Merge branch 'main' of github.com:db-ui/mono into fix-a11y-form-compo…
nmerget May 13, 2024
ce2d6a1
feat: add new tests for form-components for angular-showcase
nmerget May 13, 2024
54529cc
Merge branch 'main' into fix-a11y-form-component-ids
nmerget May 13, 2024
8749d59
Merge branch 'main' into fix-a11y-form-component-ids
nmerget May 13, 2024
70f4b76
fix: issue from PR
nmerget May 14, 2024
bb1987e
chore: update from main
nmerget May 14, 2024
309091f
Merge branch 'main' of github.com:db-ui/mono into fix-a11y-form-compo…
nmerget May 15, 2024
072bb2b
fix: issue with angular
nmerget May 15, 2024
a641c27
Merge branch 'main' into fix-a11y-form-component-ids
nmerget May 21, 2024
484ed3a
Merge branch 'main' into fix-a11y-form-component-ids
mfranzke May 30, 2024
dff6ca5
Merge branch 'main' of github.com:db-ui/mono into fix-a11y-form-compo…
nmerget Jun 26, 2024
09f1f23
chore: update from main
nmerget Jun 26, 2024
c823486
fix: issue with vue showcase
nmerget Jun 26, 2024
91f7074
Merge branch 'main' of github.com:db-ui/mono into fix-a11y-form-compo…
nmerget Jun 27, 2024
6abdad7
fix: issue with vue refs
nmerget Jun 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 28 additions & 28 deletions packages/components/src/components/checkbox/checkbox.lite.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
import { DBCheckboxProps, DBCheckboxState } from './model';
import { cls, uuid } from '../../utils';
import {
DEFAULT_ID,
DEFAULT_INVALID_MESSAGE,
DEFAULT_INVALID_MESSAGE_ID_SUFFIX,
DEFAULT_MESSAGE_ID_SUFFIX,
Expand All @@ -29,12 +28,11 @@ export default function DBCheckbox(props: DBCheckboxProps) {
// jscpd:ignore-start
const state = useStore<DBCheckboxState>({
initialized: false,
_id: DEFAULT_ID,
_messageId: DEFAULT_ID + DEFAULT_MESSAGE_ID_SUFFIX,
_validMessageId: DEFAULT_ID + DEFAULT_VALID_MESSAGE_ID_SUFFIX,
_invalidMessageId: DEFAULT_ID + DEFAULT_INVALID_MESSAGE_ID_SUFFIX,
_descByIds: '',

_id: 'checkbox-' + uuid(),
_messageId: this._id + DEFAULT_MESSAGE_ID_SUFFIX,
_validMessageId: this._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX,
_invalidMessageId: this._id + DEFAULT_INVALID_MESSAGE_ID_SUFFIX,
_descByIds: `${this._messageId}`,
mfranzke marked this conversation as resolved.
Show resolved Hide resolved
handleChange: (event: ChangeEvent<HTMLInputElement>) => {
if (props.onChange) {
props.onChange(event);
Expand All @@ -44,6 +42,19 @@ export default function DBCheckbox(props: DBCheckboxProps) {
props.change(event);
}
handleFrameworkEvent(this, event, 'checked');

if (!ref?.validity.valid || props.customValidity === 'invalid') {
state._descByIds = state._invalidMessageId;
} else if (
props.customValidity === 'valid' ||
(ref?.validity.valid && props.required)
) {
state._descByIds = state._validMessageId;
} else if (props.message) {
state._descByIds = state._messageId;
} else {
state._descByIds = '';
}
nmerget marked this conversation as resolved.
Show resolved Hide resolved
},
handleBlur: (event: InteractionEvent<HTMLInputElement>) => {
if (props.onBlur) {
Expand Down Expand Up @@ -77,33 +88,21 @@ export default function DBCheckbox(props: DBCheckboxProps) {

onMount(() => {
state.initialized = true;
state._id = props.id || 'checkbox-' + uuid();
state._id = props.id || state._id;
});

onUpdate(() => {
if (state.initialized && state._id) {
state._messageId = state._id + DEFAULT_MESSAGE_ID_SUFFIX;
state._validMessageId = state._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX;
state._invalidMessageId =
if (state._id) {
const messageId = state._id + DEFAULT_MESSAGE_ID_SUFFIX;
const validMessageId = state._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX;
const invalidMessageId =
Comment on lines +87 to +88
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two values could be assigned directly to the states. Do you do this for reasons of consistency?

state._id + DEFAULT_INVALID_MESSAGE_ID_SUFFIX;
state._messageId = messageId;
state._validMessageId = validMessageId;
state._invalidMessageId = invalidMessageId;
}
}, [state._id, state.initialized]);

onUpdate(() => {
const descByIds = [state._validMessageId, state._invalidMessageId];
if (props.message) {
descByIds.push(state._messageId);
}
state._descByIds = descByIds.join(' ');
}, [
props.message,
state._messageId,
state._validMessageId,
state._invalidMessageId
]);
// jscpd:ignore-end
}, [state._id]);

// TODO we have to check how to update on every change..
onUpdate(() => {
if (state.initialized && document && state._id) {
const checkboxElement = document?.getElementById(
Expand All @@ -123,6 +122,7 @@ export default function DBCheckbox(props: DBCheckboxProps) {
}
}
}, [state.initialized, props.indeterminate, props.checked]);
// jscpd:ignore-end

return (
<div
Expand Down
56 changes: 31 additions & 25 deletions packages/components/src/components/input/input.lite.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
import { cls, uuid } from '../../utils';
import { DBInputProps, DBInputState } from './model';
import {
DEFAULT_ID,
DEFAULT_INVALID_MESSAGE,
DEFAULT_INVALID_MESSAGE_ID_SUFFIX,
DEFAULT_LABEL,
Expand All @@ -35,12 +34,12 @@ export default function DBInput(props: DBInputProps) {
const ref = useRef<HTMLInputElement>(null);
// jscpd:ignore-start
const state = useStore<DBInputState>({
_id: DEFAULT_ID,
_messageId: DEFAULT_ID + DEFAULT_MESSAGE_ID_SUFFIX,
_validMessageId: DEFAULT_ID + DEFAULT_VALID_MESSAGE_ID_SUFFIX,
_invalidMessageId: DEFAULT_ID + DEFAULT_INVALID_MESSAGE_ID_SUFFIX,
_descByIds: '',
_dataListId: DEFAULT_ID,
_id: 'input-' + uuid(),
_messageId: this._id + DEFAULT_MESSAGE_ID_SUFFIX,
_validMessageId: this._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX,
_invalidMessageId: this._id + DEFAULT_INVALID_MESSAGE_ID_SUFFIX,
_dataListId: `datalist-` + uuid(),
nmerget marked this conversation as resolved.
Show resolved Hide resolved
_descByIds: `${this._messageId}`,
mfranzke marked this conversation as resolved.
Show resolved Hide resolved
defaultValues: {
label: DEFAULT_LABEL,
placeholder: ' '
Expand All @@ -53,6 +52,23 @@ export default function DBInput(props: DBInputProps) {
if (props.input) {
props.input(event);
}

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 = '';
}
nmerget marked this conversation as resolved.
Show resolved Hide resolved
},
handleChange: (event: ChangeEvent<HTMLInputElement>) => {
if (props.onChange) {
Expand Down Expand Up @@ -96,32 +112,22 @@ export default function DBInput(props: DBInputProps) {
});

onMount(() => {
state._id = props.id || 'input-' + uuid();
state._dataListId = props.dataListId || `datalist-${uuid()}`;
state._id = props.id ?? state._id;
state._dataListId = props.dataListId ?? state._dataListId;
});

onUpdate(() => {
if (state._id) {
state._messageId = state._id + DEFAULT_MESSAGE_ID_SUFFIX;
state._validMessageId = state._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX;
state._invalidMessageId =
const messageId = state._id + DEFAULT_MESSAGE_ID_SUFFIX;
const validMessageId = state._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX;
const invalidMessageId =
Comment on lines +114 to +115
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two values could be assigned directly to the states. Do you do this for reasons of consistency?

state._id + DEFAULT_INVALID_MESSAGE_ID_SUFFIX;
state._messageId = messageId;
state._validMessageId = validMessageId;
state._invalidMessageId = invalidMessageId;
}
}, [state._id]);

onUpdate(() => {
const descByIds = [state._validMessageId, state._invalidMessageId];
if (props.message) {
descByIds.push(state._messageId);
}
state._descByIds = descByIds.join(' ');
}, [
props.message,
state._messageId,
state._validMessageId,
state._invalidMessageId
]);

return (
<div
class={cls('db-input', props.className)}
Expand Down
5 changes: 2 additions & 3 deletions packages/components/src/components/radio/radio.lite.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
useStore
} from '@builder.io/mitosis';
import { DBRadioProps, DBRadioState } from './model';
import { DEFAULT_ID } from '../../shared/constants';
import { cls, uuid } from '../../utils';
import { ChangeEvent, InteractionEvent } from '../../shared/model';
import { handleFrameworkEvent } from '../../utils/form-components';
Expand All @@ -21,7 +20,7 @@ export default function DBRadio(props: DBRadioProps) {
// jscpd:ignore-start
const state = useStore<DBRadioState>({
initialized: false,
_id: DEFAULT_ID,
_id: 'radio-' + uuid(),
handleChange: (event: ChangeEvent<HTMLInputElement>) => {
if (props.onChange) {
props.onChange(event);
Expand Down Expand Up @@ -55,7 +54,7 @@ export default function DBRadio(props: DBRadioProps) {

onMount(() => {
state.initialized = true;
state._id = props.id || 'radio-' + uuid();
state._id = props.id ?? state._id;
});
// jscpd:ignore-end

Expand Down
59 changes: 31 additions & 28 deletions packages/components/src/components/select/select.lite.tsx
nmerget marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
import { DBSelectOptionType, DBSelectProps, DBSelectState } from './model';
import { cls, uuid } from '../../utils';
import {
DEFAULT_ID,
DEFAULT_INVALID_MESSAGE,
DEFAULT_INVALID_MESSAGE_ID_SUFFIX,
DEFAULT_LABEL,
Expand All @@ -31,12 +30,12 @@ export default function DBSelect(props: DBSelectProps) {
const ref = useRef<HTMLSelectElement>(null);
// jscpd:ignore-start
const state = useStore<DBSelectState>({
_id: DEFAULT_ID,
_messageId: DEFAULT_ID + DEFAULT_MESSAGE_ID_SUFFIX,
_validMessageId: DEFAULT_ID + DEFAULT_VALID_MESSAGE_ID_SUFFIX,
_invalidMessageId: DEFAULT_ID + DEFAULT_INVALID_MESSAGE_ID_SUFFIX,
_descByIds: '',
_placeholderId: DEFAULT_ID + DEFAULT_PLACEHOLDER_ID_SUFFIX,
_id: 'select-' + uuid(),
_messageId: this._id + DEFAULT_MESSAGE_ID_SUFFIX,
_validMessageId: this._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX,
_invalidMessageId: this._id + DEFAULT_INVALID_MESSAGE_ID_SUFFIX,
_placeholderId: this._id + DEFAULT_PLACEHOLDER_ID_SUFFIX,
_descByIds: this._placeholderId + ' ' + this._messageId,
handleClick: (event: ClickEvent<HTMLSelectElement>) => {
if (props.onClick) {
props.onClick(event);
Expand All @@ -52,6 +51,21 @@ export default function DBSelect(props: DBSelectProps) {
}

handleFrameworkEvent(this, event);

if (!ref?.validity.valid || props.customValidity === 'invalid') {
state._descByIds =
this._placeholderId + ' ' + state._invalidMessageId;
} else if (
props.customValidity === 'valid' ||
(ref?.validity.valid && props.required)
) {
state._descByIds =
this._placeholderId + ' ' + state._validMessageId;
} else if (props.message) {
state._descByIds = this._placeholderId + ' ' + state._messageId;
} else {
state._descByIds = this._placeholderId;
}
nmerget marked this conversation as resolved.
Show resolved Hide resolved
},
handleBlur: (event: InteractionEvent<HTMLSelectElement>) => {
if (props.onBlur) {
Expand Down Expand Up @@ -87,32 +101,23 @@ export default function DBSelect(props: DBSelectProps) {
});

onMount(() => {
state._id = props.id || 'select-' + uuid();
state._id = props.id || state._id;
});

onUpdate(() => {
if (state._id) {
state._placeholderId = state._id + DEFAULT_PLACEHOLDER_ID_SUFFIX;
state._messageId = state._id + DEFAULT_MESSAGE_ID_SUFFIX;
state._validMessageId = state._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX;
state._invalidMessageId =
const messageId = state._id + DEFAULT_MESSAGE_ID_SUFFIX;
const validMessageId = state._id + DEFAULT_VALID_MESSAGE_ID_SUFFIX;
const invalidMessageId =
state._id + DEFAULT_INVALID_MESSAGE_ID_SUFFIX;
const placeholderId = state._id + DEFAULT_PLACEHOLDER_ID_SUFFIX;
state._messageId = messageId;
state._validMessageId = validMessageId;
state._invalidMessageId = invalidMessageId;
state._invalidMessageId = placeholderId;
nmerget marked this conversation as resolved.
Show resolved Hide resolved
}
}, [state._id]);

onUpdate(() => {
const descByIds = [state._validMessageId, state._invalidMessageId];
if (props.message) {
descByIds.push(state._messageId);
}
state._descByIds = descByIds.join(' ');
}, [
props.message,
state._messageId,
state._validMessageId,
state._invalidMessageId
]);

return (
<div
class={cls('db-select', props.className)}
Expand Down Expand Up @@ -141,9 +146,7 @@ export default function DBSelect(props: DBSelectProps) {
onFocus={(event: InteractionEvent<HTMLSelectElement>) =>
state.handleFocus(event)
}
aria-describedby={
(props.message && state._messageId) || state._placeholderId
}>
aria-describedby={state._descByIds}>
{/* Empty option for floating label */}
<option hidden></option>
<Show when={props.options}>
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/components/switch/switch.lite.tsx
nmerget marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default function DBSwitch(props: DBSwitchProps) {
const ref = useRef<HTMLInputElement>(null);
// jscpd:ignore-start
const state = useStore<DBSwitchState>({
_id: DEFAULT_ID,
_id: 'switch-' + uuid(),
initialized: false,
handleChange: (event: ChangeEvent<HTMLInputElement>) => {
if (props.onChange) {
Expand Down Expand Up @@ -47,7 +47,7 @@ export default function DBSwitch(props: DBSwitchProps) {
});

onMount(() => {
state._id = props.id || 'switch-' + uuid();
state._id = props.id || state._id;
});
// jscpd:ignore-end

Expand Down
Loading
Loading