-
Notifications
You must be signed in to change notification settings - Fork 1
fix(Icon): Wrap in forwardRef to be used as Dropdown trigger #31
Conversation
Codecov Report
@@ Coverage Diff @@
## master #31 +/- ##
==========================================
+ Coverage 88.38% 88.55% +0.17%
==========================================
Files 13 13
Lines 198 201 +3
Branches 31 32 +1
==========================================
+ Hits 175 178 +3
Misses 16 16
Partials 7 7
Continue to review full report at Codecov.
|
Nice! |
<Dropdown | ||
width={250} | ||
placement="top" | ||
trigger={<Icon name="information-outline" />} |
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.
In order to make icon buttons accessible, we can wrap the icon in a div with role=Button and tabIndex=0. https://louiseclark.tech/tech-blog/accessibility-journey-making-buttons-accessible
How about assigning directly to the trigger itself: diff --git a/src/Dropdown/Dropdown.js b/src/Dropdown/Dropdown.js
index 7c0e70b..5341a9e 100644
--- a/src/Dropdown/Dropdown.js
+++ b/src/Dropdown/Dropdown.js
@@ -110,6 +110,8 @@ export default function Dropdown({
ref(node);
}
},
+ role: trigger.role || 'button',
+ tabIndex: trigger.tabIndex || 0,
'aria-haspopup': true,
'aria-expanded': isOpen,
onClick: handleTrigger, EDIT @cehsu The above, or wrapping in div with same properties allow me to trigger dropdown with spacebar or enter :/ |
No description provided.