-
Notifications
You must be signed in to change notification settings - Fork 152
Add support for modern CSS at-rules @layer, @scope, and @starting-style #1549
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
Changes from 4 commits
2dbafce
7469397
90c4b2a
1b4c75a
3087f1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,10 +15,12 @@ interface AtRule extends CSSListItem | |
| /** | ||
| * Since there are more set rules than block rules, | ||
| * we’re whitelisting the block rules and have anything else be treated as a set rule. | ||
| * NOTE: The new line when assigning the value is added to avoid the line length limit of codesniffer. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still needs to be removed.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was just waiting for confirmation that BLOCK_RULES will not be converted to array in this PR. |
||
| * | ||
| * @internal since 8.5.2 | ||
| */ | ||
| public const BLOCK_RULES = 'media/document/supports/region-style/font-feature-values/container'; | ||
| public const BLOCK_RULES = | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to change this to an array, but that's for another PR.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a quick win. Are you sure you want it in a separate PR?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Having PRs only addressing one thing makes it easier to backport them or temporarily undo them if a problem is found. So, yes.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #1556 for this. |
||
| 'media/document/supports/region-style/font-feature-values/container/layer/scope/starting-style'; | ||
|
|
||
| /** | ||
| * @return non-empty-string | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,21 @@ public static function provideSyntacticallyCorrectAtRule(): array | |
| 'container' => [ | ||
| '@container (min-width: 60rem) { .items { background: blue; } }', | ||
| ], | ||
| 'layer named' => [ | ||
| '@layer theme { .button { color: blue; } }', | ||
| ], | ||
| 'layer anonymous' => [ | ||
| '@layer { .card { padding: 1rem; } }', | ||
| ], | ||
| 'scope with selector' => [ | ||
| '@scope (.card) { .title { font-size: 2rem; } }', | ||
| ], | ||
| 'scope root only' => [ | ||
| '@scope { .content { margin: 0; } }', | ||
| ], | ||
| 'starting-style' => [ | ||
| '@starting-style { .dialog { opacity: 0; transform: translateY(-10px); } }', | ||
| ], | ||
| ]; | ||
| } | ||
|
|
||
|
|
@@ -94,4 +109,100 @@ public function parsesSyntacticallyCorrectAtRuleInStrictMode(string $css): void | |
|
|
||
| self::assertNotEmpty($contents, 'Failing CSS: `' . $css . '`'); | ||
| } | ||
|
|
||
| /** | ||
| * @return array<string, array{0: string, 1: string, 2: string, 3: int}> | ||
| */ | ||
| public static function provideAtRuleParsingData(): array | ||
| { | ||
| return [ | ||
| 'layer with named argument' => [ | ||
| '@layer theme { .button { color: blue; } }', | ||
| 'layer', | ||
| 'theme', | ||
| 1, | ||
| ], | ||
| 'layer without arguments' => [ | ||
| '@layer { .card { padding: 1rem; } }', | ||
| 'layer', | ||
| '', | ||
| 1, | ||
| ], | ||
| 'scope with selector' => [ | ||
| '@scope (.card) { .title { font-size: 2rem; } }', | ||
| 'scope', | ||
| '(.card)', | ||
| 1, | ||
| ], | ||
|
Comment on lines
+134
to
+139
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible to have a test for scope with a limt, e.g.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It parses correctly. |
||
| 'scope without selector' => [ | ||
| '@scope { .content { margin: 0; } }', | ||
| 'scope', | ||
| '', | ||
| 1, | ||
| ], | ||
| 'starting-style' => [ | ||
| '@starting-style { .dialog { opacity: 0; transform: translateY(-10px); } }', | ||
| 'starting-style', | ||
| '', | ||
| 1, | ||
| ], | ||
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * @return array<string, array{0: string, 1: list<string>}> | ||
| */ | ||
| public static function provideAtRuleRenderingData(): array | ||
| { | ||
| return [ | ||
| 'layer with named argument' => [ | ||
| '@layer theme { .button { color: blue; } }', | ||
| ['@layer theme', '.button', 'color: blue'], | ||
| ], | ||
| 'scope with selector' => [ | ||
| '@scope (.card) { .title { font-size: 2rem; } }', | ||
| ['@scope (.card)', '.title', 'font-size: 2rem'], | ||
| ], | ||
| 'starting-style' => [ | ||
| '@starting-style { .dialog { opacity: 0; } }', | ||
| ['@starting-style', '.dialog', 'opacity: 0'], | ||
| ], | ||
| ]; | ||
| } | ||
|
|
||
| /** | ||
| * @test | ||
| * | ||
| * @dataProvider provideAtRuleParsingData | ||
| */ | ||
| public function parsesAtRuleBlockList( | ||
| string $css, | ||
| string $expectedName, | ||
| string $expectedArgs, | ||
| int $expectedContentCount | ||
| ): void { | ||
| $contents = (new Parser($css))->parse()->getContents(); | ||
| $atRuleBlockList = $contents[0]; | ||
|
|
||
| self::assertInstanceOf(AtRuleBlockList::class, $atRuleBlockList); | ||
| self::assertSame($expectedName, $atRuleBlockList->atRuleName()); | ||
| self::assertSame($expectedArgs, $atRuleBlockList->atRuleArgs()); | ||
| self::assertCount($expectedContentCount, $atRuleBlockList->getContents()); | ||
| } | ||
|
|
||
| /** | ||
| * @test | ||
| * | ||
| * @dataProvider provideAtRuleRenderingData | ||
| * | ||
| * @param list<string> $expectedSubstrings | ||
| */ | ||
| public function rendersAtRuleBlockListCorrectly(string $css, array $expectedSubstrings): void | ||
| { | ||
| $rendered = (new Parser($css))->parse()->render(); | ||
|
|
||
| foreach ($expectedSubstrings as $expected) { | ||
| self::assertStringContainsString($expected, $rendered); | ||
| } | ||
| } | ||
| } | ||
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.
This comment belongs in the PR discussion or commit message, not the code.