-
Notifications
You must be signed in to change notification settings - Fork 77
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(input, combobox, input-date-picker, input-number, input-text, input-time-picker, radio-button-group, segmented-control, select, text-area): provide clear field error messaging for AT #9880
Changes from all commits
679a94b
9e5fba3
1931acb
ac8ab23
643091d
d750dc1
121561d
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 |
---|---|---|
|
@@ -92,7 +92,7 @@ import { syncHiddenFormInput } from "../input/common/input"; | |
import { isBrowser } from "../../utils/browser"; | ||
import { normalizeToCurrentCentury, isTwoDigitYear } from "./utils"; | ||
import { InputDatePickerMessages } from "./assets/input-date-picker/t9n"; | ||
import { CSS } from "./resources"; | ||
import { CSS, IDS } from "./resources"; | ||
|
||
@Component({ | ||
tag: "calcite-input-date-picker", | ||
|
@@ -568,8 +568,10 @@ export class InputDatePicker | |
aria-autocomplete="none" | ||
aria-controls={this.dialogId} | ||
aria-describedby={this.placeholderTextId} | ||
aria-errormessage={IDS.validationMessage} | ||
aria-expanded={toAriaBoolean(this.open)} | ||
aria-haspopup="dialog" | ||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
class={{ | ||
[CSS.input]: true, | ||
[CSS.inputNoBottomBorder]: this.layout === "vertical" && this.range, | ||
|
@@ -661,8 +663,10 @@ export class InputDatePicker | |
<calcite-input-text | ||
aria-autocomplete="none" | ||
aria-controls={this.dialogId} | ||
aria-errormessage={IDS.validationMessage} | ||
aria-expanded={toAriaBoolean(this.open)} | ||
aria-haspopup="dialog" | ||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
class={{ | ||
[CSS.input]: true, | ||
[CSS.inputBorderTopColorOne]: this.layout === "vertical" && this.range, | ||
|
@@ -689,6 +693,7 @@ export class InputDatePicker | |
{this.validationMessage && this.status === "invalid" ? ( | ||
<Validation | ||
icon={this.validationIcon} | ||
id={IDS.validationMessage} | ||
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. Since IDs are scoped within their respective shadow DOM, one option worth exploring (separately) to simplify this pattern would be:
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. @benelan Do you think we should add coverage for this in If so, we can tackle this separately. 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. |
||
message={this.validationMessage} | ||
scale={this.scale} | ||
status={this.status} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,7 +75,7 @@ import { | |
TextualInputComponent, | ||
} from "../input/common/input"; | ||
import { IconNameOrString } from "../icon/interfaces"; | ||
import { CSS, SLOTS } from "./resources"; | ||
import { CSS, IDS, SLOTS } from "./resources"; | ||
import { InputNumberMessages } from "./assets/input-number/t9n"; | ||
|
||
/** | ||
|
@@ -1081,6 +1081,8 @@ export class InputNumber | |
|
||
const childEl = ( | ||
<input | ||
aria-errormessage={IDS.validationMessage} | ||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
aria-label={getLabelText(this)} | ||
autocomplete={this.autocomplete} | ||
autofocus={this.el.autofocus ? true : null} | ||
|
@@ -1132,6 +1134,7 @@ export class InputNumber | |
{this.validationMessage && this.status === "invalid" ? ( | ||
<Validation | ||
icon={this.validationIcon} | ||
id={IDS.validationMessage} | ||
message={this.validationMessage} | ||
scale={this.scale} | ||
status={this.status} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,7 @@ import { getIconScale } from "../../utils/component"; | |
import { Validation } from "../functional/Validation"; | ||
import { syncHiddenFormInput, TextualInputComponent } from "../input/common/input"; | ||
import { IconNameOrString } from "../icon/interfaces"; | ||
import { CSS, SLOTS } from "./resources"; | ||
import { CSS, IDS, SLOTS } from "./resources"; | ||
import { InputTextMessages } from "./assets/input-text/t9n"; | ||
|
||
/** | ||
|
@@ -663,6 +663,8 @@ export class InputText | |
|
||
const childEl = ( | ||
<input | ||
aria-errormessage={IDS.validationMessage} | ||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
aria-label={getLabelText(this)} | ||
autocomplete={this.autocomplete} | ||
autofocus={this.el.autofocus ? true : null} | ||
|
@@ -712,6 +714,7 @@ export class InputText | |
{this.validationMessage && this.status === "invalid" ? ( | ||
<Validation | ||
icon={this.validationIcon} | ||
id={IDS.validationMessage} | ||
message={this.validationMessage} | ||
scale={this.scale} | ||
status={this.status} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,7 +82,7 @@ import { Validation } from "../functional/Validation"; | |
import { focusFirstTabbable } from "../../utils/dom"; | ||
import { IconNameOrString } from "../icon/interfaces"; | ||
import { syncHiddenFormInput } from "../input/common/input"; | ||
import { CSS } from "./resources"; | ||
import { CSS, IDS } from "./resources"; | ||
import { InputTimePickerMessages } from "./assets/input-time-picker/t9n"; | ||
|
||
// some bundlers (e.g., Webpack) need dynamic import paths to be static | ||
|
@@ -1025,7 +1025,9 @@ export class InputTimePicker | |
<div class="input-wrapper" onClick={this.onInputWrapperClick}> | ||
<calcite-input-text | ||
aria-autocomplete="none" | ||
aria-errormessage={IDS.validationMessage} | ||
aria-haspopup="dialog" | ||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
disabled={disabled} | ||
icon="clock" | ||
id={this.referenceElementId} | ||
|
@@ -1071,6 +1073,7 @@ export class InputTimePicker | |
{this.validationMessage && this.status === "invalid" ? ( | ||
<Validation | ||
icon={this.validationIcon} | ||
id={IDS.validationMessage} | ||
message={this.validationMessage} | ||
scale={this.scale} | ||
status={this.status} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
export const CSS = { | ||
toggleIcon: "toggle-icon", | ||
}; | ||
|
||
export const IDS = { | ||
validationMessage: "inputTimePickerValidationMessage", | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,7 +71,7 @@ import { Validation } from "../functional/Validation"; | |
import { IconNameOrString } from "../icon/interfaces"; | ||
import { InputMessages } from "./assets/input/t9n"; | ||
import { InputPlacement, NumberNudgeDirection, SetValueOrigin } from "./interfaces"; | ||
import { CSS, INPUT_TYPE_ICONS, SLOTS } from "./resources"; | ||
import { CSS, IDS, INPUT_TYPE_ICONS, SLOTS } from "./resources"; | ||
import { NumericInputComponent, syncHiddenFormInput, TextualInputComponent } from "./common/input"; | ||
|
||
/** | ||
|
@@ -1172,6 +1172,8 @@ export class Input | |
this.type === "number" ? ( | ||
<input | ||
accept={this.accept} | ||
aria-errormessage={IDS.validationMessage} | ||
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. @geospatialem Do you know which input needs the corresponding ARIA attributes? The one users interact with is in shadow DOM and the one used for form submitting/validation is in light DOM (created by 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. @jcfranco Unfortunately can't confirm with my local environment at this time, but testing via the Chromatic build is performing as-expected providing context to the message. In the past have we tried to stick to the light DOM? |
||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
aria-label={getLabelText(this)} | ||
autocomplete={this.autocomplete} | ||
autofocus={autofocus} | ||
|
@@ -1203,6 +1205,8 @@ export class Input | |
? [ | ||
<this.childElType | ||
accept={this.accept} | ||
aria-errormessage={IDS.validationMessage} | ||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
aria-label={getLabelText(this)} | ||
autocomplete={this.autocomplete} | ||
autofocus={autofocus} | ||
|
@@ -1276,6 +1280,7 @@ export class Input | |
{this.validationMessage && this.status === "invalid" ? ( | ||
<Validation | ||
icon={this.validationIcon} | ||
id={IDS.validationMessage} | ||
message={this.validationMessage} | ||
scale={this.scale} | ||
status={this.status} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ import { | |
} from "../../utils/loadable"; | ||
import { Validation } from "../functional/Validation"; | ||
import { IconNameOrString } from "../icon/interfaces"; | ||
import { CSS } from "./resources"; | ||
import { CSS, IDS } from "./resources"; | ||
|
||
/** | ||
* @slot - A slot for adding `calcite-radio-button`s. | ||
|
@@ -207,12 +207,17 @@ export class RadioButtonGroup implements LoadableComponent { | |
render(): VNode { | ||
return ( | ||
<Host role="radiogroup"> | ||
<div class={CSS.itemWrapper}> | ||
<div | ||
aria-errormessage={IDS.validationMessage} | ||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
class={CSS.itemWrapper} | ||
> | ||
<slot /> | ||
</div> | ||
{this.validationMessage && this.status === "invalid" ? ( | ||
<Validation | ||
icon={this.validationIcon} | ||
id={IDS.validationMessage} | ||
message={this.validationMessage} | ||
scale={this.scale} | ||
status={this.status} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
export const CSS = { | ||
itemWrapper: "item-wrapper", | ||
}; | ||
|
||
export const IDS = { | ||
validationMessage: "radioButtonGroupValidationMessage", | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
export const CSS = { | ||
itemWrapper: "item-wrapper", | ||
}; | ||
|
||
export const IDS = { | ||
validationMessage: "segmentedControlValidationMessage", | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ import { createObserver } from "../../utils/observers"; | |
import { Validation } from "../functional/Validation"; | ||
import { IconNameOrString } from "../icon/interfaces"; | ||
import { isBrowser } from "../../utils/browser"; | ||
import { CSS } from "./resources"; | ||
import { CSS, IDS } from "./resources"; | ||
|
||
/** | ||
* @slot - A slot for adding `calcite-segmented-control-item`s. | ||
|
@@ -202,7 +202,11 @@ export class SegmentedControl | |
render(): VNode { | ||
return ( | ||
<Host onClick={this.handleClick} role="radiogroup"> | ||
<div class={CSS.itemWrapper}> | ||
<div | ||
aria-errormessage={IDS.validationMessage} | ||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
class={CSS.itemWrapper} | ||
> | ||
<InteractiveContainer disabled={this.disabled}> | ||
<slot /> | ||
<HiddenFormInputSlot component={this} /> | ||
|
@@ -211,6 +215,7 @@ export class SegmentedControl | |
{this.validationMessage && this.status === "invalid" ? ( | ||
<Validation | ||
icon={this.validationIcon} | ||
id={IDS.validationMessage} | ||
message={this.validationMessage} | ||
scale={this.scale} | ||
status={this.status} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ import { Scale, Status, Width } from "../interfaces"; | |
import { getIconScale } from "../../utils/component"; | ||
import { Validation } from "../functional/Validation"; | ||
import { IconNameOrString } from "../icon/interfaces"; | ||
import { CSS } from "./resources"; | ||
import { CSS, IDS } from "./resources"; | ||
|
||
type OptionOrGroup = HTMLCalciteOptionElement | HTMLCalciteOptionGroupElement; | ||
type NativeOptionOrGroup = HTMLOptionElement | HTMLOptGroupElement; | ||
|
@@ -421,6 +421,8 @@ export class Select | |
<InteractiveContainer disabled={disabled}> | ||
<div class={CSS.wrapper}> | ||
<select | ||
aria-errormessage={IDS.validationMessage} | ||
aria-invalid={this.status === "invalid"} | ||
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. same here |
||
aria-label={getLabelText(this)} | ||
class={CSS.select} | ||
disabled={disabled} | ||
|
@@ -435,6 +437,7 @@ export class Select | |
{this.validationMessage && this.status === "invalid" ? ( | ||
<Validation | ||
icon={this.validationIcon} | ||
id={IDS.validationMessage} | ||
message={this.validationMessage} | ||
scale={this.scale} | ||
status={this.status} | ||
|
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.
Late to the party but I think this should use the
toAriaBoolean
function to convert this to a string of "false" or "true".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.
Excellent catch! Can you create an issue for me to set up an ESLint rule for this?
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.
Never mind, we already have one. I'll bring it up during our next triage session.