Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Adding an event to listen to for package state changes (closes #36) #38

Merged
merged 5 commits into from Mar 10, 2014

Conversation

Projects
None yet
2 participants
Contributor

bjtitus commented Feb 27, 2014

Not sure if this is the best way to do this but it seemed to be the easiest way for now. This will at least patch the potentially harmful ability to be able to install packages multiple times.

Obviously, I'm very new to this codebase (and not too experienced with CoffeeScript either) so let me know if this can be improved somehow.

bjtitus and others added some commits Feb 27, 2014

Adding an event to key off for package state changes (closes #36)
Not sure if this is the best way to do this but it seemed to be the easiest way for now. This will at least patch the potentially harmful ability to be able to install packages multiple times.
Merge pull request #44 from jsmecham/prevent-text-selection-in-config…
…-menu

Prevent text selection in the config menu

@kevinsawicki kevinsawicki commented on an outdated diff Mar 1, 2014

lib/available-package-view.coffee
@@ -18,21 +18,37 @@ class AvailablePackageView extends View
initialize: (@pack, @packageManager) ->
@type = if @pack.theme then 'theme' else 'package'
+ @eventName = @pack.name + "-event"
+ @document = $(atom.workspaceView)
@kevinsawicki

kevinsawicki Mar 1, 2014

Owner

atom.workspaceView already extends the jQuery prototype so you shouldn't need to wrap it in $().

@kevinsawicki kevinsawicki commented on an outdated diff Mar 1, 2014

lib/available-package-view.coffee
@@ -18,21 +18,37 @@ class AvailablePackageView extends View
initialize: (@pack, @packageManager) ->
@type = if @pack.theme then 'theme' else 'package'
+ @eventName = @pack.name + "-event"
+ @document = $(atom.workspaceView)
+
+ @document.on @eventName, (event, status, error) =>
@kevinsawicki

kevinsawicki Mar 1, 2014

Owner

The @packageManager object is probably a better place to listen for and emit events since the same instance if used across all panels/views in the settings view.

That way atom.workspaceView doesn't have to be involved, just the single PackageManager instance.

Contributor

bjtitus commented Mar 2, 2014

Thanks @kevinsawicki!

I learned about the jQuery element deal later. Great to know about PackageManager and emissary! It would be nice to have a list of the core events somewhere in the API docs if possible. I added a custom "package-installing" (which really covers package & theme installation). This should probably be added to the core instead, but I figured it was worth adding in here for now.

Let me know if there's any other way this can be improved.

Owner

kevinsawicki commented Mar 10, 2014

Thanks for this, sorry it took a bit long to merge in.

kevinsawicki added a commit that referenced this pull request Mar 10, 2014

Merge pull request #38 from bjtitus/master
Adding an event to listen to for package state changes (closes #36)

@kevinsawicki kevinsawicki merged commit 2e73de8 into atom:master Mar 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment