-
Notifications
You must be signed in to change notification settings - Fork 193
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
Making the CSS property appliesto
values more specific
#585
Comments
As discussed on the Mozilla chat just now, it seems you folks are moving to webref for this data. Example for align-content: https://github.com/w3c/webref/blob/main/tr/css/css-align.json#L11
|
Originally, we created mdn/data from the spec, but without a process to update it when the spec evolved. I would like to take this directly from webref. Note that the fact that this is prose and not a real enum, makes it hard to ensure coherence (there is no real guarantee that different specs use the same wording for the same "appliesTo", and there may be edge cases where the prose is very specific), or to translate into other languages. Maybe we should do some statistics to see if it could be an enum-like list, and eventually how many exceptions there would be. |
That sounds good to me. Fetching all of the webref data for CSS and checking all of the possible appliesto cases should be quick to do and would give a good idea of the complexity involved. |
I did a quick test, got all data from webref's CSS properties List
And here is a graph that shows how many times each prose appears in the WebRef repo: https://docs.google.com/spreadsheets/d/1VZGBOD1uT8IawXSiMBe_djWN5WrDOcSxQvNXMkM75p4/edit?usp=sharing This shows that there is a long tail of really seldom used one-off prose. More than 50% of all "applies to" prose is made of only 9 different sentences:
|
Thanks for this analysis! I think it will be really helpful when we come to make CSSInfo use webref. The long tail makes it seem like we could have a hybrid approach, with a fairly small enumerated set plus a bailout "See prose" option for the rest. If I'm reading the list correctly it seems like there are things that could be fed back into the specs here too, like values that differ only in punctuation or capitalization. |
Totally! It would be a really good thing to harmonize across specs. That said, I don't know if there would be a way to ensure things don't start deviating again. |
) ## Description 1. What is this PR about (link the issue and add a short description) Do not show flex properties in the style panel when a layout is not flex closes #687 <img width="272" alt="image" src="https://user-images.githubusercontent.com/5077042/210067036-4571be1d-4745-48a3-be7f-45f91791a6a9.png"> Here I patched `appliesTo` of properties `alignItems`, `justifyItems` see mdn/data#585 ## Steps for reproduction Opens designer panel, check no flex props in display: block layout ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio-designer/blob/main/apps/designer/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env.example` and the `designer/env-check.js` if mandatory
Just to revive this a little, it doesn't seem like the current webref is entirely on par with the data in the repo here, in all cases. For example, a user recently reported (mdn/content#31806) that Lines 6521 to 6540 in 4f43ef9
(It should just be ...But looking at the equivalent data in https://github.com/w3c/webref/blob/main/tr/css/css-box.json, the fact that {
"name": "margin-left",
"value": "<length-percentage> | auto",
"initial": "0",
"appliesTo": "all elements except internal table elements, ruby base containers, and ruby annotation containers",
"inherited": "no",
"percentages": "refer to logical width of containing block",
"computedValue": "the keyword auto or a computed <length-percentage> value",
"canonicalOrder": "per grammar",
"animationType": "by computed value type",
"styleDeclaration": [
"margin-left",
"marginLeft"
]
}, The information is simply not present. So, while clearly https://github.com/mdn/data/blob/main/css/properties.json needs to be corrected, it appears that if it were to be replaced with the equivalent webref content, significant detail would be lost. Even ignoring the "free-form string vs. formal enum" issues discussed above. |
Yes - as far as I know, webref/css doesn't include pseudo-elements here, I guess because it's not represented in the source: https://drafts.csswg.org/css-box/#margin-physical. |
Or anywhere, it seems — at least, not in detail. I was hoping the information might be present somewhere else in the webref data, and could simply be extracted into the structure used by MDN. (For example, MDN's own reference pages for webref's entire documentation for {
"name": "::first-line",
"prose": "The ::first-line pseudo-element represents the contents "
"of the first formatted line of its originating element."
}, Beyond that, it gets a few mentions in other contexts (like https://github.com/w3c/webref/blob/main/tr/css/css-regions.json which sees fit to note that |
It feels to me like the
appliesto
property of the CSS properties data isn't defined very specifically at the moment.For context, here is a concrete example:
https://developer.mozilla.org/en-US/docs/Web/CSS/align-items#formal_definition
MDN says that
align-items
applies to all elements (data is here). While true in the sense that it won't break if you apply it to any element, it actually won't do anything unless the element is not a grid container or a flex container.Contrast this with this other example:
https://developer.mozilla.org/en-US/docs/Web/CSS/grid-auto-columns#formal_definition
MDN says that
grid-auto-columns
only applies to grid containers, which is true. But also, it won't cause any problem if you apply it to any other type of containers.I would love if
appliesto
was more specific. There seems to be a lot of different values that theappliesto
enum can take: https://github.com/mdn/data/blob/main/css/properties.schema.json#L162and using the most specific one has advantages: it makes it easier for web authors to know why a property they're using isn't doing anything.
As an example, if you're confused about flexbox and you're trying to align items using
align-content
but you did not specifyflex-wrap:wrap
, then the property won't have any effects.In this specific case, the data is actually very helpful. MDN says that
align-content
only applies to multi-line flex containers: https://developer.mozilla.org/en-US/docs/Web/CSS/align-content#formal_definitionAlthough it's not entirely correct because it also applies to grid containers.
So I have two questions in this issue:
appliesto
property should be as specific as possible for all properties?appliesto
enum to make it able to cover more cases? For example, there is nogridAndFlexContainer
value now, but it would be useful in thealign-content
case. In fact, it might become useful, at some point, to allow more complex values than just strings. For instance:gridContainer AND flexContainer
seems like it would help.As background to this, in the past I worked on this inactive-css repo with the hope to create a reusable dataset for exactly this case: https://github.com/captainbrosset/inactive-css
I'd be willing to transfer this data over to mdn/data (by mapping it to the
appliesto
field) if there is interest.The text was updated successfully, but these errors were encountered: