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.

Sign up for GitHub

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

Jump to bottom

Support CSSNumericValues as inset #177

Merged
merged 3 commits into from
Dec 21, 2023
Merged

Support CSSNumericValues as inset #177

merged 3 commits into from
Dec 21, 2023

Conversation

johannesodland
Copy link
Contributor

@johannesodland johannesodland commented Nov 25, 2023 •
edited
Loading

Depends on #176

This PR adds support for passing CSSNumericValues as insets to ViewTimeline.

new ViewTimeline({
  subject: elm,
  axis: 'block',
  inset: [ CSS.percent(10), CSS.percent(20) ]
})

To pass the tests the PR also do the following:

  • Adds MutationObserver to observe changes to class attribute on subject.
  • Adds getters for startOffset and endOffset read only attributes.
  • Adds support for parsing math functions such as calc, through a naive implementation of CSSNumericValue.parse()

This PR does not:

  • add support for relative units such as viewport relative units.

This was referenced Nov 25, 2023
Merged
Closed
Closed
@johannesodland johannesodland force-pushed the css-unit-value-inset branch 3 times, most recently from ab45fa7 to ea5f3f7 Compare December 21, 2023 15:08
@johannesodland johannesodland marked this pull request as ready for review December 21, 2023 15:32
@johannesodland johannesodland changed the title Support CSSUnitValue as inset Support CSSUnitValue and math functions as inset Dec 21, 2023
@johannesodland johannesodland changed the title Support CSSUnitValue and math functions as inset Support CSSUnitValues as inset Dec 21, 2023
@johannesodland johannesodland changed the title Support CSSUnitValues as inset Support CSSNumericValues as inset Dec 21, 2023
@flackr
Copy link
Owner

flackr commented Dec 21, 2023

Uhoh, looks like my naive diff of test results approach has encountered it's first issue with what seems to be a flaky test.

@flackr flackr merged commit 61320c7 into flackr:master Dec 21, 2023
1 of 2 checks passed
tf added a commit to tf/scroll-timeline that referenced this pull request Jan 8, 2024
@tf
Prevent invalid-regular expression syntax error in Safari < 16.4
79c488f 
PR flackr#177 added regular expressions to parse CSSNumericValue objects
using lookbehind. Safari only supports lookbehind starting from
version 16.4 [1]. In earlier versions loading the polyfill causes an
error of the form:

```
SyntaxError: Invalid regular expression: invalid group specifier name
```

Compile RegExp on demand to ensure the polyfill can be loaded without
errors. Since the relevant functions are only called initially when
parsing options, the performance overhead appears limited.

[1] https://caniuse.com/js-regexp-lookbehind
@tf tf mentioned this pull request Jan 8, 2024
Closed
tf added a commit to tf/scroll-timeline that referenced this pull request Jan 8, 2024
@tf
Prevent invalid-regular expression syntax error in Safari < 16.4
153ed40 
PR flackr#177 added regular expressions to parse CSSNumericValue objects
using lookbehind. Safari only supports lookbehind starting from
version 16.4 [1]. In earlier versions loading the polyfill causes an
error of the form:

```
SyntaxError: Invalid regular expression: invalid group specifier name
```

Compile RegExp on demand to ensure the polyfill can be loaded without
errors. Since the relevant functions are only called initially when
parsing options, the performance overhead appears limited.

[1] https://caniuse.com/js-regexp-lookbehind
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flackr flackr flackr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@johannesodland @flackr