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

Add feature for CSS Typed OM #1847

Merged
merged 5 commits into from
Nov 6, 2024
Merged

Conversation

queengooborg
Copy link
Contributor

This PR adds a feature for the CSS Typed OM Level 1.

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Sep 25, 2024
@jamesnw
Copy link
Collaborator

jamesnw commented Sep 25, 2024

I'd propose a different approach- many of these keys should be part of separate features. For instance, api.CSS.cap_static should be part of the cap feature, similar to how the highlight feature contains api.CSS.highlights_static.

This feature should contain the keys that are related to CSS syntax and wouldn't be part of other features. Some examples-

  • api.CSSNumericArray
  • api.CSSNumericValue
  • api.CSSUnitValue.value

Some of the keys here can be added to existing features, but there are others like rcap that will still need to be created.

@jamesnw
Copy link
Collaborator

jamesnw commented Oct 1, 2024

I'm taking another look at this, and would appreciate input from others.

The Typed OM is not supported by Firefox yet, so if the Typed OM keys go with their corresponding properties, we will need to compute the status on each to ignore the typed OM keys.

Alternatively, we can have a Typed OM feature as proposed here, that tells a clearer support story, but separates api.CSS.cap_static from css.types.length.cap.

Any thoughts on where these keys should go?

@ddbeck
Copy link
Collaborator

ddbeck commented Oct 2, 2024

This feature should contain the keys that are related to CSS syntax and wouldn't be part of other features.

This is the core thing. I think a lot of the keys here could survive as a typed OM feature no matter what.

The feature-specific keys, those are the troublesome ones. I'm going to make a pragmatic suggestion: put the per-feature keys into a draft feature—call it the typed OM "junk drawer" if we must. Sort the keys by which "real" feature they ought to live with and comment the lines accordingly.

I think this is going to be one of the prime features for resolving #971. We'll need some way to declare inferior or interior or adjacent features and this seems like a likely candidate for taking it on, since we'll get a bunch of them at once. If we can deal with that soon, then great. If we get impatient, then we'll have done the work to decide where they'll need to be moved already.

@jamesnw jamesnw mentioned this pull request Oct 18, 2024
@jamesnw
Copy link
Collaborator

jamesnw commented Oct 18, 2024

Another option would be to pull out a CSS numeric factory functions feature, which would encompass all of items that would be part of another feature. This would essentially be the draft junk drawer idea, and we could add comments saying that, but it would also be a useful feature. See #2000 for a draft PR.

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some adjustments needed on the description, but otherwise I think this is good to go now.

@jamesnw
Copy link
Collaborator

jamesnw commented Nov 6, 2024

@queengooborg I shepherded this over the finish line since I had made some parallel changes in the interim- thanks for the initial feature and patience!

@jamesnw jamesnw merged commit a94df37 into web-platform-dx:main Nov 6, 2024
3 checks passed
@queengooborg queengooborg deleted the css-typed-om branch November 6, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature definition Creating or defining new features or groups of features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants