-
Notifications
You must be signed in to change notification settings - Fork 2k
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
UI: Polling Step 3 - Close connections when tabbing away #3953
Changes from 6 commits
c864697
a6319f3
01de2ea
d29942f
4e0489a
df54b6b
27ec2c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import Mixin from '@ember/object/mixin'; | ||
|
||
export default Mixin.create({ | ||
setupDocumentVisibility: function() { | ||
this.set('_visibilityHandler', this.get('visibilityHandler').bind(this)); | ||
document.addEventListener('visibilitychange', this.get('_visibilityHandler')); | ||
}.on('init'), | ||
|
||
removeDocumentVisibility: function() { | ||
document.removeEventListener('visibilitychange', this.get('_visibilityHandler')); | ||
}.on('willDestroy'), | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import Mixin from '@ember/object/mixin'; | ||
|
||
export default Mixin.create({ | ||
setupDocumentVisibility: function() { | ||
this.set('_visibilityHandler', this.get('visibilityHandler').bind(this)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess I have the same question about nested routes as I do with multiple components - do outer routes have their handler overwritten by child routes here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, independent handlers again. |
||
document.addEventListener('visibilitychange', this.get('_visibilityHandler')); | ||
}.on('activate'), | ||
|
||
removeDocumentVisibility: function() { | ||
document.removeEventListener('visibilitychange', this.get('_visibilityHandler')); | ||
}.on('deactivate'), | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,38 @@ | ||
import Mixin from '@ember/object/mixin'; | ||
import { computed } from '@ember/object'; | ||
import { assert } from '@ember/debug'; | ||
import WithVisibilityDetection from './with-route-visibility-detection'; | ||
|
||
export default Mixin.create({ | ||
export default Mixin.create(WithVisibilityDetection, { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mixin the mixin 🐢🐢🐢 |
||
watchers: computed(() => []), | ||
|
||
cancelAllWatchers() { | ||
this.get('watchers').forEach(watcher => { | ||
assert('Watchers must be Ember Concurrency Tasks.', !!watcher.cancelAll); | ||
watcher.cancelAll(); | ||
}); | ||
}, | ||
|
||
startWatchers() { | ||
assert('startWatchers needs to be overridden in the Route', false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❤ little usage helpers like this |
||
}, | ||
|
||
setupController() { | ||
this.startWatchers(...arguments); | ||
return this._super(...arguments); | ||
}, | ||
|
||
visibilityHandler() { | ||
if (document.hidden) { | ||
this.cancelAllWatchers(); | ||
} else { | ||
this.startWatchers(this.controller, this.controller.get('model')); | ||
} | ||
}, | ||
|
||
actions: { | ||
willTransition() { | ||
this.get('watchers').forEach(watcher => { | ||
assert('Watchers must be Ember Concurrency Tasks.', !!watcher.cancelAll); | ||
watcher.cancelAll(); | ||
}); | ||
this.cancelAllWatchers(); | ||
}, | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you ever have > 1 component with this mixin on a screen? Would you have a last one wins situation here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all components (or routes) would have their handlers run, right? Which is likely the desired behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes to both! Job row and node row both mix this mixin in, so multiple components. Yes all components own their own handler and independently add and remove their own handlers. This prevents accidental side effects when paginating or sorting resulting in one component being destroyed and bringing down all the visibility handlers.