fix(bootstrap-utilities): update sass in bootstrap utilities files #68#93
fix(bootstrap-utilities): update sass in bootstrap utilities files #68#93
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 45 minutes and 11 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request modernizes Sass syntax across three utility files by replacing deprecated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bootstrap-utility/_mixin-utilities.scss (1)
1-95:⚠️ Potential issue | 🟠 MajorAdd minimum Dart Sass version constraint for consumers.
The code now uses
if(sass(...): ...; else: ...)syntax which requires Dart Sass 1.95.0+. This minimum version needs to be communicated to library consumers via:
- Add
"sass": "^1.95.0"topeerDependenciesin package.json- Update the
enginesfield in package.json to include the Sass version requirement- Document the minimum Sass version in README
Currently, Sass is only in devDependencies and the requirement is not documented anywhere, which means consumers could use incompatible versions and encounter compilation failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bootstrap-utility/_mixin-utilities.scss` around lines 1 - 95, The mixin generate-utility uses the new if(sass(...): ...; else: ...) syntax (see occurrences of if(sass(...)) in _mixin-utilities.scss), so add a peer dependency "sass": "^1.95.0" to package.json, update package.json.engines to include a Sass requirement (e.g. "sass": ">=1.95.0"), and document the minimum Dart Sass version (Dart Sass 1.95.0+) in the README; ensure Sass remains in devDependencies for development but communicate the peer requirement to consumers to avoid compilation failures.
🧹 Nitpick comments (1)
src/bootstrap-utility/_mixin-utilities.scss (1)
33-33: Consider extracting the inner conditional for readability.The nested
if()syntax is valid and fully supported by Dart Sass—the inner semicolon correctly stays within parentheses. However, the line is complex and hard to parse at a glance. Extracting the inner condition into a temporary variable improves maintainability:♻️ Suggested readability refactor
- $property-class-modifier: if(sass($key): if(sass($property-class == '' and $infix == ''): ''; else: '-') + $key; else: ''); + $separator: if(sass($property-class == '' and $infix == ''): ''; else: '-'); + $property-class-modifier: if(sass($key): $separator + $key; else: '');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bootstrap-utility/_mixin-utilities.scss` at line 33, Extract the complex nested if() inside the $property-class-modifier assignment into a temporary variable to improve readability: compute the inner expression that currently reads if(sass($property-class == '' and $infix == ''): ''; else: '-') + $key (or the entire sass($key): ... branch as appropriate) into a clearly named local variable (e.g. $property-class-suffix or $key-suffix) and then use that variable inside the $property-class-modifier expression; update references to $key and the sass() checks (the symbols $property-class-modifier, $property-class, $infix, $key and sass()) so the original logic is preserved but the outer assignment becomes a simple composition of the temp variable and $key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bootstrap-utility/_mixin-utilities.scss`:
- Line 41: The current Sass if() call sets $value to null unconditionally
because the else branch is missing; change the expression in the assignment that
uses if(sass($val == rfs-fluid-value($value)) ...) so it returns null when true
but returns the original $value when false (restore the three-argument form),
ensuring $value remains non-null when appropriate and allowing the subsequent
`@if` $value != null guard to work correctly.
- Line 76: The `@each` $pseudo in $state block uses the bare variable
$enable-important-utilities which is defined in the vars module; update that
usage to reference the namespaced variable (vars.$enable-important-utilities) so
it matches the import and avoids an undefined variable error—search for the
$enable-important-utilities use inside the `@each` $pseudo in $state block and
replace it with vars.$enable-important-utilities to match the pattern used
elsewhere (e.g., the existing line that uses vars.$enable-important-utilities).
---
Outside diff comments:
In `@src/bootstrap-utility/_mixin-utilities.scss`:
- Around line 1-95: The mixin generate-utility uses the new if(sass(...): ...;
else: ...) syntax (see occurrences of if(sass(...)) in _mixin-utilities.scss),
so add a peer dependency "sass": "^1.95.0" to package.json, update
package.json.engines to include a Sass requirement (e.g. "sass": ">=1.95.0"),
and document the minimum Dart Sass version (Dart Sass 1.95.0+) in the README;
ensure Sass remains in devDependencies for development but communicate the peer
requirement to consumers to avoid compilation failures.
---
Nitpick comments:
In `@src/bootstrap-utility/_mixin-utilities.scss`:
- Line 33: Extract the complex nested if() inside the $property-class-modifier
assignment into a temporary variable to improve readability: compute the inner
expression that currently reads if(sass($property-class == '' and $infix == ''):
''; else: '-') + $key (or the entire sass($key): ... branch as appropriate) into
a clearly named local variable (e.g. $property-class-suffix or $key-suffix) and
then use that variable inside the $property-class-modifier expression; update
references to $key and the sass() checks (the symbols $property-class-modifier,
$property-class, $infix, $key and sass()) so the original logic is preserved but
the outer assignment becomes a simple composition of the temp variable and $key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b324994d-98e5-43d3-8a20-8df741817c4d
📒 Files selected for processing (3)
src/bootstrap-utility/_breakpoints.scsssrc/bootstrap-utility/_grid.scsssrc/bootstrap-utility/_mixin-utilities.scss
1 participant
Summary by CodeRabbit