Skip to content
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

Allow programmatic control of observation #397

Merged
merged 2 commits into from
Jun 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions addon/modifiers/did-intersect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions tests/dummy/app/controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ export default class IndexController extends Controller {

maxIntersections = 5;

@tracked
isObserving = true;

@tracked
numIntersectionsWithIsObserving = 0;

@tracked
shouldScrollIntoView = false;

Expand All @@ -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;
Expand Down
14 changes: 7 additions & 7 deletions tests/dummy/app/styles/app.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 20 additions & 1 deletion tests/dummy/app/templates/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,31 @@
</demo.example>
{{demo.snippet "did-intersect-with-max-enter.hbs" label="example with max enter.hbs"}}

<demo.example @name="did-intersect-with-is-observing.hbs">
<div
class="intersection-box"
{{did-intersect
isObserving=this.isObserving
onEnter=this.onEnteringWithIsObserving
}}
>
<p>Number of times this box has intersected the screen: {{this.numIntersectionsWithIsObserving}}</p>

<p>isObserving: {{this.isObserving}}</p>

<button type='button' class="action-button" {{on "click" this.toggleIsObserving}}>
{{if this.isObserving "Stop" "Start"}} observing
</button>
</div>
</demo.example>
{{demo.snippet "did-intersect-with-is-observing.hbs" label="example with isObserving.hbs"}}

<demo.example @name="scroll-into-view.hbs">
<div
class="scroll-into-view"
{{scroll-into-view shouldScroll=this.shouldScrollIntoView}}
>
<button type="button" class="scroll-into-view__button" {{on "click" this.onScrollIntoViewTrigger}}>
<button type="button" class="action-button" {{on "click" this.onScrollIntoViewTrigger}}>
Trigger Scroll-into-view on click
</button>
</div>
Expand Down
58 changes: 54 additions & 4 deletions tests/integration/modifiers/did-intersect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
}

Expand Down Expand Up @@ -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);
Copy link
Owner

@elwayman02 elwayman02 Jun 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer not to use assert.expect for tests where the assertions are always called synchronously rather than in callbacks. This makes the tests brittle to change without adding a lot of value, in my experience.

(Yes, I know a few of those snuck through above. I missed them in previous reviews :( )

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can fix this in another PR, since it needs to be updated everywhere.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #398

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I am 100% in agreement; I added it solely to match the existing tests I saw. 😅


await render(hbs`
<div
{{did-intersect
isObserving=true
onEnter=this.enterStub
onExit=this.exitStub
}}
></div>
`);

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`
<div
{{did-intersect
isObserving=false
onEnter=this.enterStub
onExit=this.exitStub
}}
></div>
`);

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');
});
});
});