apply stylelint auto fixes #5940
Conversation
christian-byrne
requested review from
viva-jinyi,
ltdrdata,
pythongosssss and
webfiltered
as code owners
dosubot
bot
added
the
size:L
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 10/08/2025, 01:21:11 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 10/08/2025, 01:48:38 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Auto-fix 68 CSS issues found by stylelint: - Convert legacy color functions (rgba() → rgb()) - Normalize font weights (bold/normal → 700/400) - Use shorthand properties (top/right/bottom/left → inset) - Add url() wrapper to @import statements - Remove redundant shorthand values - Fix pseudo-element notation (: → ::) Remaining 45 issues require manual review: - 44 descending specificity warnings - 1 duplicate selector
- Disable no-descending-specificity (43 warnings, requires CSS refactor) - Fix 2 duplicate selectors (merge into single declaration) - Remove duplicate vendor prefix fallbacks (user-select, mask-image) - Add font fallbacks for primeicons (sans-serif) - Whitelist Vue v-bind() CSS function
Remove stylelint from ignoreDependencies and ignoreBinaries now that it's actively used. Add tw-animate-css and tailwindcss to ignoreDependencies as they're used in CSS @import statements which knip doesn't detect.
DrJKL
force-pushed
the
fix/stylelint-auto-fixes
branch
from
00f953b to
cc44118
Compare
christian-byrne
added
the
claude-review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Comprehensive PR Review
This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another.
Review Summary
PR: apply stylelint auto fixes (#5940)
Impact: 99 additions, 131 deletions across 25 files
Issue Distribution
- Critical: 0
- High: 2
- Medium: 3
- Low: 0
Category Breakdown
- Architecture: 1 issue
- Security: 0 issues
- Performance: 0 issues
- Code Quality: 4 issues
Key Findings
Architecture & Design
The PR applies stylelint auto-fixes which modernize CSS syntax but raise some architectural concerns. The decision to disable the no-descending-specificity rule sidesteps underlying CSS architecture issues that should ideally be addressed rather than suppressed.
Code Quality Considerations
Several modern CSS properties were introduced that have browser compatibility implications:
- flex-flow and place-content shorthand properties (limited older browser support)
- inset property usage (not supported in IE, partial older browser support)
- Removal of explicit font-size declaration may cause visual regression
The color function modernization (rgba → rgb with slash notation) is correctly implemented and follows CSS Color Module Level 4 specification.
Integration Points
These CSS changes affect the visual presentation across multiple components including:
- Node preview interfaces
- Tree explorer drag-and-drop indicators
- Workflow tab popovers
- Design system color variables
Positive Observations
- Proper modernization of color functions from rgba() to rgb() syntax
- Correct conversion of pseudo-elements from :before to ::before
- Appropriate margin shorthand optimizations
- Proper numeric font-weight conversions (bold → 700)
- Consistent application of stylelint rules across the codebase
References
- CSS Color Module Level 4: https://www.w3.org/TR/css-color-4/
- CSS Flexible Box Layout Module: https://www.w3.org/TR/css-flexbox-1/
- CSS Box Alignment Module Level 3: https://www.w3.org/TR/css-align-3/
Next Steps
- Address browser compatibility concerns for modern CSS properties
- Consider restoring explicit font-size declaration to prevent visual regression
- Evaluate whether to address underlying CSS specificity issues rather than disabling the rule
- Test visual output across supported browsers to ensure no regressions
This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.
- Remove `declaration-block-no-redundant-longhand-properties` stylelint rule to avoid browser compatibility issues with modern CSS shorthands - Revert `flex-flow` and `place-content` back to longhand properties in NodePreview.vue for better browser support - Revert `inset: 0` back to longhand `top/left/right/bottom` in TreeExplorer.vue for browser compatibility - Update knip.config.ts CSS compiler to properly handle `url()` wrappers in @import statements
|
I removed the declaration-block-no-redundant-longhand-properties rule and reverted the relevant fixes - according to Claude, it forces us to use shorthand which is sometimes not super well supported and the tradeoff does not seem worth it. |
I generally prefer the shorthand, but we can always add rules incrementally later 🙂 |
- Revert all remaining `inset:` shorthands to longhand in design-system, SidebarHelpCenterIcon, menu.css, and PackCard - Revert `flex-flow` and `place-content` to longhand in design-system
| top: 0 !important; | ||
| bottom: 0 !important; | ||
| left: auto !important; | ||
| right: 0 !important; |
There was a problem hiding this comment.
The importants still make me sad
Summary
Applied stylelint auto-fixes and resolved manual CSS issues across 25 files to achieve full compliance with stylelint rules.
Changes
no-descending-specificityrule (43 warnings require architectural CSS refactor)Review Focus
Verify no visual regressions from modernized CSS syntax:
rgba(0, 0, 0, 0.5)→rgb(0 0 0 / 50%)bold/normal→700/400:before→ `::before┆Issue is synchronized with this Notion page by Unito