[terra-dropdown-button] Add support for icons in split button #4080
Conversation
| ] | ||
| ], | ||
| "devDependencies": { | ||
| "terra-icon": "^3.60.0" |
There was a problem hiding this comment.
Added this as a devDependency for use in Jest tests
| @@ -105,7 +105,11 @@ exports[`Dropdown Button correctly applies the theme context className 1`] = ` | |||
| onKeyUp={[Function]} | |||
| type="button" | |||
| > | |||
| Primary Option | |||
| <span | |||
| className="" | |||
There was a problem hiding this comment.
Not sure if we want to leave the className attribute blank here or just remove it entirely.
There was a problem hiding this comment.
NIT: I'd probably removed it for cleaner look, unless I am missing something. What is a benefit of keeping it?
There was a problem hiding this comment.
No benefit, and I agree with you on the cleaner look. I'll remove it.
|
@sycombs - The split button looks good and navigation is working. I observed on issue with JAWS where, when I Tab to the first button in the group, JAWS does not announce the name, role, value. |
There was a problem hiding this comment.
It looks like text_first and icon_first are switched, as text first has actually icon first (unless I missed something)
There was a problem hiding this comment.
oops, good catch! fixed in 40003a0
I rebooted by desktop and JAWS has returned to normal. The Split Button is working as required. Great work! |
| @@ -331,6 +331,26 @@ Terra.describeViewports('Split Button', ['medium'], () => { | |||
| }); | |||
| }); | |||
|
|
|||
| describe('With icon', () => { | |||
| it('displays a button with icon first', () => { | |||
| browser.url('/raw/tests/cerner-terra-core-docs/dropdown-button/left-icon-split-button'); | |||
There was a problem hiding this comment.
All these example seem pretty small, so I wonder if we can optimize these tests by having them all on the same page. Unless if it's not worth the effort.
There was a problem hiding this comment.
sure, combined them into one test/snapshot here: 7396391
There was a problem hiding this comment.
That looks good! Can you also add text labels for easier identification?
e.g.
<h3>icon left</h3>
<iconLeftExample
<h3>icon right</h3>
<iconRightexample
Summary
This PR updates the
terra-dropdown-buttonSplitButtonto support icon-only, left-aligned icon, and right-aligned icon buttons in order to maintain passivity for the Fusion passthrough.Testing
This change was tested using:
Screenshots:
Reviews
In addition to engineering reviews, this PR needs:
Additional Details
This PR resolves:
UXPLATFORM-10337
Thank you for contributing to Terra.
@cerner/terra