{"action":"created","issue":{"url":"https://api.github.com/repos/suda/tool-bar/issues/163","repository_url":"https://api.github.com/repos/suda/tool-bar","labels_url":"https://api.github.com/repos/suda/tool-bar/issues/163/labels{/name}","comments_url":"https://api.github.com/repos/suda/tool-bar/issues/163/comments","events_url":"https://api.github.com/repos/suda/tool-bar/issues/163/events","html_url":"https://github.com/suda/tool-bar/pull/163","id":177009005,"number":163,"title":"✨ Plugin Manager","user":{"login":"jerone","id":55841,"avatar_url":"https://avatars.githubusercontent.com/u/55841?v=3","gravatar_id":"","url":"https://api.github.com/users/jerone","html_url":"https://github.com/jerone","followers_url":"https://api.github.com/users/jerone/followers","following_url":"https://api.github.com/users/jerone/following{/other_user}","gists_url":"https://api.github.com/users/jerone/gists{/gist_id}","starred_url":"https://api.github.com/users/jerone/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jerone/subscriptions","organizations_url":"https://api.github.com/users/jerone/orgs","repos_url":"https://api.github.com/users/jerone/repos","events_url":"https://api.github.com/users/jerone/events{/privacy}","received_events_url":"https://api.github.com/users/jerone/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/suda/tool-bar/labels/enhancement","name":"enhancement","color":"84b6eb"},{"url":"https://api.github.com/repos/suda/tool-bar/labels/needs%20update","name":"needs update","color":"006b75"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2016-09-14T20:15:59Z","updated_at":"2016-09-29T11:13:20Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/suda/tool-bar/pulls/163","html_url":"https://github.com/suda/tool-bar/pull/163","diff_url":"https://github.com/suda/tool-bar/pull/163.diff","patch_url":"https://github.com/suda/tool-bar/pull/163.patch"},"body":"Closes #49\r\n\r\n![Tool Bar plugins](https://cloud.githubusercontent.com/assets/55841/18816833/c0fe5e0c-8351-11e6-99d0-a52e53cc1654.gif)\r\n\r\nSome comments:\r\n* Config settings are set programmatically.\r\n* New config setting (by installing package) require an Atom restart (Settings Views cache needs to be cleared for the setting to appear).\r\n* By default all plugins are enabled (backwards compatible).\r\n* Disabling & enabling works instantly (see screenie).\r\n* All plugins settings in the Settings View are grouped in a section named \"Plugins\".\r\n* Groups can't have description, but I think the plugin setting descriptions are sufficient.\r\n* I don't think we can remove the config setting when plugin is uninstalled (needs confirmation).\r\n* Ideas \"borrowed\" from [Minimap](https://github.com/atom-minimap/minimap/blob/master/lib/mixins/plugin-management.js) package.\r\n* This opens support for [custom button types](https://github.com/suda/tool-bar/issues/43).\r\n\r\nTODO:\r\n* [x] Implement plugin manager.\r\n* [x] Figure out why plugin setting description don't show up anymore.\r\n* [x] Specs.\r\n* [x] Documentation.\r\n* [x] Remove `console`-messages.\r\n* [ ] Test, test, test."},"comment":{"url":"https://api.github.com/repos/suda/tool-bar/issues/comments/250437734","html_url":"https://github.com/suda/tool-bar/pull/163#issuecomment-250437734","issue_url":"https://api.github.com/repos/suda/tool-bar/issues/163","id":250437734,"user":{"login":"jerone","id":55841,"avatar_url":"https://avatars.githubusercontent.com/u/55841?v=3","gravatar_id":"","url":"https://api.github.com/users/jerone","html_url":"https://github.com/jerone","followers_url":"https://api.github.com/users/jerone/followers","following_url":"https://api.github.com/users/jerone/following{/other_user}","gists_url":"https://api.github.com/users/jerone/gists{/gist_id}","starred_url":"https://api.github.com/users/jerone/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jerone/subscriptions","organizations_url":"https://api.github.com/users/jerone/orgs","repos_url":"https://api.github.com/users/jerone/repos","events_url":"https://api.github.com/users/jerone/events{/privacy}","received_events_url":"https://api.github.com/users/jerone/received_events","type":"User","site_admin":false},"created_at":"2016-09-29T11:13:20Z","updated_at":"2016-09-29T11:13:20Z","body":"[**@zertosh**](https://github.com/zertosh) commented on [29 sep. 2016 05:09 CEST](https://github.com/suda/tool-bar/pull/163#issuecomment-250359768 \"2016-09-29T03:09:51Z - Replied by Github Reply Comments\"):\r\n> > How does this PR compare, performance-wise, to the current situation?\r\n> \r\n> If a button is hidden, then you're increasing its cost because it gets added to the DOM first, then removed.\r\n\r\nOh sorry, I meant other then adding-and-then-removing-issue (fully agree on that, am looking into that)...\r\n\r\n> > atom.config.set only happens once, when the plugin is for the first time is registered.\r\n> \r\n> So perf aside on this one, I know many people (including myself) that version control `~/.atom/config.cson`. This going to cause churn :( \r\n\r\nActually I'm [syncing](https://atom.io/packages/sync-settings) [my config](https://gist.github.com/jerone/c4a69b06920a0471e71a) too.\r\n\r\n> From looking at the code, I can guess that you're doing this because immediately below you call `atom.config.observe` and that was giving you `undefined`?\r\n\r\n> You can't access the defaults for exported configs during `load`. We've had that problem too. If you know what the default is supposed to be, then use that if you get `undefined` from `atom.config.get` (that means the user hasn't set a value). See https://github.com/facebook/nuclide/blob/8c8206d/lib/main.js#L188-L194.\r\n\r\nThinking about it, I could probably remove [that block](https://github.com/suda/tool-bar/pull/163/files#diff-54907223f3ec35e247aaa95a941346aeR28) as the [default](https://github.com/suda/tool-bar/pull/163/files#diff-54907223f3ec35e247aaa95a941346aeR24) is already `true`. The idea was copied [from Minimap](https://github.com/atom-minimap/minimap/blob/master/lib/mixins/plugin-management.js#L232-L234). I have to check if it's not required for the dynamic config schema.\r\n\r\n> > This change is fully backwards compatible, so I don't understand your comment about \"breaking change\"...\r\n> \r\n> At least Nuclide assumes that buttons always have a parent node. If a user chooses to hide one, that's no longer true. This is fine, but def a breaking change :)\r\n\r\nIn an early draft the buttons where hidden with `display: none` for this reason. I removed that solution; because it was just to easy to force show the button again. Right now they are [indeed removed from their parent](https://github.com/suda/tool-bar/pull/163/files#diff-1a1d06dd483f0b6084823a52bf18e9fbR104). I've not seen any other package having issue with that. Would something break for Nuclide if you've build an extra parentnode check around it? \r\n\r\n> So now would be the time to implement the queue since that implies that consumers can't assume that a button in the DOM. I think the current implementation of enabled/disabled living as a separate, yet tightly coupled module, `tool-bar-plugin-manager.js`, makes the queue more complicated. I generally really like the idea of the user having control over the buttons, but maybe that logic belongs in `tool-bar-manager.js`. I have to think about it.\r\n\r\n:+1:\r\n\r\n> > Do you have an example of DOM queue solution?\r\n> \r\n> Let me think about it, because the tricky part is only applying the queue during Atom's init. Or always applying it, but flush it on a `requestAnimationFrame`. I don't know what the profile of that is. I'll fiddle with it tomorrow and put up a RFC PR.\r\n\r\n> > This PR is open for maintainers, so if you want and have the time I would value your contribution.\r\n> \r\n> Thank you :) I'll give this time tomorrow!\r\n\r\nReally value your input :heart: (and others)!"}}
{"actor":{"display_login":"jerone"}}