don't always set tooltip.keyBindingCommand

[aminya]

• Fixes [Performance] improve atom.tooltips.add #307
• Almost doubles the speed of button creation (X2 faster)
• See [Performance] improve atom.tooltips.add #307 (comment) for the reason

Button creation time:

After:

Before:

This is not shown in the loading time of tool-bar since other packages add buttons using the service, but instead, it can be easily measured using window.performance.now()

[ericcornelissen]

I'm personally not a fan of this, as it actually changes the tool-tip as seen here:

Before	After

I would be fine with adding it as an option though.

[aminya]

I'm personally not a fan of this, as it actually changes the tool-tip as seen here:

Before After

I would be fine with adding it as an option though.

We can certainly make a PR to Atom, and see why this key-binding adding takes time. I uploaded the branch that I ported the Atom's tooltip function:
https://github.com/aminya/tool-bar/blob/df0436db663e401471ea4acd4fd6f599f5ada84b/lib/items/tool-bar-button-view.js#L254-L325

We can also add the key-binding to the actual text instead of going through Atom's code.

tooltip.title = tooltip.title + this.options.callback

[ericcornelissen]

We can certainly make a PR to Atom, and see why this key-binding adding takes time.

I prefer that 100% over just removing this feature from the tool-bar. But it's probably because it needs to find the keybind value in a rather large set of callback possibilities, though that doesn't mean it cannot be optimized.

We can also add the key-binding to the actual text instead of going through Atom's code.

tooltip.title = tooltip.title + this.options.callback

That won't work. The callback value, in the case if-branch you removed is entered, is not the keybind string itself, it is the command name.

[aminya]

We can certainly make a PR to Atom, and see why this key-binding adding takes time.

I prefer that 100% over just removing this feature from the tool-bar. But it's probably because it needs to find the keybind value in a rather large set of callback possibilities, though that doesn't mean it cannot be optimized.

We can also add the key-binding to the actual text instead of going through Atom's code.

tooltip.title = tooltip.title + this.options.callback

That won't work. The callback value, in the case if-branch you removed is entered, is not the keybind string itself, it is the command name.

I see. Thanks for the clarification. We can look into the actual function. The performance is not that bad though, but we may able to boost it a bit.

[ericcornelissen]

I see. Thanks for the clarification. We can look into the actual function. The performance is not that bad though, but we may able to boost it a bit.

That means we can close this PR, right?

Again, though, I'm not against adding an option that disables this feature 😉

[aminya]

Let's keep it open for the future reference. If I close it, it might get lost 😄