-
Notifications
You must be signed in to change notification settings - Fork 272
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
feat(ui5-segmentedbutton): implement accessibility spec #1475
Conversation
> | ||
<slot></slot> | ||
{{#each this.buttons}} | ||
<div class="ui5-segmentedbutton-item" role="radio" aria-checked="{{this.pressed}}"> |
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.
-
The class seems to do nothing? Unless there should be some css for it, please delete it
-
aria-checked
does not change upon space. It changes whenever the segmented button itself is rerendered later, for whatever reason. If you want the segmented button to be invalidated whenever the children are invalidated (and you do in this case), you should uselistenFor
in the slot metadata.
"default": { propertyName: "buttons", type: HTMLElement, individualSlots: true, listenFor: { include: ["pressed"] } },
Once you do this, whenever pressed
changes for a toggle button, the template for segmented button will be re-evaluated and the aria-checked binding will be updated as well.
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.
Done. The classes are now currently used for selecting the wrapper divs and applying the style.
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.
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.
if it closes this issue, reference it:
#1286
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.
Is this everything or work in progress? Shouldn't there be more information about the current options, selected option, etc... pretty much like a Select comonent?
@@ -243,6 +243,9 @@ MULTIINPUT_SHOW_MORE_TOKENS={0} More | |||
#XTOL: Tooltip for panel expand title | |||
PANEL_ICON=Expand/Collapse | |||
|
|||
#XACT: ARIA description for the segmented button | |||
SEGMENTEDBUTTON_ARIA_DESCRIPTION=Segmented button |
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.
Is this really the right text? Isn't "Segmented button" a term only we know, and not meant for the end user?
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.
That's the specification given me by @elenastoyanovaa .
Eli, I think you can give more information?
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.
Yes, this is the right text. Since the component/control is specific we are adding an aria-roledescription according to Aria 1.1. This is already discussed and will also be implemented in openui5.
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.
Approved from ACC side
* feat(ui5-segmentedbutton): implement acc spec * fixed styling * fixed comments * changes related to spec changing
fixes: #1286
To get it merged faster, kindly review the checklist below:
Pull Request Checklist