Added container at-rule support #15
Conversation
The @container rule is similar enough (if not down right identical) to the @media rule. Copied and pasted the @media lines, replaced the regex to look for @container instead of @media. All other patterns should work the same, so no need to create new ones.
|
@microsoft-github-policy-service agree |
Added comments on what I changed.
Added 'aspect-ratio' to the Standard CSS list
| Saw that someone had created a pull request to add color syntax to the properties, but not the at-rule (see PR#14). After studying the cson file, I realized that the existing @media rule is basically identical to the new @container rule. Just copied and pasted the @media rule lines, and changed the regex lines to look for 'container' instead of 'media'. | ||
| This, however, still does not correct the problem that any properties (i.e. padding, margin, etc.) inside an @container block do not display a description thumbnail thingy when hovering over them. |
There was a problem hiding this comment.
@container preludes are completely different from @media preludes.
Does this change account for this difference?
There was a problem hiding this comment.
I also agree with @romainmenke above. Media queries and container queries are different, and should be treated differently.
As an aside, this text excerpt should not be put in the repo's README. Rather, it should be in the PR's description.
|
Also try to keep in mind that any at-rule that you reference here on GitHub is a ping to some person/account :) You can wrap them in backticks to prevent that. |
|
Thanks for approving the PR. I should have clarified that:
A: The Readme should not be added to the main project, it was just there to
describe what I did.
2: When I said ***@***.***` and ***@***.***` were the same, I meant in behavior
when it comes to the REGEX in the CSON file, which looks for text after the
@, then looks for stuff in (), then looks for stuff in {}, etc. I
understand programmatically, these two queries do two different things in
CSS.
😅😁
…On Fri, Sep 8, 2023, 14:41 Andrea Mah ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In README.md
<#15 (comment)>:
> +Saw that someone had created a pull request to add color syntax to the properties, but not the at-rule (see PR#14). After studying the cson file, I realized that the existing @media rule is basically identical to the new @container rule. Just copied and pasted the @media rule lines, and changed the regex lines to look for 'container' instead of 'media'.
+This, however, still does not correct the problem that any properties (i.e. padding, margin, etc.) inside an @container block do not display a description thumbnail thingy when hovering over them.
I also agree with @romainmenke <https://github.com/romainmenke> above.
Media queries and container queries are different, and should be treated
differently.
As an aside, this text excerpt should not be put in the repo's README.
Rather, it should be in the PR's description.
—
Reply to this email directly, view it on GitHub
<#15 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3AOYGXY6EA4OQFZYFQQHPDXZNYGLANCNFSM6AAAAAA3OJAC4U>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
If it should not be in the Readme, please remove it from the PR. Is there any way that you can add a more concise way of incorporating syntax highlighting for |
|
I'll remove the stuff from the Readme later today.
The syntax rules are the same in my view, so it is, and is not, a quick
fix. The syntax rules in the CSON file are based on a regular expression
search, no? Both rules look for a word after the `@` symbol, both require
stuff after that; media looks for screen, print, etc, container looks for a
container name. Then it looks for stuff wrapped in () like `(width <
30em)`, then it looks for stuff wrapped in {}. Does it really have to be
more complicated than that?
…On Fri, Sep 8, 2023, 15:16 Andrea Mah ***@***.***> wrote:
If it should not be in the Readme, please remove it from the PR.
Is there any way that you can add a more concise way of incorporating
syntax highlighting for @container? Although adopting the rules for @media
would be a quick fix, I'd prefer something more accurate so that it works
long-term.
—
Reply to this email directly, view it on GitHub
<#15 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3AOYGSEQ4CXTWYW4ZFDIO3XZN4JXANCNFSM6AAAAAA3OJAC4U>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
If you look at the differences between media queries and container queries in their docs, you can see that they use different keywords (ie: media features vs. the specific container descriptors). The CSON file specifically addresses media features for Line 1195 in 3bd0020 |
I get what you are saying, however, this solution would only be "strange" if people start writing up something like Line 1198 in 3bd0020 I am assuming from line 1198 that there would be another file involved to help support a specific section for "container-features," though that would be beyond my expertise if so. So it is a patch for now until full support for the rule is added. |
|
By the way, I seem to be unable to remove the README from the PR..? |
We don't want to highlight invalid syntax as if it were valid. Even if we were to introduce a patch, I would prefer something incomplete (ie: not covering all container queries and cases) rather than something that is incorrect. This way, we can try to keep the grammar better in the long-term. |
The @container rule is similar enough (if not down right identical) to the @media rule. Copied and pasted the @media lines, replaced the regex to look for @container instead of @media. All other patterns should work the same, so no need to create new ones.