Skip to content

fix: move @property declaration to top-level for gradient-border comp…#84

Merged
naymurdev merged 1 commit intoui-layouts:mainfrom
Yeom-JinHo:fix/gradient-border
Oct 25, 2025
Merged

fix: move @property declaration to top-level for gradient-border comp…#84
naymurdev merged 1 commit intoui-layouts:mainfrom
Yeom-JinHo:fix/gradient-border

Conversation

@Yeom-JinHo
Copy link
Contributor

@Yeom-JinHo Yeom-JinHo commented Oct 25, 2025 •
edited by coderabbitai bot
Loading

Description

This PR fixes the gradient-border animation bug caused by invalid placement of the @property rule.
Previously, the declaration was nested inside @layer and @supports, which prevented the browser from properly registering the --border-angle custom property.

Changes

  • Moved @property declaration to top-level for proper registration.
  • Ensured --border-angle is globally available before keyframes and gradient logic.
  • Verified that the gradient-border component now animates smoothly without missing variable errors.

Motivation

Browsers ignore @property rules declared inside nested blocks like @layer or @supports.
By moving it to the top-level scope, we ensure consistent animation and fix the missing --border-angle issue across all browsers.

Screenshots

✅ Gradient border animation is now smooth and visible

2025-10-25.1.01.45.mov

Summary by CodeRabbit

Release Notes

  • Refactor
    • Simplified CSS property declaration structure for improved maintainability. No changes to user-facing appearance or functionality.

@coderabbitai
Copy link

coderabbitai bot commented Oct 25, 2025 •
edited
Loading

Walkthrough

The change removes a conditional @supports wrapper around the @property --border-angle declaration in the CSS file, moving the property declaration to the top level. Browser behavior remains functionally equivalent since unsupported browsers will simply ignore the declaration.

Changes

Cohort / File(s) Summary
CSS Refactoring
apps/ui-layout/app/globals.css		 Removes @supports conditional wrapper from @property --border-angle declaration, moving it to top level

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 Hop hop, CSS made neat,
Unwrapped the @Property so sweet,
No @supports needed today,
Browsers ignore in their own way! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix: move @Property declaration to top-level for gradient-border comp…" directly and accurately describes the main change in the changeset. The title clearly identifies the specific action taken (moving the @Property declaration) and the context (to top-level for the gradient-border component), which matches the file-level summary showing the removal of the @supports wrapper and relocation of the @Property declaration. The title is concise, avoids vague terminology, and provides sufficient specificity for a teammate to understand the primary change when scanning the project history.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

coderabbitai[bot]
coderabbitai bot reviewed Oct 25, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/ui-layout/app/globals.css (1)

569-573: Move @property declaration near the top of the file.

While moving @property --border-angle to top-level scope (outside @layer/@supports) correctly fixes the registration issue, placing it at the end of the file is unconventional. Since the property is used in the @keyframes border animation at line 115, it's better practice to declare the @property near the beginning of the file, after imports but before rules that use it. This ensures the custom property is registered early in the parsing phase.

Consider relocating this declaration to just after the @custom-variant line (after line 10) or at the start of the @layer utilities block:

@custom-variant dark (&:is(.dark *));

+@property --border-angle {
+  inherits: false;
+  initial-value: 0deg;
+  syntax: '<angle>';
+}
+
@theme {

Then remove the declaration from lines 569-573.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f89595 and 7e26a76.

📒 Files selected for processing (1)
  • apps/ui-layout/app/globals.css (1 hunks)
🔇 Additional comments (1)
apps/ui-layout/app/globals.css (1)

569-573: Approve the core fix: @property now registered at top-level.

The change correctly moves the @property --border-angle declaration out of nested at-rules (@layer, @supports) to top-level scope. This resolves the browser registration issue—browsers ignore @property rules declared inside conditional blocks, and moving it to top-level ensures consistent animation across all supporting browsers. The fix aligns with the PR objective.

Please verify that the gradient-border component animates smoothly after this change. You mentioned a screenshot was included in the PR—ensure the animation displays correctly in all target browsers.

@Yeom-JinHo Yeom-JinHo mentioned this pull request Oct 25, 2025
Closed
@naymurdev naymurdev merged commit 8023f9f into ui-layouts:main Oct 25, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Yeom-JinHo @naymurdev