-
Notifications
You must be signed in to change notification settings - Fork 278
feat(ui5-bar): allow role change #11134
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
Conversation
| * | ||
| */ | ||
| @property() | ||
| accessibleRole: `${BarAccessibleRole}` = "Toolbar"; |
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.
We'll need to provide recommendations to the users on how to use this new property.
In the openui5 project the logic is internal and no explanations are required.
Also in order to properly test this ourselves we could provide a dev sample that changes the number of interactive elements at runtime and therefore changes the role accordingly.
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.
I've added additional documentation and example. But i've added a static example with just two bars explaining the both cases, because we don't provide dynamic change of the role, so such example wont be needed.
Stoev
left a comment
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.
Please consider adding short explanation:
feat(ui5-bar): allow role changes - maybe "role changes" is more explanatory?..
OR, eventually:
feat(ui5-bar): support for specific roles - just another suggestion...
And maybe consider adding some explanatory text, for example:
- Added new property
accessibleRoleto theBarclass. Barcomponent now supports custom roles for specific use cases.- Updated the
accInfomethod to include theeffectiveRole.
Hi @Stoev, I was about to add you after I add the documentation that's ready for review. I've just added the documentation. The version you've reviewed is pre-documentation. Can you please review it now? I've added API documentation with example when and how it should be used. Also an additional sample was added showing both cases of the bar usage. |
Stoev
left a comment
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.
Just minor recommendations:
Introduces a new accessibleRole property to the ui5-bar component, allowing developers to customize the ARIA role of the bar for accessibility purposes.
By default, the role is set to "toolbar", but now it can be explicitly changed by the application.
|
🎉 This PR is included in version v2.11.0-rc.1 🎉 The release is available on v2.11.0-rc.1 Your semantic-release bot 📦🚀 |
Introduces a new
accessibleRoleproperty to theui5-barcomponent, allowing developers to customize the ARIA role of the bar for accessibility purposes.By default, the role is set to "toolbar", but now it can be explicitly changed by the application.
fixes: #10990