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 all standard CSS properties to JsxDOMStyle #7205

Merged
merged 2 commits into from
Dec 27, 2024

Conversation

stephanoskomnenos
Copy link
Contributor

@stephanoskomnenos stephanoskomnenos commented Dec 17, 2024

JsxDOMStyle lacks some properties that already in CSS standards (e.g. aspectRatio, scrollbarWidth).

The top half are from https://github.com/mdn/data/blob/main/css/properties.json and filtered by:

// ignore vendor properties
!key.startsWith("-")
// ignore non-standard properties
&& value.status === "standard"

The bottom half are those already in JsxDOMStyle but not in current CSS standards.

Here are the script to generate new record fields (https://github.com/stephanoskomnenos/rescript-cssprops/blob/main/scripts/generate.ts).

@stephanoskomnenos
Copy link
Contributor Author

stephanoskomnenos commented Dec 18, 2024

Another option is to define props by variant type.

  scrollbarWidth?: scrollbarWidth,
@unboxed
type scrollbarWidth =
  | @as("auto") Auto
  | @as("thin") Thin
  | @as("none") None
  | ...globals

I think just keep it as string and use extra packages (like styled-ppx) if needed.

@stephanoskomnenos stephanoskomnenos force-pushed the css-props branch 3 times, most recently from 24fbfe4 to 37568d9 Compare December 19, 2024 18:04
@fhammerschmidt
Copy link
Member

Nice work so far!

Yes strings are fine since you should use a CSS library anyway in most cases. However I don't really get all the @deprecateds. Some of them are just non-standard but those could be standard at some point right? So deprecating them might not be the accurate thing. I am not even sure if we should use the deprecate feature at all here, except if the features have really been removed from modern browsers.

Tagging @nojaf since he did some work on the new webapi which might need some alignment here.

@stephanoskomnenos
Copy link
Contributor Author

stephanoskomnenos commented Dec 27, 2024

Nice work so far!

Yes strings are fine since you should use a CSS library anyway in most cases. However I don't really get all the @deprecateds. Some of them are just non-standard but those could be standard at some point right? So deprecating them might not be the accurate thing. I am not even sure if we should use the deprecate feature at all here, except if the features have really been removed from modern browsers.

Tagging @nojaf since he did some work on the new webapi which might need some alignment here.

Some of those properties are in CSS 2.1 but removed in CSS 3 (like azimuth, clip). Some are included in WD/CR but removed in standard REC (like marquee-direction, overflow-style, rotation, nav-down). Some are in draft long time ago, but maybe obsolete now (like those in CSS Speech Module Level 1). Some are in draft (like text-decoration-skip), I didn't notice this part previously.

Those in draft should be kept. But I'm not sure how to do with the others?

@fhammerschmidt
Copy link
Member

Well, let's stay as simple as possible: If MDN says it is deprecated, it should probably be deprecated, otherwise not.

@stephanoskomnenos
Copy link
Contributor Author

I think just keep them. They are not on MDN.

@fhammerschmidt
Copy link
Member

Looks good. Now please add an entry to the changelog and we are good to go.

@fhammerschmidt fhammerschmidt enabled auto-merge (squash) December 27, 2024 21:00
@fhammerschmidt fhammerschmidt merged commit dfd36a2 into rescript-lang:master Dec 27, 2024
19 checks passed
@stephanoskomnenos
Copy link
Contributor Author

Thanks for merging. Glad to contribute to Rescript.

@stephanoskomnenos stephanoskomnenos deleted the css-props branch December 27, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants