-
Notifications
You must be signed in to change notification settings - Fork 330
fix: fix the issue of missing the tiny prefix in CSS after packaging #3695
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
Conversation
WalkthroughMoved the grid-system LESS import from the global scope into component-scoped blocks for NavMenu and Row, limiting grid styles to their respective prefixed classes. No public API or class name changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/theme/src/row/index.less (1)
21-21: Document the intent to prevent future hoisting.Add a brief comment so future refactors don’t move this import back to top-level.
Apply:
.clearfix(); - @import '../grid-system/index.less'; + // Scope grid-system under the row block to preserve the 'tiny-' prefix post-packaging. Do not hoist. + @import '../grid-system/index.less';packages/theme/src/nav-menu/index.less (1)
20-20: Add a guardrail comment mirroring Row.Clarify that the import is intentionally scoped to fix the packaging prefix issue.
Apply:
.inject-NavMenu-vars(); - @import '../grid-system/index.less'; + // Scope grid-system under the nav-menu block to avoid losing 'tiny-' prefix during packaging. + @import '../grid-system/index.less';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/theme/src/nav-menu/index.less(1 hunks)packages/theme/src/row/index.less(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (2)
packages/theme/src/row/index.less (1)
21-21: Scoped import looks correct — confirm toolchain supports @import inside rulesetsFound "less" == 4.3.0 in package.json; @import '../grid-system/index.less' present at:
- packages/theme/src/row/index.less:21
- packages/theme/src/nav-menu/index.less:20
- packages/theme-saas/src/row/index.less:3
less-loader not present in package.json and no compiled CSS (dist) was available, so nesting could not be verified. Run a local build and confirm the grid rules are nested under the row prefix (e.g.
.tiny-row .tiny-col-*) and that no top-level.tiny-col-*selectors remain:npm run build
rg -nP '.tiny-row\b.*.tiny-(xs|sm|md|lg|col)-\d+' dist -S
rg -nP '^.(tiny-(xs|sm|md|lg|col)-\d+)\b' dist -Spackages/theme/src/nav-menu/index.less (1)
20-20: Verify/remove grid-system import from NavMenupackages/theme/src/nav-menu/index.less imports ../grid-system/index.less (≈ line 20). Repo scan found no .tiny-* grid classes or other grid references; grid-system/index.less exists but couldn't be fully inspected in the sandbox — confirm whether it emits global .tiny-col/.tiny-row selectors or only exposes mixins/variables. If only mixins are needed, import those specifically; if it emits unused global CSS, remove the import to avoid extra bundle weight. After building, search generated CSS (dist/*.css) for .tiny-(col|row|xs|sm|md|lg) and compare size impact.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #3444
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit