-
Notifications
You must be signed in to change notification settings - Fork 50
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
Add button by element #294
Conversation
8ebd036
to
9361600
Compare
9361600
to
cc9af24
Compare
Nice! LGTM :) If you add some docs to the readme, we could ship it! Do you agree @ericcornelissen ? |
Implementation looks good to me 👍 (not sure if the comment about calling super in the constructor are necessary, as that's pretty common for classes 🤔) |
This comment has been minimized.
This comment has been minimized.
Small detail, but I would prefer to move the |
@ericcornelissen done in 533d8f6 |
Fixes #291
Motivation
Similar to Status-Bar that has a function that accepts an HTML element, we should allow this too. This feature in combination with the typical
addButton
allows complex usage of Toolbar.Then, the user can do whatever they wanna do with it. For example, define custom buttons like dropdowns, define click callbacks, hide it when they want, etc. It easily solves #43 and #192 by removing the limitations.
Changes