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

Logical properties missing computed values #477

Open
johanholmerin opened this issue Feb 22, 2021 · 8 comments
Open

Logical properties missing computed values #477

johanholmerin opened this issue Feb 22, 2021 · 8 comments
Labels
idle Issues and pull requests with no activity for three months.

Comments

@johanholmerin
Copy link
Contributor

This is basically a follow question up to a comment in #319: some logical shorthands miss a computed property, because at the time it wasn't clear what those would be. My question is, is this something that can/should be fixed?

The reason I'm wondering is because csstype uses the computed property as an indicator for shorthands, which causes an issue here.

List of shorthands with missing computed list:

  • inset
  • inset-block
  • inset-inline
  • margin-block
  • margin-inline
  • padding-block
  • padding-inline
  • scroll-margin
  • scroll-margin-block
  • scroll-margin-inline
  • scroll-padding
  • scroll-padding-block
  • scroll-padding-inline
  • scroll-margin
@chrisdavidmills
Copy link
Contributor

Can I get your input on this one @rachelandrew ?

@rachelandrew
Copy link
Collaborator

yep, I'll assign it to me so I don't forget :)

@rachelandrew
Copy link
Collaborator

ah I can't assign things in this repo - could you do that @chrisdavidmills? I'll probably take a look tomorrow but if things show up in my issue list they are easier to find again.

@chrisdavidmills
Copy link
Contributor

@rachelandrew cheers! I've assigned you, but I've also given you write access to this repo, so it shouldn't be a problem in the future.

@rachelandrew
Copy link
Collaborator

@johanholmerin @chrisdavidmills

So this is still the same situation in the spec, it refers to the physical properties which I am guessing is not actually that useful in terms of what csstype is doing.

https://drafts.csswg.org/css-logical/#propdef-inset

@johanholmerin
Copy link
Contributor Author

csstype doesn't use the the value of the computed property, it only checks if it's an array and if it is then the property is considered a shorthand. Could it be set to an empty array, or do you have any other suggestion for how to differentiate longhands from shorthands?

@frenic do you have an opinion on this?

@frenic
Copy link
Contributor

frenic commented Mar 1, 2021

CSSType checks if computed is an array to determine if it is a shorthand property. But it doesn't use the values like @johanholmerin mentioned.

We need to separate longform and shorthand properties because shorthand in CSS in JS can be problematic in some cases and vendors need to be able to exclude shorthand properties from their types. It would be preferable if this could be detected from its source (MDN) like everything else. Either thought a new shorthand: boolean property or the computed: string[] property. Last resort would be to patch this directly to CSSType.

@johanholmerin
Copy link
Contributor Author

@rachelandrew @chrisdavidmills what do you think about changing the computed property to an empty array for these values? Seems like the easiest solution.

@rachelandrew rachelandrew removed their assignment Sep 28, 2021
@github-actions github-actions bot added the idle Issues and pull requests with no activity for three months. label Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idle Issues and pull requests with no activity for three months.
Projects
None yet
Development

No branches or pull requests

4 participants