Skip to content

test: cover @page margin properties - #524

Merged
DMartens merged 2 commits into
mainfrom
test/page-margin-properties
Sep 5, 2026
Merged

test: cover @page margin properties#524
DMartens merged 2 commits into
mainfrom
test/page-margin-properties

Conversation

@Pixel998

@Pixel998 Pixel998 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Prerequisites checklist

AI acknowledgment

  • I did not use AI to generate this PR.
  • (If the above is not checked) I have reviewed the AI-generated content before submitting.

What is the purpose of this pull request?

Prevent regressions where valid page-margin declarations are reported as unknown @page descriptors.

What changes did you make? (Give an overview)

Added a no-invalid-at-rules test covering margin, margin-top, margin-right, margin-bottom, and margin-left inside @page.

Related Issues

Closes #519

Is there anything you'd like reviewers to focus on?

Summary by CodeRabbit

  • Tests
    • Added coverage confirming that valid @page rules with page size and margin declarations are accepted.

@github-project-automation github-project-automation Bot moved this to Needs Triage in Triage Aug 4, 2026
@Pixel998 Pixel998 moved this from Needs Triage to Implementing in Triage Aug 4, 2026

@DMartens DMartens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes LGTM, thanks. I changed this to "blocked" as this requires csstree update (see PR).

@DMartens DMartens moved this from Implementing to Blocked in Triage Aug 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

@github-actions github-actions Bot added the Stale label Aug 14, 2026
@Pixel998 Pixel998 added accepted There is consensus among the team that this change meets the criteria for inclusion and removed Stale labels Aug 15, 2026
@Pixel998
Pixel998 marked this pull request as ready for review September 5, 2026 19:27
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: ddaa81b6-494f-49a8-80af-f15129745a0a

📥 Commits

Reviewing files that changed from the base of the PR and between 33e7314 and e8aa24f.

📒 Files selected for processing (1)
  • tests/rules/no-invalid-at-rules.test.js

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The test suite adds a valid @page at-rule case with page size and margin declarations.

Changes

@page at-rule validation

Layer / File(s) Summary
Valid @page descriptor test
tests/rules/no-invalid-at-rules.test.js
Adds coverage for @page rules containing size and margin declarations.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to e8aa2

This adds regression coverage for valid @page margin descriptors without changing runtime behavior. The change is ready to merge.

Suggested reviewers: dmartens

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the test change for @page margin properties and matches the pull request content.
Linked Issues check ✅ Passed The pull request adds a valid-case test for @page margin declarations. This directly addresses issue #519, which requires margin to be accepted without an unknown-descriptor error.
Out of Scope Changes check ✅ Passed The only reported change is a focused test addition for @page margin descriptors. No unrelated changes are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/page-margin-properties

Comment @coderabbitai help to get the list of available commands.

@DMartens DMartens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes still LGTM, thanks.

@DMartens
DMartens merged commit 0599e0f into main Sep 5, 2026
39 checks passed
@DMartens
DMartens deleted the test/page-margin-properties branch September 5, 2026 20:31
@github-project-automation github-project-automation Bot moved this from Blocked to Complete in Triage Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion chore

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Bug: no-invalid-at-rules doesn't recognize margin

2 participants