Coherent default for focus box in select
Description
Change default setting for $form-select-focus-box-shadow.
Motivation & Context
$form-select-focus-box-shadow doesn't fallback on common settings about focus boxes, and has a different behavior when applying customizations described in the documentation (just setting $focus-ring-box-shadow does not impact on select boxes).
Type of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Refactoring (non-breaking change)
- [X] Breaking change (fix or feature that would change existing functionality)
This is actually a breaking change, as it alters an established behavior (even if wrong).
Checklist
- [X] I have read the contributing guidelines
- [X] My code follows the code style of the project (using
npm run lint) - [ ] My change introduces changes to the documentation
- [ ] I have updated the documentation accordingly
- [ ] I have added tests to cover my changes
- [X] All new and existing tests passed
Live previews
- https://www.madbob.org/tests/bootstrap-select-focus/plain-bootstrap/ - vanilla Bootstrap
- https://www.madbob.org/tests/bootstrap-select-focus/fixed-bootstrap/ - version with this change
Oh hmm, this does cause errors with $form-select-focus-width being unused. Unsure how I'd want to tackle yet.
Oops... My fault, I just looked to the output of npm run lint without the necessary attention.
Removing both the unused variables ($input-btn-focus-color and $form-select-focus-width) seems to have no side effects.
More in general, I feel that all the $input-btn-focus-* variables are just aliases for $focus-ring-* and should be suppressed. $btn-focus-* already permit customization of buttons over inputs and can be defaulted directly to $focus-ring-*, like in a few other situations.
My proposal: fix the behavior of select with this PR (or a new, clean one, as I messed a bit around trying to loosely sync my branch...), and dive a bit deeper to remove some other redundant variable.
Yeah there are too many reused variables, so that's a thing I'm working on in v6. Need to reduce these things down a bit. As for this branch, we could mark those other variables as deprecated here and merge this, but I'd need @julien-deramond's thoughts on that.
As for this branch, we could mark those other variables as deprecated here and merge this
Hey! I’ve added this commit to retain the Sass variables and mark them as deprecated using fusv, following the approach we've used so far.
Since this would introduce breaking changes, we’ll need to hold off on merging the PR until v5.4.0 to avoid issues in a patch release. I’ve moved this PR to the v5.4.0 project.