feat(ui5-datepicker): implement ACC support #763
Conversation
packages/main/src/Input.js
Outdated
| } | ||
|
|
||
| get roleAttribute() { | ||
| if (this._accInfo && this._accInfo.roleAttribute) { |
There was a problem hiding this comment.
Should be fine to return undefined, try
return this._accInfo && this._accInfo.roleAttribute;
Same for ariaOwns ariaExpanded
packages/main/src/DatePicker.hbs
Outdated
| ></ui5-calendar> | ||
| </ui5-popover> | ||
|
|
||
| <span id="{{_id}}-date" class="ui5-hidden-text">{{dateAriaDescriber}}</span> |
There was a problem hiding this comment.
Describer => Description
packages/main/src/DatePicker.js
Outdated
| "ariaDescribedBy": `${this._id}-date`, | ||
| "ariaHasPopup": "true", | ||
| "ariaAutoComplete": "none", | ||
| "roleAttribute": "combobox", |
There was a problem hiding this comment.
IMO just "role", they are all attributes
| #XACT: ARIA announcement for the emphasized button | ||
| BUTTON_ARIA_TYPE_EMPHASIZED=Emphasized | ||
|
|
||
| #XACT: Aria information for the Date Picker |
There was a problem hiding this comment.
This is for the translation team, they don't really know what this means, so simply put a text such as "Date" IMO
| BUTTON_ARIA_TYPE_EMPHASIZED=Emphasized | ||
|
|
||
| #XACT: Aria information for the Date Picker | ||
| DATEPICKER_DATE_TYPE=Date |
There was a problem hiding this comment.
DATE_TYPE sounds inaccurate, maybe DATEPICKER_DATE_ACC_TEXT or something
| "ariaAutoComplete": "none", | ||
| "roleAttribute": "combobox", | ||
| "ariaOwns": `${this._id}-popover`, | ||
| "ariaExpanded": this.isOpen(), |
There was a problem hiding this comment.
What happens when the value of this.isOpen() is changed? Does something invalidate so that the component can be invalidated?
There was a problem hiding this comment.
When the picker gets opened or closed this. _isPickerOpen is mutated, which invalidates the ui5-datepicker, and this.isOpen() will return either true, or false when calculated for the HBS template upon rendering. (If that was the question)
ilhan007
changed the title
packages/main/src/Input.js
Outdated
| } | ||
|
|
||
| get ariaHasPopup() { | ||
| if (this._accInfo && this._accInfo.ariaHasPopup) { |
There was a problem hiding this comment.
You can do one check less, as undefined is fine:
if (this._accInfo) {
return this._accInfo.ariaHasPopup;
}
packages/main/src/Input.js
Outdated
| } | ||
|
|
||
| get ariaAutoComplete() { | ||
| if (this._accInfo && this._accInfo.ariaAutoComplete) { |
There was a problem hiding this comment.
Same as above, you can do one check less, as undefined is fine:
if (this._accInfo) {
return this._accInfo. ariaAutoComplete;
}
packages/main/src/Input.js
Outdated
| return { | ||
| "wrapperAccInfo": { | ||
| }, | ||
| "inputAccInfo": { |
There was a problem hiding this comment.
AccInfo is already in the name of the main object, so "input" and "wrapper" are enough here
accInfo.input.role f.e.
packages/main/src/Input.js
Outdated
| "inputAccInfo": { | ||
| "ariaDescribedBy": this._inputAccInfo ? `${this.suggestionsTextId} ${this.valueStateTextId} ${this._inputAccInfo.ariaDescribedBy}`.trim() : `${this.suggestionsTextId} ${this.valueStateTextId}`.trim(), | ||
| "ariaInvalid": this.valueState === ValueState.Error ? "true" : undefined, | ||
| "ariaHasPopup": this._inputAccInfo ? this._inputAccInfo.ariaHasPopup : () => { return this.showSuggestions ? "true" : undefined; }, |
There was a problem hiding this comment.
IMO better to extract this as a const above
same for the other such lines, seems a bit too complex to read
packages/main/src/DatePicker.hbs
Outdated
| ?disabled="{{disabled}}" | ||
| ?readonly="{{readonly}}" | ||
| value-state="{{valueState}}" | ||
| aria-autocomplete="none" |
There was a problem hiding this comment.
remove the aria stuff from the ui5-input
packages/main/src/DatePicker.hbs
Outdated
| ></ui5-calendar> | ||
| </ui5-popover> | ||
|
|
||
| <span id="{{_id}}-date" class="ui5-hidden-text">{{dateAriaDescription}}</span> |
There was a problem hiding this comment.
this is not being read out when it is outside the native input DOM. We should add it in the same shadow root as the native input element
Hi team, can you please review the PR.
Best Regards,
Tsani