feat(Dropdown): Allow for tooltips on aria-disabled items #6038
Conversation
|
PF4 preview: https://patternfly-react-pr-6038.surge.sh |
| @@ -264,7 +267,7 @@ export class InternalDropdownItem extends React.Component<InternalDropdownItemPr | |||
| role={role} | |||
| onKeyDown={this.onKeyDown} | |||
| onClick={(event: any) => { | |||
| if (!isDisabled) { | |||
| if (!isDisabled && !isAriaDisabled) { | |||
There was a problem hiding this comment.
Should this be an || instead? Or does isDisabled also set isAriaDisabled if isAriaDisabled isn't passed in.
There was a problem hiding this comment.
Good question. Maybe add a test for this scenario.
There was a problem hiding this comment.
I may be going cross eyed - but i think it's true that if the dropdown item is disabled OR ariaDisabled, then we don't want to fire the passed in onClick or onSelect handlers. So this logic, with the && is right.
There was a problem hiding this comment.
Oh you're right, the double negative with the && got me
There was a problem hiding this comment.
Can rewrite as isDisabled || isAriaDisabled to be more clear. de Morgan's theorem states !A && !B = A || B.
|
@jessiehuff did you do any more investigation into this? I can try playing with it in VO this afternoon. |
|
I'm wondering if what Katie pointed out above is affecting the screen reader experience. It seems like if it's just |
nicolethoen
dismissed stale reviews from dlabrecq, mcoker, and mcarrano
via
82ae82b
nicolethoen
force-pushed
the
tooltips_on_disabled_dropdown
branch
from
5149ac1 to
82ae82b
Compare
|
I opened #6109 because tooltips on dropdown items are not screenreader accessible. |
| @@ -264,7 +267,7 @@ export class InternalDropdownItem extends React.Component<InternalDropdownItemPr | |||
| role={role} | |||
| onKeyDown={this.onKeyDown} | |||
| onClick={(event: any) => { | |||
| if (!isDisabled) { | |||
| if (!isDisabled && !isAriaDisabled) { | |||
There was a problem hiding this comment.
Can rewrite as isDisabled || isAriaDisabled to be more clear. de Morgan's theorem states !A && !B = A || B.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #5826
You can see it working in the very first example - one of the disabled items is now using aria-disabled with a simple tooltip.