-
Notifications
You must be signed in to change notification settings - Fork 194
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
feat: add container at rule #853
base: main
Are you sure you want to change the base?
Conversation
de560b9
to
e924dd1
Compare
css/at-rules.json
Outdated
@@ -120,6 +120,20 @@ | |||
"status": "standard", | |||
"mdn_url": "https://developer.mozilla.org/docs/Web/CSS/@counter-style" | |||
}, | |||
"@container": { | |||
"syntax": "@container <container-condition># {\n <block-contents>\n}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"syntax": "@container <container-condition># {\n <block-contents>\n}", | |
"syntax": "@container <container-condition># {\n <block-contents>\n}", |
the syntax need to add to syntaxes.json if it is not existed in that file
e.g. #835
Applied your feedback, thanks :) |
css/at-rules.json
Outdated
"interfaces": [ | ||
"CSSGroupingRule", | ||
"CSSConditionRule", | ||
"CSSContainerRule" | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently sone at-rule includes its interface and inherited interface, while most are not. This may need desicion, but it OK to not modify this at present.
In docs:
interfaces
(array of strings): These are the CSSOM interfaces that belong to the at-rule.
For this reason, personally I think only CSSContainerRule
should be represented.
Wonder if you have other opinions:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mhm, I just followed some of the other at-rules which included the top 2 interfaces as well as their own
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it is true, so I mean what do you think of this case? If not sure, you could simply keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind, let's keep it for now I guess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think twice, and I think we should not keep inherit interfaces.
i. currently only three cases list inherit interfaces (including one that no list direct interface), while nine cases only list direct interface
ii. the inherit info is a API feature, which is better to maintain at inheritance.json
iii. as per the docs, the interface belong to the at-rule, mostly I think this is only the direct interface
see also #856
2d890b5
to
067b698
Compare
Description
Add
@container
at ruleMotivation
VSCode not correctly syntax hightlighting
Additional details
https://developer.mozilla.org/en-US/docs/Web/CSS/@container
https://drafts.csswg.org/css-conditional-5/#container-rule
Related issues and pull requests