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

[scroll-animations-1] Use of DOMString in API surface #5213

Closed
plinss opened this issue Jun 16, 2020 · 1 comment · Fixed by #5300
Closed

[scroll-animations-1] Use of DOMString in API surface #5213

plinss opened this issue Jun 16, 2020 · 1 comment · Fixed by #5300
Assignees
@majido
Labels
scroll-animations-1 Current Work
Milestone
scroll-animations...

Comments

@plinss
Copy link
Member

plinss commented Jun 16, 2020

Small point that came up in the TAG Review, the ScrollTimeline interface is currently using DOMString to represent keyword and length/percentage values. This should use the appropriate CSS Typed OM classes instead.

Using strings for CSS values is a historical artifact we're trying to eliminate, new APIs shouldn't use them.
@majido majido added the scroll-animations-1 Current Work label Jun 24, 2020
@majido
Copy link
Contributor

majido commented Jun 24, 2020

Thanks. This has been something that was suggested before as well by @andruud in #4352 as well.
Given TAG's recommendation here we should go ahead and make this change. I also like Tab's suggestion to treat strings implicitly as keyword.

@majido majido added this to the scroll-animations-1 FPWD milestone Jun 24, 2020
@majido majido self-assigned this Jun 24, 2020
majido added a commit to majido/csswg-drafts that referenced this issue Jul 8, 2020
@majido
[scroll-animations] Use CSSTypedOM instead of strings (w3c#5213)
d6fe7b0 
Fixes w3c#5213
@majido majido mentioned this issue Jul 8, 2020
Merged
majido added a commit that referenced this issue Jul 9, 2020
@majido
[scroll-animations-1] Use CSSTypedOM instead of strings (#5213) (#5300)
44bca0a 
Use appropriate CSSTypedOM instead of DOMStrings in the API. Fixes #5213.

Changes:
 - For offsets use `CSSNumericValue` or `CSSKeywordish` instead of DOMString
 - When setting the offset, the grammars for these values is check to ensure only the valid subset can be set.
 - When computing effective scroll offset check the offset actual type and interpret it accordingly
 - Update examples to showcase the API and add noted to clarify the usage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@majido majido

Labels
scroll-animations-1 Current Work
Projects
None yet
Milestone
scroll-animations-1 FPWD
Development

Successfully merging a pull request may close this issue.

[scroll-animations-1] Use CSSTypedOM instead of strings (#5213) majido/csswg-drafts
2 participants
@majido @plinss