Skip to content

Consolidate multiple 'none' values in box-shadow Sass mixin#41469

Merged
mdo merged 1 commit intomainfrom
main-jd-enhance-box-shadow-mixin
May 20, 2025
Merged

Consolidate multiple 'none' values in box-shadow Sass mixin#41469
mdo merged 1 commit intomainfrom
main-jd-enhance-box-shadow-mixin

Conversation

@julien-deramond
Copy link
Copy Markdown
Member

@julien-deramond julien-deramond commented May 14, 2025

Description

This PR fixes an issue with the box-shadow mixin where multiple none values could result in invalid CSS combinations like box-shadow: none none none. The fix consolidates multiple none values (and other single-value keywords like initial, inherit, unset) into a single value.

The changes are:

  • Modified box-shadow mixin to detect and consolidate single-value keywords
  • Added comprehensive test suite to verify the fix and other edge cases

They prevent invalid CSS output while maintaining backward compatibility with existing usage of the mixin.

These changes are covered by new tests that verify:

  • Single and multiple none values
  • Other CSS keywords (initial, inherit, unset)
  • Regular box-shadow values
  • Mixed keyword and shadow values
  • Null value handling
  • $enable-shadows variable behavior

Hopefully, these tests cover pretty much every use cases that could happen, reasonably 🤞 Don't hesitate if you're thinking about any other weird combinations; we could add them to the test suite.

I've also tested manually to compile this branch with $enable-shadows: true and compare the generated bootstrap.css with the one generated from the main branch with $enable-shadows: true. Same output.

Type of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • (N/A) My change introduces changes to the documentation
  • (N/A) I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Related issues

Closes #41381

@julien-deramond julien-deramond force-pushed the main-jd-enhance-box-shadow-mixin branch from ac1dd7a to a48f48a Compare May 14, 2025 20:07
@julien-deramond julien-deramond moved this from To do to Needs review in v5.3.7 May 14, 2025
@julien-deramond julien-deramond marked this pull request as ready for review May 14, 2025 20:10
@julien-deramond julien-deramond requested a review from a team as a code owner May 14, 2025 20:10
@mdo mdo merged commit 7f946d4 into main May 20, 2025
14 checks passed
@mdo mdo deleted the main-jd-enhance-box-shadow-mixin branch May 20, 2025 03:53
@github-project-automation github-project-automation Bot moved this from Needs review to Done in v5.3.7 May 20, 2025
Rajshree1126 pushed a commit to Rajshree1126/bootstrap that referenced this pull request May 20, 2025
Consolidate multiple 'none' values in `box-shadow` Sass mixin (twbs#41469)

Test commit to enable PR creation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

When setting both $input-focus-box-shadow and $input-box-shadow to none, Bootstrap generates invalid CSS.

2 participants