Conversation
packages/react/src/lib/accessibility/Behaviors/Button/splitButtonBehavior.ts
Outdated
Show resolved
Hide resolved
docs/src/examples/components/Button/SplitButton/SplitButtonExample.shorthand.tsx
Outdated
Show resolved
Hide resolved
packages/react/src/lib/accessibility/Behaviors/Button/splitButtonBehavior.ts
Outdated
Show resolved
Hide resolved
docs/src/examples/components/Button/SplitButton/SplitButtonExample.shorthand.tsx
Outdated
Show resolved
Hide resolved
docs/src/examples/components/Button/SplitButton/SplitButtonExample.shorthand.tsx
Outdated
Show resolved
Hide resolved
…dust-ui/react into feat/split-button-component
| }) | ||
|
|
||
| return ( | ||
| <ElementType className={classes.root} {...accessibility.attributes.root} {...unhandledProps}> |
There was a problem hiding this comment.
does this mean people can use onClick and it will be invoked for both click on the button as well as click on the toggle?
There was a problem hiding this comment.
Yep, for the whole container actually. And I can't obviously what is expected
| handleMenuButtonOverrides = (predefinedProps: MenuButtonProps) => ({ | ||
| onMenuItemClick: (e: React.SyntheticEvent, menuItemProps: MenuItemProps) => { | ||
| this.setState({ open: false }) | ||
| _.invoke(this.props, 'onOpenChange', e, { open: false, ...menuItemProps }) |
There was a problem hiding this comment.
I vote for sending this.props back to the user. popup here is internal, no need to expose it. what do you think @layershifter?
There was a problem hiding this comment.
actually, similarly to what we have in popup:
{ ...this.props, ...{ open: false }
| onOpenChange: (e: React.SyntheticEvent, popupProps: PopupProps) => { | ||
| e.stopPropagation() | ||
| this.setState({ open: popupProps.open }) | ||
| _.invoke(this.props, 'onOpenChange', e, popupProps) |
There was a problem hiding this comment.
{ ...this.props, ...{ open: popupProps.open }
CHANGELOG.md
Outdated
| - Fix broken code editor in some doc site examples and improve error experience @levithomason ([#1906](https://github.com/stardust-ui/react/pull/1906)) | ||
|
|
||
| ### Features | ||
| - Add the `SplitButton` component @silviuavram ([#1789](https://github.com/stardust-ui/react/pull/1798)) |
There was a problem hiding this comment.
Please move it upper, Features section has been already created
|
|
||
| const SplitButtonExampleIconShorthand = () => <SplitButton menu={items} button={items[0]} /> | ||
|
|
||
| export default SplitButtonExampleIconShorthand |
There was a problem hiding this comment.
| export default SplitButtonExampleIconShorthand | |
| export default SplitButtonIconAndContentExampleShorthand |
Oops, filename is not match
| import * as React from 'react' | ||
| import { SplitButton } from '@stardust-ui/react' | ||
|
|
||
| const SplitButtonExampleDisabledShorthand = () => <SplitButton disabled button="New conversation" /> |
There was a problem hiding this comment.
I understand that items are not required for this example, but can we add them?
<SplitButton
disabled
menu={[
{ key: 'group', content: 'New group message' },
{ key: 'channel', content: 'New channel message' },
]}
button="New conversation"
/>| <ExampleSection title="Usage"> | ||
| <ComponentExample | ||
| title="Main option change" | ||
| description="A split button can have its main option changed based on last selection." |
There was a problem hiding this comment.
| description="A split button can have its main option changed based on last selection." | |
| description="A SplitButton can have its main option changed based on last selection." |
nit: to be consistent between sections
| * @param {SyntheticEvent} event - React's original SyntheticEvent. | ||
| * @param {object} data - All props and proposed value. | ||
| */ | ||
| onOpenChange?: ComponentEventHandler<PopupProps> |
There was a problem hiding this comment.
| onOpenChange?: ComponentEventHandler<PopupProps> | |
| onOpenChange?: ComponentEventHandler<SplitButtonProps> |
As I see in latest change it should be SplitButtonProps
| } | ||
|
|
||
| class SplitButton extends AutoControlledComponent<WithAsProp<SplitButtonProps>, SplitButtonState> { | ||
| static create: Function |
There was a problem hiding this comment.
This thing is typed now
| static create: Function | |
| static create: ShorthandFactory<SplitButton> |
| handleMenuButtonOverrides = (predefinedProps: MenuButtonProps) => ({ | ||
| onMenuItemClick: (e: React.SyntheticEvent, menuItemProps: MenuItemProps) => { | ||
| this.setState({ open: false }) | ||
| _.invoke(this.props, 'onOpenChange', e, { open: false, ...this.props }) |
There was a problem hiding this comment.
| _.invoke(this.props, 'onOpenChange', e, { open: false, ...this.props }) | |
| _.invoke(this.props, 'onOpenChange', e, { ...this.props, open: false }) |
Order should be change otherwise a value from props will win
| onOpenChange: (e: React.SyntheticEvent, popupProps: PopupProps) => { | ||
| e.stopPropagation() | ||
| this.setState({ open: popupProps.open }) | ||
| _.invoke(this.props, 'onOpenChange', e, { open: popupProps.open, ...this.props }) |
There was a problem hiding this comment.
| _.invoke(this.props, 'onOpenChange', e, { open: popupProps.open, ...this.props }) | |
| _.invoke(this.props, 'onOpenChange', e, { ...this.props, open: popupProps.open }) |
| }) | ||
|
|
||
| return ( | ||
| <ElementType className={classes.root} {...accessibility.attributes.root} {...unhandledProps}> |
There was a problem hiding this comment.
Yep, for the whole container actually. And I can't obviously what is expected
|
|
||
| this.setState(state => { | ||
| const open = !state.open | ||
| _.invoke(this.props, 'onOpenChange', e, { open, ...this.props }) |
There was a problem hiding this comment.
| _.invoke(this.props, 'onOpenChange', e, { open, ...this.props }) | |
| _.invoke(this.props, 'onOpenChange', e, { ...this.props, open }) |
…dust-ui/react into feat/split-button-component
* first implementation * shorthand on slots * fix the click handlers * addressed comments from PR * prevent default behavior * address code review with ref * moved it as a separate example * changed the example content * replaced specification of behaviour * address more code review * add behavior in tests * moved behavior * add styles * focus button on select * some minor improvements * updated styles * keep the Tab key handler * refactored buttons * updates on styles and toggle close * addressed code review * fix behavior test * made component conformant * defaultProp toggleButton * changelog * add more unit tests * add more tests * fix focus style on toggle button * more examples * changed the padding according to specs * code review integration * use focus-visible in styles * revert style slot rename * add best practices * fix variable name * update changelog * call onOpenChange with this.props * change onOpenChange prop type * changelog update * integrate code review (cherry picked from commit e7a59c0)
Main action should be executed by
onMainButtonClick. (should be namedonButtonClick?)Other actions should be executed by
onMenuItemClick. Or by havingonClickon each Menu Item.Shorthands for both
buttonandtoggleButton.Added open by ArrowDown+Alt and close by ArrowUp+Alt.
ToDo as future task:
smallvariation.