-
Notifications
You must be signed in to change notification settings - Fork 359
feat: Dropdown button #1887 #1906
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
Conversation
mturoci
left a comment
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.
Thanks @marek-mihok! Instead of introducing a new ButtonChoice, let's stick with what we already have for these types of scenarios: ui.command.
|
@mturoci originally I've considered it (please, check this commit out) but our existing And what about the [EDIT] And |
That's not a problem. The point is to have a consistent API since the split button menu is just a collection of commands same as ui.menu or card menu. Is the
Go with commands.
Why? |
e13bb31 to
69c6cb2
Compare
mturoci
left a comment
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.
mturoci
left a comment
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.
Looks good, just need to remove 1 line I missed before and we are good to go.
mturoci
left a comment
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.
LGTM.
This change brings the
commandsattribute for ui.button where you can provide other relevant actions for the button that are displayed in the context menu.Example:
Screen.Recording.2023-04-18.at.11.01.48.mov
Closes #1887