Skip to content
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

Merged
merged 7 commits into from Apr 15, 2020
Merged

Conversation

aminya
Copy link
Member

@aminya aminya commented Apr 12, 2020

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.

item = toolbar.addItem({
  element: myElement,
  priority: someNumber // optional
})

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

  • Defined ToolBarItem which is the parent (super) of all classes (basically refractors the common part of ToolBarButton and ToolBarSpacer)
  • addItem function that constructs a ToolBarItem and adds it to the Toolbar.
  • Made ToolBarButton inherit from ToolBarItem
  • Made ToolBarSpacer inherit from ToolBarItem
  • Made classes named exports

@aminya aminya force-pushed the addButtonByElement branch 6 times, most recently from 8ebd036 to 9361600 Compare April 13, 2020 00:46
@suda
Copy link
Collaborator

suda commented Apr 13, 2020

Nice! LGTM :) If you add some docs to the readme, we could ship it! Do you agree @ericcornelissen ?

@ericcornelissen
Copy link
Contributor

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 🤔)

@aminya

This comment has been minimized.

@ericcornelissen
Copy link
Contributor

Small detail, but I would prefer to move the addItem documentation after the addSpacer documentation as addItem is the "most advanced" method (or at least after addButton, in alphabetical order).

@aminya
Copy link
Member Author

aminya commented Apr 14, 2020

@ericcornelissen done in 533d8f6

@suda suda merged commit 4c435c2 into atom-community:master Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passing HTML element to addButtonByElement
3 participants