From a70d1cd4ed2efb1acabc868d7aba77d53ad7d212 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Thu, 24 Jun 2021 10:48:52 -0600 Subject: [PATCH 1/2] Allow programmatic control of observation Add a named new argument, `isObserving`, with a default value of `true`, which users can pass to explicitly opt into or out of observing an element at a given time. This allows for a declarative invocation of the modifier itself based on tracked state, rather than having to handle it in the callback(s) passed to the modifier. Users can work around this today by only passing the `onEnter` or `onExit` callbacks conditionally:
However, as this example shows, this is repetive (and potentially error- prone). The new functionality allows a less error-prone, less repetitive, more expressive approach:
Resolves #396 --- addon/modifiers/did-intersect.js | 15 ++++- .../modifiers/did-intersect-test.js | 58 +++++++++++++++++-- 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/addon/modifiers/did-intersect.js b/addon/modifiers/did-intersect.js index 742f424c..4e899ea2 100644 --- a/addon/modifiers/did-intersect.js +++ b/addon/modifiers/did-intersect.js @@ -77,7 +77,14 @@ export default class DidIntersectModifier extends Modifier { return; } - let { onEnter, onExit, maxEnter, maxExit, options } = this.args.named; + let { + onEnter, + onExit, + maxEnter, + maxExit, + options, + isObserving = true, + } = this.args.named; assert('onEnter or/and onExit is required', onEnter || onExit); @@ -161,7 +168,11 @@ export default class DidIntersectModifier extends Modifier { this._options = options; } - this.observe(); + if (isObserving) { + this.observe(); + } else { + this.unobserve(); + } } // Move to willDestroy when we want to drop support for versions below ember-modifier 2.x diff --git a/tests/integration/modifiers/did-intersect-test.js b/tests/integration/modifiers/did-intersect-test.js index df36bcc0..e32888eb 100644 --- a/tests/integration/modifiers/did-intersect-test.js +++ b/tests/integration/modifiers/did-intersect-test.js @@ -16,13 +16,21 @@ module('Integration | Modifier | did-intersect', function (hooks) { this._admin = {}; } - observe = sinon.stub(); - unobserve = sinon.stub(); + observe = sinon.stub().callsFake(() => { + this.isObserving = true; + }); + unobserve = sinon.stub().callsFake(() => { + this.isObserving = false; + }); addEnterCallback = sinon.stub().callsFake((element, callback) => { - this.onEnterCallback = sinon.spy(callback); + this.onEnterCallback = sinon.spy(() => { + if (this.isObserving) callback(); + }); }); addExitCallback = sinon.stub().callsFake((element, callback) => { - this.onExitCallback = sinon.spy(callback); + this.onExitCallback = sinon.spy(() => { + if (this.isObserving) callback(); + }); }); } @@ -344,4 +352,46 @@ module('Integration | Modifier | did-intersect', function (hooks) { assert.equal(this.newExitStub.callCount, 1, 'new exit callback is called'); }); + + module('modifier accepts `isObserving` argument', function () { + test('with a truth(y) value', async function (assert) { + assert.expect(2); + + await render(hbs` +
+ `); + + this.observerManagerMock.onEnterCallback(); + this.observerManagerMock.onExitCallback(); + + assert.ok(this.enterStub.calledOnce, 'the enter callback is invoked'); + assert.ok(this.exitStub.calledOnce, 'the onExit callback is invoked'); + }); + + test('with a false(y) value', async function (assert) { + assert.expect(2); + + await render(hbs` +
+ `); + + this.observerManagerMock.onEnterCallback(); + this.observerManagerMock.onExitCallback(); + + assert.ok(this.enterStub.notCalled, 'the enter callback is not invoked'); + assert.ok(this.exitStub.notCalled, 'the onExit callback is not invoked'); + }); + }); }); From 220c6d0511665f2d6f28777b0d76ba6ebd9e9793 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Thu, 24 Jun 2021 19:13:48 -0600 Subject: [PATCH 2/2] Demo the `isObserving` functionality --- tests/dummy/app/controllers/index.js | 16 ++++++++++++++++ tests/dummy/app/styles/app.scss | 14 +++++++------- tests/dummy/app/templates/index.hbs | 21 ++++++++++++++++++++- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/tests/dummy/app/controllers/index.js b/tests/dummy/app/controllers/index.js index 2ca1f6fa..9075ad1e 100644 --- a/tests/dummy/app/controllers/index.js +++ b/tests/dummy/app/controllers/index.js @@ -11,6 +11,12 @@ export default class IndexController extends Controller { maxIntersections = 5; + @tracked + isObserving = true; + + @tracked + numIntersectionsWithIsObserving = 0; + @tracked shouldScrollIntoView = false; @@ -24,6 +30,16 @@ export default class IndexController extends Controller { this.numIntersectionsWithMaxEnter++; } + @action + toggleIsObserving() { + this.isObserving = !this.isObserving; + } + + @action + onEnteringWithIsObserving() { + this.numIntersectionsWithIsObserving++; + } + @action onScrollIntoViewTrigger() { this.shouldScrollIntoView = true; diff --git a/tests/dummy/app/styles/app.scss b/tests/dummy/app/styles/app.scss index 91f47ccb..30cc92fb 100644 --- a/tests/dummy/app/styles/app.scss +++ b/tests/dummy/app/styles/app.scss @@ -9,17 +9,17 @@ } } +.action-button { + border-radius: 4px; + padding: 0.5em; + background-color: orangered; + color: white; +} + .scroll-into-view { border: 1px dotted black; padding: 50px 16px; text-align: center; - - button { - border-radius: 4px; - padding: .5em; - background-color: orangered; - color: white; - } } // prevent copy button from overlapping code diff --git a/tests/dummy/app/templates/index.hbs b/tests/dummy/app/templates/index.hbs index 6db2f557..3afe07d2 100644 --- a/tests/dummy/app/templates/index.hbs +++ b/tests/dummy/app/templates/index.hbs @@ -24,12 +24,31 @@ {{demo.snippet "did-intersect-with-max-enter.hbs" label="example with max enter.hbs"}} + +
+

Number of times this box has intersected the screen: {{this.numIntersectionsWithIsObserving}}

+ +

isObserving: {{this.isObserving}}

+ + +
+
+ {{demo.snippet "did-intersect-with-is-observing.hbs" label="example with isObserving.hbs"}} +
-