Passing HTML element to addButtonByElement

[aminya]

Proposal

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.

toolbar.addButtonByElement(element)

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.

Implementation

This implementation is old. See the actual PR: https://github.com/suda/tool-bar/pull/294/files

we just need to define an addButtonByElement:
In https://github.com/suda/tool-bar/blob/master/lib/tool-bar-manager.js#L11-L16

  addButtonByElement(element) {
    this.toolBarView.addElement(element);
    this.touchBarManager.addElement(element);
    return element;
  }

and define toolBarView.addElement which is edited from addItem.
https://github.com/suda/tool-bar/blob/0b8a1ced016b852ed175f72d7a860b324ef438fd/lib/tool-bar-view.js#L65-L90

Similarly, touchBarManager.addElement can also be defined.

Alternative implementation

Instead of defining a new function, we can use the curren addButton and get the element as a property (named element or something), and instead of creating the element inside ToolBarButtonView, use the supplied element. User can use some desired features of addButton regularly (if their element is compatible). callback property should be optional in this case (we need to add if for all of the options in ToolBarButtonView. Even for onClick)

toolbar.addButton({
 element: myHTMLElement
 icon: "play"
 // etc
})

This implementation is easier to work with, since a lot of options are already available to use.

Usage Example

This could be a typical example of that button (similar to https://github.com/aminya/atom-indent-detective/blob/TypeScript/src/status.ts)

// Called only once for each Atom session
export function consumeToolBar(getToolBar) {
    toolBar = getToolBar('packageName');
    toolbarItem = new MyToolbarItem (toolBar )
    return
}

export class MyToolbarItem {
    button: ButtonElement | null
    text: HTMLElement | null
    view: HTMLElement

    constructor(toolBar) {

        // create an element and add stuff to it:
        this.view = document.createElement("button")
        this.view.classList.add("myclass")
        this.text = document.createElement("a")
        this.text.innerText = "Spaces (2)"
        this.view.appendChild(this.text)

        // add onclick callback
        this.view.onclick = function () {
             // ..........
        }

        // consumeToolBar
        this.button = toolBar.addButtonByElement(this.view)
    }
}

This basically allows the user to do whatever is done in https://github.com/suda/tool-bar/blob/master/lib/tool-bar-button-view.js by themselves.

References

Based on these:
https://github.com/atom/status-bar/blob/master/lib/status-bar-view.coffee#L64-L78
https://github.com/atom/status-bar/blob/master/lib/tile.coffee

[aminya]

@suda @ericcornelissen What do you think?

[ericcornelissen]

The feature request seems fine to me. I'm not a fan of your alternative implementation as, e.g. it's is very confusing to me what the icon key is going to do with my element. Just passing the HTML element as an argument seems better to me. If you want this kind of freedom, than you will have to do everything yourself, in my opinion.

It easily solves #43 and #192 by removing the limitations.

It does to some extend, but I don't think that means there should never be "native" support for those features 😉

[aminya]

The feature request seems fine to me. I'm not a fan of your alternative implementation as, e.g. it's is very confusing to me what the icon key is going to do with my element. Just passing the HTML element as an argument seems better to me. If you want this kind of freedom, than you will have to do everything yourself, in my opinion.

This seems fair.

It easily solves #43 and #192 by removing the limitations.

It does to some extend, but I don't think that means there should never be "native" support for those features 😉

Once we implement this feature, we can make some ready HTML Toolbar components (like the class I showed under Usage Example), which user can just call with a simple syntax:

addButtonsByElement(new DropDown(options))

or export a wrapper function for the class, which just does the above under the hood:

addDropDown(options)

[aminya]

I ended up using a different implementation which allows using classes! One can just inherit ToolbarItem for defining new button types.
#294