-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restrict Dropdown #212
Restrict Dropdown #212
Conversation
Should all the options use a pointer cursor? 🤔 |
Let me consult with design. |
}); | ||
private renderIcon(icon: React.ReactNode, isHighlighted: boolean, option: DropdownItem | DropdownLinkItem) { | ||
return ( | ||
React.isValidElement(icon) && |
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.
Can we add a check to make sure the type is icon?
id={id} | ||
key={index} | ||
onClick={() => this.handleOnItemClick(option)} | ||
onFocus={this.handleOnItemFocus} |
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.
Should onFocus
and onMouseOver
call the same fn?
|
||
interface BaseItem extends Omit<ListItemProps, 'children' | 'content' | 'onClick' | 'value'> { |
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.
Can you move the BaseItem
definition before DropdowItem
and DropdownLinkItem
🙏
<Dropdown.Item value={2}>Option</Dropdown.Item> | ||
<Dropdown.Item value={3}>Option</Dropdown.Item> | ||
</Dropdown> | ||
<Dropdown |
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.
🍹
<Dropdown
options={[
{ content: 'Option', type: 'string', value: '0' },
{ content: 'Option', type: 'string', value: '1', onClick },
{ content: 'Option', type: 'string', value: '2', actionType: 'destructive' },
{ content: 'Option', type: 'string', value: '3', icon: <CheckCircleIcon /> },
]}
trigger={<Button>Button</Button>}
/>
{ content: 'Option', url: '#', type: 'link' }, | ||
]} | ||
trigger={<Button>Button</Button>} | ||
></Dropdown>, |
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.
Same as above.
> | ||
{option.type === 'link' && !option.disabled ? ( | ||
<Link href={option.url} target={option.target}> | ||
<Flex alignItems="center"> |
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.
<Flex alignItems="center"> | |
<Flex alignItems="center" flexDirection="row"> |
</Flex> | ||
</Link> | ||
) : ( | ||
<Flex alignItems="center"> |
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.
Same as above
dc87104
to
8d33ae4
Compare
Added new
options
prop that will restrict the usage of the items. TheonClick
for each item with return theitem
itself. No need to usedata-value
anymore.Now also accepts
icons
as a DropdownItem prop.Wasn't sure how to document the new
DropdownItem
andDropdownLinkItem
. Feedback pls 🙏 .