feat(ui5-ai-button): improve acc attributes #11929
Conversation
| const ariaLabel = `${this._leftButtonTooltipText} ${(attrLabel?.concat(labelRefTexts) || "")}`.trim(); | ||
|
|
||
| return { | ||
| hasPopup: hasPopup ?? (this._hideArrowButton ? "false" : "menu"), |
There was a problem hiding this comment.
hasPopup || "false" to use the provided by the application dev value directly.
| getAssociatedLabelForTexts(this) || | ||
| ""; | ||
|
|
||
| const ariaLabel = `${this._leftButtonTooltipText} ${(attrLabel?.concat(labelRefTexts) || "")}`.trim(); |
There was a problem hiding this comment.
${this._leftButtonTooltipText} ${labelRefTexts}.trim();
|
|
||
| get _computedAccessibilityAttributes(): AIButtonAccesibilityAttributes { | ||
| const { | ||
| ariaLabel: attrLabel, |
There was a problem hiding this comment.
Not sure we need to provide ariaLabel in the accessibility attributes of the button for now. The tooltip text is already made to be part of the aira-label attribute value.
| @@ -28,31 +31,35 @@ export default function SplitButtonTemplate(this: SplitButton) { | |||
| onMouseUp={this._textButtonRelease} | |||
| onFocusIn={this._onInnerButtonFocusIn} | |||
| onFocusOut={this._onFocusOut} | |||
| title={this.accessibilityAttributes.ariaLabel} | |||
| tooltip={this.accessibilityAttributes.ariaLabel} | |||
There was a problem hiding this comment.
Only tooltip attribute is needed. There is no title property.
Something like tooltip={this.accInfo.root.title}. We need to adjust the accInfo method so that root.title is present.
| @@ -9,6 +9,9 @@ export default function SplitButtonTemplate(this: SplitButton) { | |||
| class="ui5-split-button-root" | |||
| tabindex={this._tabIndex} | |||
| aria-labelledby={`${this._id}-invisibleTextDefault ${this._id}-invisibleText`} | |||
| aria-haspopup={this.accessibilityAttributes.hasPopup} | |||
There was a problem hiding this comment.
it will be good to use the accInfo method here as well.
aria-haspopup={this.accInfo.root.hasPopup}
aria-label={this.accInfo.root.ariaLabel}
aria-label={this.accInfo.root.roleDescription}
| @@ -35,6 +36,8 @@ import SplitButtonTemplate from "./SplitButtonTemplate.js"; | |||
| // Styles | |||
| import SplitButtonCss from "./generated/themes/SplitButton.css.js"; | |||
|
|
|||
| type SplitButtonAccessibilityAttributes = Pick<AccessibilityAttributes, "hasPopup" | "ariaRoleDescription" | "ariaLabel">; | |||
There was a problem hiding this comment.
We might need to have something like:
type SplitButtonRootA11yAttributes = Pick<AccessibilityAttributes, "hasPopup" | "ariaRoleDescription">;
type SplitButtonAccessibilityAttributes = { root?: SplitButtonRootA11yAttributes, arrowButton?: SplitButtonRootA11yAttributes, }
| @@ -12,6 +12,7 @@ export default function ButtonTemplate(this: Button) { | |||
| _hideArrowButton={this._hideArrowButton} | |||
| onClick={this._onClick} | |||
| onArrowClick={this._onArrowClick} | |||
| accessibilityAttributes={this._computedAccessibilityAttributes} | |||
There was a problem hiding this comment.
And for this to work we might want to split the ai accessibility attributes like this { root: {}, arrowButton: {} } similar to the same property in the ui5-split-button.
Depending on the use case the application developer could set aria-haspoup="menu" to the root button or to the arrow button.
| @@ -183,6 +205,10 @@ class Button extends UI5Element { | |||
| return !!this._stateText; | |||
| } | |||
|
|
|||
| get _leftButtonTooltipText() { | |||
| return this._hasText ? Button.i18nBundle.getText(BUTTON_TOOLTIP_TEXT, this._stateText as string) : ""; | |||
There was a problem hiding this comment.
We can directly store and use this as a variable in the _computedAccessibilityAttributes.
Don't think we need this method.
Improve acc attributes for ui5-ai-button component