elwayman02/ember-scroll-modifiers

Scroll-based Modifiers for Ember.js Applications

ember-scroll-modifiers's Issues

New scroll-by modifier

Description
Adding a new scroll-by functional modifier that mimics the native scrollBy.

API
The arguments are chosen to closely mimc the API of the native scrollBy that supports an option dictionary. We will also introduce a boolean/promise to conditionally decide when to trigger the scrollBy.

Examples

<div {{scroll-by options=this.scrollOptions shouldScroll=this.shouldScroll}}></div>

Export cleanup method for did-intersect mock

The test support util we created for did-intersect doesn't have a way to restore the mock after a test: https://github.com/elwayman02/ember-scroll-modifiers/blob/master/addon-test-support/did-intersect-mock.js

We should export a method that makes it easy to clean up after the tests and avoid leaking state.

We do something similar here: https://github.com/elwayman02/ember-scroll-modifiers/blob/master/addon-test-support/scroll-into-view-mock.js#L24

cc: @ygongdev

Investigate why the demo doesn't work on mobile browsers

did-intersect-mock doesn't carry boundingClientRect information

When we do onEnter/onExit, it will yield a IntersectionObserverEntry which contains boundingClientRect information.

When we use mockDidIntersect for testing, the boundingClientRect information is not added

Add willRemove test

Use clearRender

https://github.com/emberjs/ember-test-helpers/blob/master/API.md#clearrender

Figure out why extra events are fired for did-intersect

The IntersectionObserver fires an event immediately after it starts observing, even though no user activity has taken place and there's no intersection in progress. It also appears to fire two or three times instead of once, whenever the threshold is crossed.

Move documentation from e-c-addon-docs to field-guide

e-c-addon-docs is more or less unmaintained, so let's move our docs to the future and use field-guide instead. Here's an example of a similar addon that uses field-guide:

https://github.com/elwayman02/ember-resize-modifier/tree/master/docs

Setup should be fairly simple - Copy the markdown files from e-c-addon-docs to the location field-guide expects them to be and delete any e-c-addon-docs code from the demo app.

Address deprecations to prepare for Ember 4.0

Our deprecation workflow silences deprecation warnings in console output, but we still need to address these warnings in order to eventually upgrade to Ember 4.0, where most of these issues will become errors. Some of these deprecations come from ember-cli-addon-docs and don't impact the code we ship to consumers; they may be addressed by resolving #359.

Many of the Ember deprecations are detailed here: https://deprecations.emberjs.com/v3.x/

• deprecated-run-loop-and-computed-dot-access
• ember.built-in-components.import
• ember.built-in-components.legacy-arguments
• ember.built-in-components.reopen
• ember-glimmer.link-to.positional-arguments
• ember-keyboard.first-responder-inputs
• manager-capabilities.modifiers-3-13
• this-property-fallback

Close this ticket when the Ember 4.0 upgrade lands and all deprecations have been addressed. Future deprecations will be tracked separately.

Remove `assert.expect` from tests

Nearly all of our tests call assertions synchronously - that is, they aren't wrapped in callbacks or other scenarios where it's possible that one or more of the assertions won't get called. As a result, using assert.expect becomes unnecessary, since we'll never have a scenario where only some of the assertions get run. We should remove assert.expect from all tests of this nature to make our tests less brittle to future changes.

intersection-observer-admin does not support options from (hash) helper

Right now {{hash }} can't be used to pass options into intersection-observer-admin because the currently released version does use hasOwnProperty on the options object (which is wrong).

There is a fix in unreleased code on the master branch, see snewcomer/intersection-observer-admin@aabb579

I made an issue there to keep track of this, see snewcomer/intersection-observer-admin#23.

For others running into this error, you can pass in a regular object from your controller/component class.

New scroll-into-view modifier

Description
Adding a new scroll-into-view functional modifier that mimics the native scrollIntoView.

API
The arguments are chosen to closely mimc the API of the native scrollIntoView, which accepts an object parameter. We also introduce a boolean/promise to conditionally trigger the scrollIntoView functionality.

Examples

<div {{scroll-into-view options=this.scrollOptions shouldScroll=this.shouldScroll}}></div>

Update to/support Ember v5

Ember v5 is out! :) It would be nice to update to it and/or add support for it in peerDependencies.

Demo site broken by Ember 3.27

emberjs/ember.js#19538

Published package contains the entire yarn cache

The currently published version of this package (https://registry.npmjs.org/ember-scroll-modifiers/-/ember-scroll-modifiers-7.0.0.tgz) contains a .yarn directory which bloats the archive size to over 64 MiB.

To fix that, update this repo's .gitignore file as per the yarn docs and publish a new version of the package without the bloat 🙂

Needs meaningful tests

How can we test and/or mock IntersectionObserver?

Fix ember-modifier deprecations

The ember-modifer.* deprecations should be fixed instead of silenced.

Feature suggestion: programmatic control over observation

Right now, the modifier essentially assumes that you are always observing if installed on an element, modulo the maxEnter and maxExit arguments. However, there are scenarios where it's important to have programmatic control.

There are basically two options here:

1. Add the ability to control observation via an argument, isObserving or some such.
2. Wait until support for {{modifier ...}} arrives and let people do conditional installation.

While (2) is preferable in a lot of ways, it's unclear when it will be possible—it certainly won't be during the Ember 3.x release cycle. That means that for consumers who use LTS releases, we are the better part of a year away from being able to do it at earliest. Assume 3.28 is the last of the 3.x series, which is the earliest possible cutoff; then the first 4.x LTS (v4.4), which would become an LTS in March 2020 or so. If 3.29 or 3.30 ends up being the last release in 3.x, that pushes it back 6 or 12 weeks further.

I'm happy to open a PR which supports this on both the 1.x and 2.x branches (the app I work on won't be on 3.26+ for probably another 3+ months, as we're still working our way onto 3.24 LTS and will start the 3.28 LTS after it reaches LTS sometime next quarter).

New scroll-position modifier

Description
Adding a new scroll-position functional modifier that mimics the native scrollTop and scrollBottom.

API

1. top
2. left

Each API will take an numerical value or a callback that exposes the element as its first argument, e.g callback(element). This allows interacting with this.element for more flexible implementation like scrollBottom or scrollRight, if needed.

Examples
1.

<div {{scroll-position top=12 left=12}}></div>

<div {{scroll-position top=this.scrollTopCallback}}></div>

Perf comparison for docs

https://itnext.io/1v1-scroll-listener-vs-intersection-observers-469a26ab9eb6

ember-modifier v4

We need to upgrade the package to v4 and cut a breaking release

Drop decorators polyfill

This can be removed when we drop support for Ember 3.8

Restore interactive demo

We had one in v3.1.1, but removed it when moving to field-guide. We just need to re-add some interactive demos in the new docs.

Support nested scroll containers when using offset

Nested scroll containers are supported with scrollIntoView as it takes the element's ancestor containers. When using offset the modifier falls back to scrollTo which always scroll the window and so doesn't work when having nested scroll containers.

Allowing to pass a custom scroll container to use instead of window should do the trick.

Ideas for other modifiers

What other scroll-based modifiers would you like to see added?

The obvious one is did-scroll - what other contextual interactions would be useful? Maybe modifier that is tailored to generating impressions? e.g. when an element scrolls into the visible viewport, it calls the handler to note that an impression was registered, configurable with different % visibility thresholds.

Embroider builds failing due to did-intersect-mock

            TypeError: Attempted to replace instances which is already replaced
                at eval (webpack://dummy/../../sinon/lib/sinon/sandbox.js?:166:15)
                at Array.forEach (<anonymous>)
                at verifyNotReplaced (webpack://dummy/../../sinon/lib/sinon/sandbox.js?:164:5)
                at Sandbox.replace (webpack://dummy/../../sinon/lib/sinon/sandbox.js?:187:5)
                at mockDidIntersect (webpack://dummy/../rewritten-packages/ember-scroll-modifiers.e10ccf07/test-support/did-intersect-mock.js?:122:9)
                at Object.eval (webpack://dummy/./tests/integration/modifiers/did-intersect-mock-test.js?:20:127)

[did-intersect] Export addon-test-support helpers

The async nature of Intersection Observer makes it hard to test deterministically in the Ember ecosystem.

We should implement a set of helpers around a deterministic Intersection observer mock and export them for the consumers to use.

Fix lerna integration

It appears to no longer be generating the changelog correctly.

Fix docs page

[#321] broke the doc site when it did the major version bump. The landing page works, but the actual docs won't load.

If the fix is easy, we could just fix it and move on (I'm not sure what's broken).

Otherwise, the longer term fix is that we should migrate to use field-guide like ember-resize-modifier.

Drop IE11 Support

Once we're very far from needing IE11 support, cut a breaking release that drops IE11 support by no longer checking to see if IntersectionObserver exists and just assuming it's there in every browser.

Support only running once

It's fairly common to want to run a callback the first time an item becomes visible, and not again after that. We could perhaps support something like once=true as an argument here. Very open to design discussion around the right way to do this, though; the caller can also simply cache a value (which is what I'm going to do to work around the absence of this for now).

New scroll-to modifier

Description
Adding a new scroll-to functional modifier that mimics the native scrollTo.
It is possible that we only want to perform a scroll-to if a certain condition is satisfied, so this modifier will optionally take a boolean to know when to trigger its scroll-to

API
The arguments are chosen to closely mimc the API of the native scrollTo, which supports 2 positional arguments or a named option. We just add one more API to support the optional boolean.

Positional arguments: [top, left, shouldScroll] or scrollOptions
Named arguments: { top, left, behavior, shouldScroll }

Examples
Debating between explicit positional arguments and a single positional object containing the config, which gives more flexibility if the native scrollOptions API happens to change.

<div {{scroll-to 100 100 true}}></div>

vs

<div {{scroll-to scrollOptions true}}></div>

Named

<div {{scroll-to top=this.top left=this.left behavior=this.behavior shouldScroll=this.shouldScroll}}></div>

Accept options hash instead of just threshold as second argument

Use intersection-observer-admin?

https://github.com/snewcomer/intersection-observer-admin

This can help manage IOs across an app more performantly

Move to GitHub Actions

Now that travis has limited free build time, we need to move to GitHub Actions.

Example: https://github.com/elwayman02/ember-user-activity/blob/master/.github/workflows/ci.yml

Known limitation: No "allowed failure" config yet - actions/runner#2347

Indeterministic race condition between `onEnter` trigger and `click` on load more button

Hi Team,

Context

We have used {{did-intersect}} modifier in a load more button, and it triggers this.loadMore on onEnter and click.

<Container>
  <LoadMoreButton {{did-intersect onEnter=this.loadMore}} {{on "click" this.loadMore}} />
  <MessageList />
</Container>

Issue

When the component was tested, the <LoadMoreButton /> was initially outside the viewport of the <Container />. But when we run

await click(LoadMoreButton);

the <LoadMoreButton /> was scrolled into view, triggering onEnter=this.loadMore and {{on "click" this.loadMore}}.

Problem

In some cases (usually in CLI mode), this.loadMore.callCount === 2, sometimes it's this.loadMore.callCount === 1.

Current Workaround

1. We can pass 2 different methods that do loadMore but that's mixing tests into app code, which is not ideal because that was needed only to pass a test.

  <LoadMoreButton {{did-intersect onEnter=this.loadMoreOnScroll}} {{on "click" this.loadMoreOnClick}} />

2. We have tried the following, but the click action seems to have called loadMore twice.

await click(LoadMoreButton);
this.loadMore.resetHistory(); // if the click action pulls the <LoadMoreButton /> into view, we reset the callCount

await click(LoadMoreButton);
assert.equal(this.loadMore.callCount, 1);

Asks

I have read the integration tests for ember-scroll-modifier but the services were stub, so I couldn't find a case like this. Could you advise on how we want to make sure the following test result?

assert.equal(this.loadMore.callCount, 2, 'it triggers loadMore on onEnter and click');