search

flackr / scroll-timeline

A polyfill of ScrollTimeline.
Apache License 2.0
951 stars 94 forks source link

Only return 'normal' as the range when no value is given. #185

Closed bramus closed 9 months ago

bramus commented 9 months ago

In #178 parseAnimationRange was changed to use string values of "normal" instead of objects representing the normal range. This essentially broke the parsing of animation-range, as later on that function tries to update values in that (now inexistent) object.

On the console, these errors could be seen, as reported in #183:

Can’t assign to property "rangeName": not an object

This PR fixes this: return the default "normal" when no value is given, but continue with the objects when there is a value to parse.

Note that this is only for parsing ranges of ViewTimelime instances. For ScrollTimeline, more work needs to be done in #153.

PS: I didn’t get to updating the test results as I’ve got issues launching them with firefox for some reason. Safari works fine, but gives totally different base results.

johannesodland commented 9 months ago

I'm so sorry.

I think it might be better to just roll back https://github.com/flackr/scroll-timeline/pull/178.

I was too optimistic when rebasing on top of the recent changes, and leaned too heavily on the WPT test result being ok.

If not rolled back, #187 should probably be merged as well, as there is an issue matching the range names entry-crossing and exit-crossing.

bramus commented 9 months ago

I’ll get this one and #178 in, as #178 contained some other good changes.

In #153 the range parsing will undergo a rewrite, so I think the current code is fine – it will get refactored anyways.