Feat(web-react): Introduce UncontrolledSplitButton #DS-1656
#1949
+402
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for spirit-design-system-storybook ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
pavelklibani
commented
Feb 17, 2025
✅ Deploy Preview for spirit-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
pavelklibani
force-pushed
the
feat/ds-1671-uncontrolled-splitbutton
branch
2 times, most recently
from
3b8eebe to
f57727c
Compare
pavelklibani
requested review from
literat,
curdaj,
katerinarehorkova and
a team
as code owners
pavelklibani
force-pushed
the
feat/ds-1671-uncontrolled-splitbutton
branch
from
f57727c to
9539fbd
Compare
literat
reviewed
Feb 18, 2025
pavelklibani
force-pushed
the
feat/ds-1671-uncontrolled-splitbutton
branch
from
9539fbd to
0bd4f4e
Compare
literat
reviewed
Feb 19, 2025
packages/web-react/src/components/SplitButton/UncontrolledSplitButton.tsx
Outdated
Show resolved
Hide resolved
pavelklibani
force-pushed
the
feat/ds-1671-uncontrolled-splitbutton
branch
2 times, most recently
from
dedd130 to
0c1eac2
Compare
packages/web-react/src/components/SplitButton/__tests__/UncontrolledSplitButton.test.tsx
Show resolved
Hide resolved
pavelklibani
force-pushed
the
feat/ds-1656-split-button-react
branch
from
992f1b2 to
f14714e
Compare
pavelklibani
force-pushed
the
feat/ds-1671-uncontrolled-splitbutton
branch
from
0c1eac2 to
8d37fbd
Compare
pavelklibani
force-pushed
the
feat/ds-1656-split-button-react
branch
from
f14714e to
23274aa
Compare
pavelklibani
force-pushed
the
feat/ds-1671-uncontrolled-splitbutton
branch
from
8d37fbd to
951c3ec
Compare
packages/web-react/src/components/SplitButton/stories/UncontrolledSplitButton.stories.tsx
Show resolved
Hide resolved
pavelklibani
force-pushed
the
feat/ds-1671-uncontrolled-splitbutton
branch
from
951c3ec to
03e3a26
Compare
crishpeen
reviewed
Feb 20, 2025
| | `isDisabled` | `boolean` | `false` | ✕ | Disables the Split Button | | ||
| | `size` | [Size dictionary][dictionary-size] | `medium` | ✕ | Size variant | | ||
|
|
||
| (\*) Conditionally required: either `buttonIconName` or `buttonLabel` must be provided. |
There was a problem hiding this comment.
How can I add a label (for a11y) to a button with only an icon, please?
There was a problem hiding this comment.
Added a new prop, isLabelHidden, so the label is always required, and the button has its label.
packages/web-react/src/components/SplitButton/UncontrolledSplitButton.tsx
Outdated
Show resolved
Hide resolved
packages/web-react/src/components/SplitButton/UncontrolledSplitButton.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
We don't have uncontrolled demos visible due to visual tests, but I keep the demo files for development and debugging, so we don’t need to create a new demo to test it, etc.
There was a problem hiding this comment.
But uncontrolled variant is available in Storybook
- Solves #DS-1671
pavelklibani
force-pushed
the
feat/ds-1671-uncontrolled-splitbutton
branch
from
03e3a26 to
791bb5d
Compare
pavelklibani
force-pushed
the
feat/ds-1671-uncontrolled-splitbutton
branch
from
791bb5d to
56dfb38
Compare
literat
reviewed
Feb 20, 2025
packages/web-react/src/components/SplitButton/UncontrolledSplitButton.tsx
Outdated
Show resolved
Hide resolved
curdaj
reviewed
Feb 21, 2025
| | `id` | `string` | - | ✓ | Id of the Split Button and part of Dropdown id | | ||
| | `isDisabled` | `bool` | `false` | ✕ | Disables the Split Button | | ||
| | `isLabelHidden` | `bool` | `false` | ✕ \* | Whether is label hidden | | ||
| | `isDropdownLabelHidden` | `bool` | `false` | ✕ \*\* | Whether is label hidden | |
There was a problem hiding this comment.
what ** means? There is legend only for one *
curdaj
reviewed
Feb 21, 2025
| return ( | ||
| <SplitButton id={id} {...restProps} isDisabled={isDisabled}> | ||
| <Button onClick={buttonOnClick}> | ||
| {buttonIconName && <Icon name={buttonIconName} marginRight={!isLabelHidden ? 'space-400' : undefined} />} |
There was a problem hiding this comment.
Suggested change
| {buttonIconName && <Icon name={buttonIconName} marginRight={!isLabelHidden ? 'space-400' : undefined} />} | |
| {buttonIconName && <Icon name={buttonIconName} marginRight={isLabelHidden ? undefined : 'space-400'} />} |
IMHO more readable.
Or maybe marginRight={isLabelHidden && 'space-400'}
curdaj
approved these changes
Feb 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
TODO
Additional context
Uncontrolled demo will be removed before merge
Issue reference
Component UncontrolledSplitButton | React