feat: add zero-specificity support and standardize mixin parameters #83
Conversation
Add $zero-specificity parameter to theme mixins with :where() wrapping Rename $props to $use-custom-props across all mixins for consistency Update spread-map-into-attrs mixin to use $use-custom-props parameter Update all mixin calls in components and theme files Remove obsolete $props parameter from spread-map-into-bem mixin
|
Caution Review failedThe pull request is closed. WalkthroughThis PR renames an SCSS mixin parameter from Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/tools/mixin-modules/_bem.scss (1)
41-59: Clarify the distinction between$custom-propsand$use-custom-propsparameters.The mixin has both
$use-custom-props(line 49) and$custom-props(line 51). While they serve different purposes—$use-custom-propscontrols CSS custom property generation and$custom-propsis a flag to skip the props section—the naming similarity could cause confusion for consumers.Consider adding a brief inline comment or updating the docblock to clarify the distinction, especially since the PR's goal is to standardize naming.
🔎 Suggested documentation improvement
// @param {Bool} $use-custom-props - Whether to use CSS custom properties // @param {Bool} $zero-specificity - Whether to wrap in :where() -// @param {Bool} $custom-* - Flags to skip specific parts +// @param {Bool} $custom-props - Skip auto-generating :root custom properties block +// @param {Bool} $custom-block - Skip auto-generating the block selector +// @param {Bool} $custom-elements - Skip auto-generating element selectors +// @param {Bool} $custom-modifiers - Skip auto-generating modifier selectors +// @param {Bool} $custom-states - Skip auto-generating state selectors +// @param {Bool} $custom-breakpoints - Skip auto-generating breakpoint rules +// @param {Bool} $custom-transitions - Skip auto-generating transition rules // @param {Map} $bps - Breakpoints map
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
.eslintrc-auto-import.jsonauto-imports.d.tspackage.jsonsrc/components/vv-accordion-group.scsssrc/components/vv-accordion.scsssrc/components/vv-alert-group.scsssrc/components/vv-alert.scsssrc/components/vv-avatar-group.scsssrc/components/vv-avatar.scsssrc/components/vv-badge.scsssrc/components/vv-breadcrumb.scsssrc/components/vv-button-group.scsssrc/components/vv-button.scsssrc/components/vv-card.scsssrc/components/vv-checkbox-group.scsssrc/components/vv-checkbox.scsssrc/components/vv-dialog.scsssrc/components/vv-dropdown-action.scsssrc/components/vv-dropdown-optgroup.scsssrc/components/vv-dropdown-option.scsssrc/components/vv-dropdown.scsssrc/components/vv-input-file.scsssrc/components/vv-input-text.scsssrc/components/vv-nav.scsssrc/components/vv-progress.scsssrc/components/vv-radio-group.scsssrc/components/vv-radio.scsssrc/components/vv-select.scsssrc/components/vv-skeleton.scsssrc/components/vv-tab.scsssrc/components/vv-table.scsssrc/components/vv-text.scsssrc/components/vv-textarea.scsssrc/components/vv-tooltip.scsssrc/themes/dark/components/_vv-alert.scsssrc/themes/dark/components/_vv-avatar.scsssrc/themes/dark/components/_vv-badge.scsssrc/themes/dark/components/_vv-button.scsssrc/themes/dark/components/_vv-dropdown-action.scsssrc/themes/dark/components/_vv-dropdown-option.scsssrc/themes/dark/components/_vv-nav.scsssrc/themes/dark/components/_vv-select.scsssrc/tools/mixin-modules/_attrs.scsssrc/tools/mixin-modules/_bem.scsssrc/tools/mixin-modules/_theme.scss
🔇 Additional comments (58)
src/components/vv-breadcrumb.scss (1)
8-14: LGTM! Parameter rename is correct.The parameter rename from
$propsto$use-custom-propsaligns with the PR's goal of standardizing mixin parameter naming across the codebase. The value passed remains unchanged, ensuring no behavioral impact.src/tools/mixin-modules/_attrs.scss (4)
67-77: LGTM! Mixin signature and documentation updated correctly.The parameter rename from
$propsto$use-custom-propsis properly reflected in both the documentation comment and the function signature. The default value remainstrue, preserving backward compatibility.
110-118: LGTM! Recursive call updated correctly.The recursive call to
spread-map-into-attrswithin the pseudo-element handler correctly propagates the renamed$use-custom-propsparameter.
124-132: LGTM! Recursive call updated correctly.The recursive call to
spread-map-into-attrscorrectly uses the renamed$use-custom-propsparameter.
135-139: LGTM! Conditional logic updated correctly.The conditional that gates custom property generation now correctly references the renamed
$use-custom-propsparameter, maintaining the original logic.src/components/vv-skeleton.scss (1)
9-15: LGTM! Parameter rename is correct.The parameter rename from
$propsto$use-custom-propsis consistent with the standardization effort across all component files.src/components/vv-avatar-group.scss (1)
8-14: LGTM! Parameter rename is correct.The parameter rename from
$propsto$use-custom-propsis consistent with the standardization effort across the codebase..eslintrc-auto-import.json (1)
102-102: LGTM! VueUse auto-import addition is correct.The addition of
refManualResetglobal follows the established pattern for other VueUse ref utilities and is properly positioned in alphabetical order.src/components/vv-radio-group.scss (1)
8-14: LGTM! Parameter rename is correct.The parameter rename from
$propsto$use-custom-propsis consistent with the standardization effort.src/components/vv-dropdown-action.scss (1)
8-14: LGTM! Parameter rename is correct.The parameter rename from
$propsto$use-custom-propsis consistent with the standardization effort across all component files.src/components/vv-checkbox-group.scss (1)
8-14: LGTM! Parameter rename is correct.The parameter rename from
$propsto$use-custom-propsis consistent with the standardization effort across the codebase.src/components/vv-dropdown.scss (1)
8-14: LGTM! Consistent parameter rename.The parameter rename is correct and consistent with the standardization effort.
src/components/vv-tooltip.scss (1)
9-15: LGTM! Consistent parameter rename.The parameter rename is correct and aligns with the standardization effort.
src/components/vv-radio.scss (1)
9-15: LGTM! Consistent parameter rename.The parameter rename is correct and consistent with the codebase standardization.
src/components/vv-dropdown-option.scss (1)
8-14: LGTM! Consistent parameter rename.The parameter rename is correct and aligns with the broader refactoring effort.
src/components/vv-button-group.scss (1)
11-17: LGTM! Consistent parameter rename.The parameter rename is correct and consistent with the standardization objectives.
src/components/vv-card.scss (1)
8-14: LGTM! Consistent parameter rename.The parameter rename is correct and aligns with the codebase-wide standardization.
src/components/vv-badge.scss (1)
8-14: LGTM! Consistent parameter rename.The parameter rename is correct and completes the standardization effort for this file.
src/components/vv-table.scss (1)
8-14: Parameter rename is correctly and consistently applied across the codebase.All
spread-map-into-bemcall sites and the mixin definition itself have been updated to use$use-custom-props. No instances of the old$propsparameter remain. The mixin and all nested calls (elements, modifiers, states, breakpoints, transitions, and attributes) properly pass the parameter through, confirming consistent implementation across all 30+ affected component files.src/components/vv-alert-group.scss (1)
8-14: LGTM! Parameter renaming is consistent.The parameter rename from
$propsto$use-custom-propsaligns with the PR's standardization objective. The passed value and overall logic remain unchanged.src/components/vv-dialog.scss (1)
8-14: LGTM! Consistent parameter standardization.The parameter rename is correctly applied and maintains the same value, consistent with the broader refactoring effort.
src/components/vv-alert.scss (1)
8-14: LGTM! Parameter rename applied correctly.The mixin invocation has been updated with the standardized parameter name while preserving the original value.
src/themes/dark/components/_vv-badge.scss (1)
7-14: LGTM! Zero-specificity parameter added correctly.The addition of the
$zero-specificityparameter to the theme mixin enables zero-specificity support as intended. The trailing comma formatting is appropriate for multi-line argument lists.src/themes/dark/components/_vv-dropdown-action.scss (1)
7-14: LGTM! Consistent zero-specificity addition.The zero-specificity parameter is properly threaded through the theme component mixin, consistent with the pattern applied across other dark theme components.
src/components/vv-input-file.scss (1)
10-16: LGTM! Parameter standardization applied correctly.The parameter rename is correctly applied. I also note that
$zero-specificity-for-componentsis already being used consistently throughout this file in thewrap-with-wherecalls (lines 24, 48, 68, 90), which shows proper integration of the zero-specificity feature.src/components/vv-avatar.scss (1)
8-14: LGTM! Clean parameter rename.The parameter standardization is correctly applied, maintaining consistency with the broader refactoring effort across all component files.
src/components/vv-accordion-group.scss (1)
8-14: Parameter refactoring verified across all call sites.All 32
spread-map-into-bemcalls consistently use the standardized parameters ($map, $block, $use-custom-props, $zero-specificity, $bps) with no remnants of the old $props parameter. All 9spread-theme-componentcalls in the theme modules include the $zero-specificity parameter. The mechanical refactoring is complete and uniformly applied.src/components/vv-input-text.scss (1)
13-13: LGTM! Parameter rename applied correctly.The parameter rename from
$propsto$use-custom-propsis consistent with the standardization effort described in the PR objectives.auto-imports.d.ts (1)
91-91: Verify the inclusion ofrefManualResetin this PR.The addition of
refManualResetfrom@vueuse/coreis technically correct and follows the established pattern. However, this TypeScript change appears unrelated to the SCSS parameter standardization described in the PR objectives. Can you confirm this was intentionally included in this PR?Also applies to: 400-400
src/themes/dark/components/_vv-button.scss (1)
12-13: LGTM! Zero-specificity parameter added correctly.The addition of the
$zero-specificityparameter to thespread-theme-componentmixin is consistent with the PR objectives and follows the established pattern.src/themes/dark/components/_vv-nav.scss (1)
12-13: LGTM! Zero-specificity parameter added correctly.The addition of the
$zero-specificityparameter is consistent with the PR objectives and maintains uniformity across theme components.src/themes/dark/components/_vv-dropdown-option.scss (1)
12-13: LGTM! Zero-specificity parameter added correctly.The changes are consistent with the zero-specificity support being added across all theme components.
src/components/vv-tab.scss (1)
11-11: LGTM! Parameter rename applied correctly.The parameter rename from
$propsto$use-custom-propsis consistent with the standardization effort across all components.src/themes/dark/components/_vv-avatar.scss (1)
12-13: LGTM! Zero-specificity parameter added correctly.The changes align with the zero-specificity support being rolled out across theme components.
src/themes/dark/components/_vv-alert.scss (1)
12-13: LGTM! Zero-specificity parameter added correctly.The addition of the
$zero-specificityparameter completes the consistent implementation across all reviewed theme components.src/components/vv-progress.scss (1)
8-14: LGTM!Consistent parameter rename from
$propsto$use-custom-props.src/components/vv-checkbox.scss (1)
9-15: LGTM!Consistent parameter rename from
$propsto$use-custom-props.src/components/vv-accordion.scss (1)
8-14: LGTM!Consistent parameter rename from
$propsto$use-custom-props.src/components/vv-textarea.scss (1)
10-16: LGTM!Consistent parameter rename from
$propsto$use-custom-props.src/components/vv-nav.scss (1)
8-14: LGTM!Consistent parameter rename from
$propsto$use-custom-props.src/components/vv-select.scss (1)
10-16: LGTM!Consistent parameter rename from
$propsto$use-custom-props.src/themes/dark/components/_vv-select.scss (1)
7-14: Zero-specificity parameter implementation is correct.The
$zero-specificityparameter is properly supported in thespread-theme-componentmixin definition (src/tools/mixin-modules/_theme.scss) with a default value oftrue. The mixin call in this file correctly passes the parameter, aligning with the PR objectives for zero-specificity rule support in theme mixins.src/components/vv-text.scss (1)
8-14: Parameter rename is complete across all call sites.Verification confirms that all calls to
spread-map-into-bemthroughout the codebase have been successfully updated from$propsto$use-custom-props. No instances of the old parameter name remain.src/components/vv-dropdown-optgroup.scss (1)
8-14: LGTM!The parameter rename from
$propsto$use-custom-propsaligns with the standardized mixin signature, and the$zero-specificityparameter is correctly passed through.src/components/vv-button.scss (1)
9-34: LGTM!Both
spread-map-into-bemandspread-map-into-attrscalls correctly use the renamed$use-custom-propsparameter. The custom block pattern withwrap-with-whereproperly leverages the zero-specificity feature.src/tools/mixin-modules/_bem.scss (5)
176-278: LGTM!The
spread-map-into-modifiersmixin correctly propagates$use-custom-propsthrough all internal calls tospread-map-into-attrs,spread-map-into-elements,spread-map-into-states, and recursivespread-map-into-modifierscalls.
284-391: LGTM!The
spread-map-into-elementsmixin properly passes$use-custom-propstospread-map-into-attrs,spread-map-into-states, andspread-map-into-breakpoints.
397-492: LGTM!The
spread-map-into-statesmixin correctly handles$use-custom-propspropagation through all internal mixin calls.
580-667: LGTM!The
spread-map-into-breakpointsmixin properly propagates$use-custom-propsthrough all breakpoint operator branches (up,down,between).
673-750: LGTM!The
spread-map-into-transitionsmixin correctly passes$use-custom-propstospread-map-into-attrs,spread-map-into-elements, andspread-map-into-breakpoints.package.json (1)
208-235: LGTM!The props export mappings have been relocated within the exports object for better organization. The public API surface remains unchanged—only the declaration order has been adjusted to group props exports after components.
src/tools/mixin-modules/_theme.scss (7)
6-6: LGTM!The
@use 'helpers' as *;import is required for thewrap-with-wheremixin used throughout this file for zero-specificity wrapping.
59-68: LGTM!The
spread-theme-componentmixin signature correctly adds$zero-specificity: trueas a new parameter with a sensible default that maintains backward compatibility.
97-104: LGTM!Block styles are now conditionally wrapped with
:where()based on the$zero-specificityflag, enabling zero-specificity rules when enabled.
156-200: LGTM!The
_theme-generate-allhelper correctly propagates$zero-specificityto all nested calls (_theme-spread-modifiers,_theme-spread-elements,_theme-spread-states) and useswrap-with-wherefor the block selector.
207-261: LGTM!The
_theme-spread-modifiersmixin properly propagates$zero-specificitythrough all recursive and nested calls, ensuring consistent zero-specificity behavior for modifier, state, and element rules.
264-295: LGTM!The
_theme-spread-elementsmixin correctly handles$zero-specificitypropagation for element selectors and nested state calls.
298-373: LGTM!The
_theme-spread-statesmixin comprehensively applieswrap-with-whereto all state selector variants (BEM modifier style, class style, attribute-based, and pseudo-class style), ensuring consistent zero-specificity behavior across all state representations.
|
alessandrobellesia
deleted the
feature/fix-zero-specificity-for-themes
branch
Add $zero-specificity parameter to theme mixins with :where() wrapping Rename $props to $use-custom-props across all mixins for consistency Update spread-map-into-attrs mixin to use $use-custom-props parameter Update all mixin calls in components and theme files Remove obsolete $props parameter from spread-map-into-bem mixin
Summary by CodeRabbit
New Features
refManualResetadded and exposed with TypeScript support.Chores
✏️ Tip: You can customize this high-level summary in your review settings.