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

[scroll-animations] Support series of scroll offsets #4912

Closed
mattgperry opened this issue Mar 31, 2020 · 17 comments
Closed

[scroll-animations] Support series of scroll offsets #4912

mattgperry opened this issue Mar 31, 2020 · 17 comments
Assignees
Labels

Comments

@mattgperry
Copy link

Inspired by the proposal to allow Element-based offsets, I propose we can simplify the startScrollOffset and endScrollOffset props into a single scrollOffsets array.

This would allow us to leverage KeyframeEffect's ability to accept more than two keyframes in an animation.

Example

The "Example 1. Reveal / Unreveal" example in the above ticket shows the definition of two ScrollTimelines, like this:

const revealTimeline = new ScrollTimeline({
  startScrollOffset: { target: image, edge: 'start', threshold: 0 },
  endScrollOffset: { target: image, edge: 'start', threshold: 100 },
});

const unrevealTimeline = new ScrollTimeline({
  startScrollOffset:{ target: image, edge: 'end', threshold: 100}
  endScrollOffset:{ target: image, edge: 'end', threshold:0}
});

To create one conceptually unique effect (fade in while on screen), we create two timelines, two effects, and two animations.

Instead, we can half this to just one, with a single scrollOffset array:

const timeline = new ScrollTimeline({
  source: scroller,
  scrollOffsets: [100, 200, 800, 900]
});

Taken from @majido's example, this would change the above to:

const timeline = new ScrollTimeline({
  scrollOffsets: [
     { target: image, edge: 'start', threshold: 0 },
     { target: image, edge: 'start', threshold: 100 },
     { target: image, edge: 'end', threshold: 100 }, 
     { target: image, edge: 'end', threshold: 0 }
  ]
});

const effect = new KeyframeEffect(
  image,
  { opacity: [0, 1, 1, 0]},
  { duration: 1000, fill: both }
);

const revealUnrevleaAnimation = new Animation(effect, timeline);
revealUnrevleaAnimation.play();
@majido
Copy link
Contributor

majido commented Mar 31, 2020

This is an interesting idea which I am in favor of. I can see how multiple scroll offsets can simplify the API and is more ergonomics due to the fact that it can be paired with multi-frame keyframe effects.

Follow the processing logic for keyframes here are some ideas on how to spec and implement this:

  • Consider single-entry offsets as being the "end"

  • We need to have the scroll offset in increasing order otherwise time can go negative in some sections which is almost definitely a bug (Issue [scroll-animations] End < Start offset can cause the ScrollTimeline to produce negative time #4351 but supercharged!). There are two options:

    1. require scroll offsets to be in increasing order and error if not otherwise. This is consistent and easy to debug behavior.
    2. allow scroll offsets to be provided in any order, sort them before using them to ensure the are in increasing offsets. This may work better with element-based offsets whose exact value may not be known before starting the animation.
  • Treat these scroll offsets as equal-distanced in time. This is similar to having keyframes with no offset on them. See compute missing keyframes offsets in web-animations for how this works.

    • Note: While keyframes may have an additional offsets specified on them but it is not clear if we should allow these for scrollOffsets. The additional complexity is not justified at the moment and we can add it later. To this end I like the name scrollOffsets instead of offsets since it allows adding the offset to each entry in the future.
  • Allow static and element-based offset to be mixed in the scrollOffsets array.

/cc @flackr @ogerchikov @birtles for awareness and in case they have suggestions here.

@birtles
Copy link
Contributor

birtles commented Apr 9, 2020

This makes sense. I think treating the scroll offsets as equidistant in time would mesh well with the proposal for an auto duration for scroll timelines (#4862 / #4890). I haven't really though through the mixing static and element-based offsets however.

@ogerchikov
Copy link
Collaborator

This comment summarizes offline discussion between @majido and @ogerchikov regarding handling unsorted offsets.

Below are presented options:

  1. Sort offsets on every current time calculation.
  2. Throw if offsets are in wrong order.
  3. Produce unresolved time if offsets are in wrong order.
  4. Define heuristics for keeping only sorted values and ignore unsorted.

We feel that options 1 and 4 are hard to reason about and debug, option 2 is not practical since layout can change.

For initial implementation we feel that option 3 is the most desirable as it lets unsorted offsets to exist and resolved current time to be produced once/if the offsets become sorted. It also allows for future modifications towards options 1 or 4 if practical needs arise.

@majido
Copy link
Contributor

majido commented Jun 25, 2020

This was discussed in last F2F sync.

@birtles suggested an interesting approach based on clamping the offending values. The clamp value could be a middle value and neither of the two offending value. This can lead to an outcome that is closer to author intent.

Consider this example offsets: [100, 300, 200, 400]. Here 300 and 200 are in the invalid order so we replace them with 250 so the calculate offsets will be [100, 250, 250 400].

In the context of reveal/unreveal example a realistic case is when the item we want to reveal is larger than the viewport which would produce such a situation. The suggested clamping approach will cause the reveal to continue for 50% of total animation and unreveal to start to the remainder. While we never reach opacity 1 but it seems a better option than alternatives.

The consensus was that this looks like a promising approach instead of returning unresolved time.

Action Items:

  • @ogerchikov is planning to spec this in more details
  • @flackr was considering to prototype this in the polyfill to see how it will behave in practice.

@flackr
Copy link
Contributor

flackr commented Jun 25, 2020

Just a correction, my understanding is that we would in fact reach opacity 1 at exactly scroll offset 250. It's just that the time to get to opacity 1 would be only 150px instead of the requested 200px on either end.

@birtles
Copy link
Contributor

birtles commented Jun 26, 2020

My expectation is you would never get to opacity 1. That seems to me personally to be more in line with what the author intended rather than speeding up the effect.

@flackr
Copy link
Contributor

flackr commented Jun 30, 2020

Thanks for the clarification, I agree this is probably more in line with the developer intention, however I think this isn't equivalent to [100, 250, 250, 400], because it loses the information that 250 is only 2/3 of the way to the second / third keyframe.

Perhaps we should keep the conflicting scroll offsets in the calculated offsets and define a procedure for determining the current offset which applies this logic, e.g. something like

If offset i + 1 is less than offset i, find the first offset j which is greater than or equal to offset i. If the scroll offset falls between offset i and offset j then the resulting animation offset uses the pair [i, i + 1] or [j - 1, j] whichever of i or j is closer to the current position.

@majido
Copy link
Contributor

majido commented Jun 30, 2020

Earlier in this thread I speculated if we should allow "offsets" for these scrollOffset similar to how we have these on keyframes.

My conclusion then was that perhaps its complexity is not justified but I wonder if having offset makes achieving the desired clamping behavior without unnecessary speed up actually simpler.

Consider if we actually have implied offsets for the current example. So [100, 300, 200, 400] is actually interpreted as the following map: {0 => 100, 0.33 => 300, 0.66 => 200, 1 => 400}. Notice the implied input offsets are equi-distance in the case where no offset is provided by the author.

The logic for clamping then detects any series of offsets that don't match the expected order then it clamps both the values and their offsets to the middle point.

So in the example case it will turn it into {0 => 100, 0.5 => 250, 0.5 => 250, 1 => 400} (or even {0 => 100, 0.5 => 250, 1 => 400} if we eliminate duplicate entries for 0.5 => 250)

The advantages are:

  • Achieves the most preferred outcome where animations are not sped up.
  • Easier to reason about since the output of scroll timeline can easily be determined given the calculated offset.
  • More consistent with keyframes having offsets.
  • We can allow authors to provide offset for better ergonomics in more complex cases.

@majido
Copy link
Contributor

majido commented Jun 30, 2020

While at it perhaps it is also worth bikeshedding scrollOffsets name a bit more.

I think we should not include scroll in the name given that ScrollTimeline.scrollOffset does not read that well. Also we should not include offsets if we every plan to introduce offsets similar to Keyframes.

The only other good name that I have come up with so far is ScrollTimeline.thresholds where each threshold has a scroll position and an optional offset.

@flackr
Copy link
Contributor

flackr commented Jun 30, 2020

I'm confused, isn't the map we're trying to build scroll offset to animation offset, not the reverse?

My expectation would be that if we include animation offsets [100, 300, 200, 400] would produce {100 => 0, 250 => 0.22, 250 => 0.78, 400 => 1} where the 0.22 and 0.78 are the relative proportion of those interpolations that would be covered (e.g. 66% of the 33% animation progress achieved at offset 250). This clearly shows that we're skipping the entire animation between 22% and 78%.

If we strictly use the midpoint of the overlapping animation offsets we might in fact change the speed of the effect, consider:
[100, 300, 100, 400]
The midpoint is 150 with value 0.5. This means that at 151, we will be just over 51%, rather than the expected position of being well into the [100, 400] interpolation between keyframes 3 and 4.

@majido
Copy link
Contributor

majido commented Jun 30, 2020

@flackr you are right that I confused the offset calculation 🤦 . But your interpretation of it is correct. Basically adjust the computed offsets to maintain the speed.

BTW here is another edge case to consider when writing up the algorithm for this: [100, 500, 400].
There is at least two ways to handle this 1) nothing special about last value so we turn this into [100, 450, 450] or 2) honor the last value as the intended maximum of the range and produce [100, 400, 400]

@flackr
Copy link
Contributor

flackr commented Jul 15, 2020

We've been discussing alternate cases and have come to the conclusion that while the magic middle checkpoint makes a lot of sense for the one use case, it falls apart when you look at more complex use cases such as the one Majid mentioned above.

  1. animate({ opacity: [0, 1, 1, 0]}, {timeline: new ScrollTimeline({offsets: [100, 300, 200, 400]})) produces computed offsets [100 => 0, 250 => 0.25, 250 => 0.75, 400 => 1], which makes a lot of sense for this effect. However:
  2. animate({ opacity: [0, 0.5, 1]}, {fill: 'both', timeline: new ScrollTimeline({offsets: [100, 300, 200]})) produces computed offsets [100, 250, 250]? This is the case Majid brought up above, and it makes sense that the effect should end at the last scroll offset in the list.
  3. animate({ opacity: [0, 1, 0, 1, 0]}, {fill: 'both', timeline: new ScrollTimeline({offsets: [100, 400, 300, 200, 500]})) is a little uncertain what we should produce. I would assume that the 400 and the 200 are averaged to make 300, e.g. [100, 300 => 0.1667, 300 => 0.5, 300 => 0.8333, 500 => 1] however this is assuming that the animation from [300, 200] is unimportant as it is entirely clipped.
  4. animate({ opacity: [0, 1, 0, 1, 0]}, {fill: 'both', timeline: new ScrollTimeline({offsets: [100, 200, -1000, 500]})) is sort of the inverse of case 2. We should probably at least clamp the produced offsets to be within the first and last.

I think that there's two takeaways from the above:
A. The start and end offsets seem as though they should be respected, as the intermediate offsets extending the duration of the animation seems like it would be very surprising.
B. I think without knowing the developer intent, it may be surprising to construct offsets for the developer that they didn't specify. I think it would be easier for developers to understand a simpler clamping model where simply the first or last matching range are applied. My preference would be for the last since this is somewhat analogous to precedence rules or what would happen if you had specified the animation as a bunch of separate scroll animations in order.

@ogerchikov
Copy link
Collaborator

The only other good name that I have come up with so far is ScrollTimeline.thresholds where each threshold has a scroll position and an optional offset.

Naming scroll offsets threshold can be misleading since threshold already participates in element offset definition. How about ScrollTimeline.ranges instead?

@flackr
Copy link
Contributor

flackr commented Jul 22, 2020

I feel as though it might be better to stick with scrollOffsets, since it's completely clear what this means.

@majido
Copy link
Contributor

majido commented Jul 27, 2020

Thanks @ogerchikov for making me realize that the thresholds also has a conflict! On naming decision, I tried to summarized my thoughts based on latest suggestion.

My perspective is the the CSS syntax of scroll-timeline is the one that will be used more often so we should optimize for that if there is no good option for both. And so far nothing proposed is great in both JS and CSS.

With this premise, my initial concern about the property name conflicting with others is not as important since we don't spell out either offset or threshold in the CSS syntax. Instead the double "scroll" issues is more jarring in CSS syntax.

@scroll-timeline {
    scroll-offsets: 10% 90%;
}

Based on above my preference would be:

  1. offsets
  2. thresholds
  3. scrollOffsets
  4. ranges

Another alternative is to have different names in css and js but I like to avoid that.

Here are all the suggested options in multiple real examples in JS and CSS with my opinion on pros and cons.

offsets

Pros:

  • Avoids double "scroll" in the name issue. (Mainly an issue CSS syntax)

Cons:

  • Possibly confusing if element-based scroll offsets have threshold of their own. (Only an issue in JS syntax)
  • Not immediately obvious what is the value unit i.e., scroll dimensions.
  const scrollTimeline = new ScrollTimeline({
      source: scroller,
      offsets: [CSS.percent(20), CSS.percent(50), CSS.percent(80)]
  });

  const scrollTimelineWithOffset = new ScrollTimeline({
      source: scroller,
      offsets: [{value: CSS.percent(20), offset: 0} , {value: CSS.percent(50), offset: 0.5}, {value: CSS.percent(80), offset:1}]
  });

  const scrollTimelineWithElements = new ScrollTimeline({
      source: scroller,
      offsets: [{target: image, threshold: 0}, {target: image, threshold: 0.8}, {target: image, threshold: 1}]
  });

  console.log(scrollTimeline.offsets);
@scroll-timeline basic {
  source: selector(#scroller);
  offsets: 20, 50, 80; 
}


@scroll-timeline with-offsets {
  source: selector(#scroller);
  offsets: 20 0%, 50 50%, 80 100%; 
}

@scroll-timeline with-elements {
  source: selector(#scroller);
  offsets: selector(#image) 0, selector(#image) 0.8, selector(#image) 1; 
}

scrollOffsets

Pros:

  • Immediately obvious what is the value unit i.e., scroll dimensions.

Cons:

  • Double "scroll" which is more jarring in css syntax. (Mainly an issue in CSS syntax)
  • Possibly confusing if scroll offsets have offsets of their own. (Only an issue in JS syntax)
  const scrollTimeline = new ScrollTimeline({
      source: scroller,
      scrollOffsets: [CSS.percent(20), CSS.percent(50), CSS.percent(80)]
  });

  const scrollTimelineWithOffset = new ScrollTimeline({
      source: scroller,
      scrollOffsets: [{value: CSS.percent(20), offset: 0} , {value: CSS.percent(50), offset: 0.5}, {value: CSS.percent(80), offset:1}]
  });

  const scrollTimelineWithElements = new ScrollTimeline({
      source: scroller,
      scrollOffsets: [{target: image, threshold: 0}, {target: image, threshold: 0.8}, {target: image, threshold: 1}]
  });

  console.log(scrollTimeline.scrollOffsets);
@scroll-timeline basic {
  source: selector(#scroller);
  scroll-offsets: 20, 50, 80; 
}


@scroll-timeline with-offsets {
  source: selector(#scroller);
  scroll-offsets: 20 0%, 50 50%, 80 100%; 
}

@scroll-timeline with-elements {
  source: selector(#scroller);
  scroll-offsets: selector(#image) 0, selector(#image) 0.8, selector(#image) 1; 
}

thresholds

Pros:

  • Avoids double "scroll" in the name issue. (Mainly an issue in CSS syntax)

Cons:

  • Possibly confusing if element-based scroll offsets have threshold of their own. (Only an issue in JS syntax)
  • Not immediately obvious what is the value unit (i.e., scroll dimensions).
  const scrollTimeline = new ScrollTimeline({
      source: scroller,
      thresholds: [CSS.percent(20), CSS.percent(50), CSS.percent(80)]
  });

  const scrollTimelineWithOffset = new ScrollTimeline({
      source: scroller,
      thresholds: [{value: CSS.percent(20), offset: 0} , {value: CSS.percent(50), offset: 0.5}, {value: CSS.percent(80), offset:1}]
  });

  const scrollTimelineWithElements = new ScrollTimeline({
      source: scroller,
      thresholds: [{target: image, threshold: 0}, {target: image, threshold: 0.8}, {target: image, threshold: 1}]
  });

  console.log(scrollTimeline.thresholds);
@scroll-timeline basic {
  source: selector(#scroller);
  thresholds: 20, 50, 80; 
}


@scroll-timeline with-offsets {
  source: selector(#scroller);
  thresholds: 20 0%, 50 50%, 80 100%; 
}

@scroll-timeline with-elements {
  source: selector(#scroller);
  thresholds: selector(#image) 0, selector(#image) 0.8, selector(#image) 1; 
}

ranges

Pros:

  • No conflict with threshold or offset. (Only an issue in JS syntax)
  • Avoids double scroll in the name issue. (Mainly an issue in CSS syntax)

Cons:

  • IMHO not semantically correct as we are not specifying ranges.
  • Not immediately obvious what is the value unit i.e., scroll dimensions.
  const scrollTimeline = new ScrollTimeline({
      source: scroller,
      ranges: [CSS.percent(20), CSS.percent(50), CSS.percent(80)]
  });

  const scrollTimelineWithOffset = new ScrollTimeline({
      source: scroller,
      ranges: [{value: CSS.percent(20), offset: 0} , {value: CSS.percent(50), offset: 0.5}, {value: CSS.percent(80), offset:1}]
  });

  const scrollTimelineWithElements = new ScrollTimeline({
      source: scroller,
      ranges: [{target: image, threshold: 0}, {target: image, threshold: 0.8}, {target: image, threshold: 1}]
  });

  console.log(scrollTimeline.thresholds);
@scroll-timeline basic {
  source: selector(#scroller);
  ranges: 20, 50, 80; 
}


@scroll-timeline with-offsets {
  source: selector(#scroller);
  ranges: 20 0%, 50 50%, 80 100%; 
}

@scroll-timeline with-elements {
  source: selector(#scroller);
  ranges: selector(#image) 0, selector(#image) 0.8, selector(#image) 1; 
}

ogerchikov added a commit that referenced this issue Feb 2, 2021
* Initial revision

* Fixed formatting.

* Addressed review comments.

* Addressed review comments.
@bramus
Copy link
Contributor

bramus commented Apr 22, 2021

If I'm not mistaken this issue may be closed as #5803 got merged. Or are there any other outstanding things that need to be tackled?

On a side note: the implementation in Chromium just caught up with the changes outlined in #5803 :) — https://bugs.chromium.org/p/chromium/issues/detail?id=1094014#c9

@mattgperry
Copy link
Author

Agreed, that looks great, thanks for the heads up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants