Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ Please also have a look at our

### Added

- Add support for modern CSS at-rules: `@layer`, `@scope`, and `@starting-style` (#1549)

### Changed

### Deprecated
Expand Down
4 changes: 3 additions & 1 deletion src/Property/AtRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Copy Markdown
Collaborator

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.

Suggested change
* NOTE: The new line when assigning the value is added to avoid the line length limit of codesniffer.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs to be removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 =
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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?

Having PRs only addressing one thing makes it easier to backport them or temporarily undo them if a problem is found. So, yes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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
Expand Down
142 changes: 142 additions & 0 deletions tests/CSSList/AtRuleBlockListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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); } }',
],
];
}

Expand Down Expand Up @@ -94,4 +109,131 @@ public function parsesSyntacticallyCorrectAtRuleInStrictMode(string $css): void

self::assertNotEmpty($contents, 'Failing CSS: `' . $css . '`');
}

/**
* @test
*/
public function parsesLayerWithNamedArgument(): void
{
$css = '@layer theme { .button { color: blue; } }';
$contents = (new Parser($css))->parse()->getContents();
$atRuleBlockList = $contents[0];

self::assertInstanceOf(AtRuleBlockList::class, $atRuleBlockList);
self::assertSame('layer', $atRuleBlockList->atRuleName());
self::assertSame('theme', $atRuleBlockList->atRuleArgs());

$nestedContents = $atRuleBlockList->getContents();
self::assertCount(1, $nestedContents, 'Layer should contain one declaration block');
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does parsing a statement @layer rule like

@layer theme, layout, utilities;

also work following this change?

If it does, we should add tests to confirm. If not, that will be for a future PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it doesn't work currently :(
The parser discards @layer theme, layout, utilities; and returns an empty contents array and renders nothing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it doesn't work currently :(

I feared that would be the case. There are statement at-rules, and block at-rules, and @layer can be either. Some congnitive effort and refactoring will likely be required to support both flavours of @layer (as a future PR).


/**
* @test
*/
public function parsesLayerWithoutArguments(): void
{
$css = '@layer { .card { padding: 1rem; } }';
$contents = (new Parser($css))->parse()->getContents();
$atRuleBlockList = $contents[0];

self::assertInstanceOf(AtRuleBlockList::class, $atRuleBlockList);
self::assertSame('layer', $atRuleBlockList->atRuleName());
self::assertSame('', $atRuleBlockList->atRuleArgs());

$nestedContents = $atRuleBlockList->getContents();
self::assertCount(1, $nestedContents, 'Layer should contain one declaration block');
}

/**
* @test
*/
public function parsesScopeWithSelector(): void
{
$css = '@scope (.card) { .title { font-size: 2rem; } }';
$contents = (new Parser($css))->parse()->getContents();
$atRuleBlockList = $contents[0];

self::assertInstanceOf(AtRuleBlockList::class, $atRuleBlockList);
self::assertSame('scope', $atRuleBlockList->atRuleName());
self::assertSame('(.card)', $atRuleBlockList->atRuleArgs());

$nestedContents = $atRuleBlockList->getContents();
self::assertCount(1, $nestedContents, 'Scope should contain one declaration block');
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does parsing with a scope limit also work following this change? And parsing with a selector list? If so, we should perhaps add tests to confirm this.

We could use a data provider to provide various @scope arguments, perhaps lifted from the MDN examples, so we'd only need one @test method, which would insert the argument into $css, and check it in assertSame( ... atRuleArgs()). This may also work for @scope without an argument.


/**
* @test
*/
public function parsesScopeWithoutSelector(): void
{
$css = '@scope { .content { margin: 0; } }';
$contents = (new Parser($css))->parse()->getContents();
$atRuleBlockList = $contents[0];

self::assertInstanceOf(AtRuleBlockList::class, $atRuleBlockList);
self::assertSame('scope', $atRuleBlockList->atRuleName());
self::assertSame('', $atRuleBlockList->atRuleArgs());

$nestedContents = $atRuleBlockList->getContents();
self::assertCount(1, $nestedContents, 'Scope should contain one declaration block');
}

/**
* @test
*/
public function parsesStartingStyle(): void
{
$css = '@starting-style { .dialog { opacity: 0; transform: translateY(-10px); } }';
$contents = (new Parser($css))->parse()->getContents();
$atRuleBlockList = $contents[0];

self::assertInstanceOf(AtRuleBlockList::class, $atRuleBlockList);
self::assertSame('starting-style', $atRuleBlockList->atRuleName());
self::assertSame('', $atRuleBlockList->atRuleArgs());

$nestedContents = $atRuleBlockList->getContents();
self::assertCount(1, $nestedContents, 'Starting-style should contain one declaration block');
}

/**
* @test
*/
public function rendersLayerCorrectly(): void
{
$css = '@layer theme { .button { color: blue; } }';
$document = (new Parser($css))->parse();
$rendered = $document->render();

self::assertStringContainsString('@layer theme', $rendered);
self::assertStringContainsString('.button', $rendered);
self::assertStringContainsString('color: blue', $rendered);
}

/**
* @test
*/
public function rendersScopeCorrectly(): void
{
$css = '@scope (.card) { .title { font-size: 2rem; } }';
$document = (new Parser($css))->parse();
$rendered = $document->render();

self::assertStringContainsString('@scope (.card)', $rendered);
self::assertStringContainsString('.title', $rendered);
self::assertStringContainsString('font-size: 2rem', $rendered);
}

/**
* @test
*/
public function rendersStartingStyleCorrectly(): void
{
$css = '@starting-style { .dialog { opacity: 0; } }';
$document = (new Parser($css))->parse();
$rendered = $document->render();

self::assertStringContainsString('@starting-style', $rendered);
self::assertStringContainsString('.dialog', $rendered);
self::assertStringContainsString('opacity: 0', $rendered);
}
}
12 changes: 11 additions & 1 deletion tests/Unit/Property/AtRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,17 @@ final class AtRuleTest extends TestCase
public function blockRulesConstantIsCorrect(): void
{
self::assertEqualsCanonicalizing(
['media', 'document', 'supports', 'region-style', 'font-feature-values', 'container'],
[
'media',
'document',
'supports',
'region-style',
'font-feature-values',
'container',
'layer',
'scope',
'starting-style',
],
explode('/', AtRule::BLOCK_RULES)
);
}
Expand Down
Loading