chore: update warning in button component #1455
Conversation
Chisomchima
changed the title
|
🚀 Deployed on https://pr-1455--dhis2-ui.netlify.app |
Passing run #3249 ↗︎
Details:
Review all test suite changes for PR #1455 ↗︎ |
|||||||||||||||
|
|
||
| if (!children && !title && !ariaLabel) { | ||
| console.debug( | ||
| 'Button component has no children but is missing title or ariaLabel attribute.' |
There was a problem hiding this comment.
Do we want to ariaLabel to fallback to title?
| expect(consoleSpy).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('does not warn if aria-label or title is present', () => { |
There was a problem hiding this comment.
This "or" should be "and" according to the test implementation, right? Could be more clear if we had two tests, where ariaLabel is passed in without title, and the opposite. But I guess that would make more sense without passing children?
There was a problem hiding this comment.
yeah, you're right, "and" seems like a better choice in this case.
| @@ -37,6 +37,14 @@ export const Button = ({ | |||
| } | |||
| }, [initialFocus, ref.current]) | |||
|
|
|||
| const { 'aria-label': ariaLabel, title } = otherProps | |||
There was a problem hiding this comment.
not part of this PR - but I think ariaLabel in the previous tests should be updated to aria-label like in this test
There was a problem hiding this comment.
yeah we should make sure these are aria-label (not camelcased). This applies to your previous PR as well @d-rita - let's update them to make them consistent aria-*
In React, all DOM properties and attributes (including event handlers) should be camelCased. For example, the HTML attribute tabindex corresponds to the attribute tabIndex in React. The exception is aria-* and data-* attributes, which should be lowercased. For example, you can keep aria-label as aria-label.
kabaros
force-pushed
the
LIBS-565/button-accessibility
branch
from
2c94aef to
c8abea9
Compare
kabaros
changed the title
Implements LIBS-565
Description
Implemented LIBS-565 by updating developer warning in button component. This change ensures that accessibility attributes can be easily included when using the Button component.
Known issues
Checklist
All points above should be relevant for feature PRs. For bugfixes, some points might not be relevant. In that case, just check them anyway to signal the work is done.
Screenshots
Additional Notes
Buttoncomponent to destructuretitleandariaLabelfromotherPropsand apply them to the<button>element.titleandaria-labelattributes are correctly passed to the rendered<button>element.otherPropswhile maintaining accessibility.supporting text