Allow fewer breakpoints for navbars#38909
Conversation
pine3ree
force-pushed
the
custom-navbar-breakpoints
branch
2 times, most recently
from
df80866 to
f703036
Compare
Signed-off-by: pine3ree <pine3ree@gmail.com>
Signed-off-by: pine3ree <pine3ree@gmail.com>
pine3ree
force-pushed
the
custom-navbar-breakpoints
branch
from
f703036 to
55dffaa
Compare
louismaximepiton
left a comment
louismaximepiton
left a comment
There was a problem hiding this comment.
If this can help people, I see no reason why it shouldn't be added. It doesn't increase the bundle and offer more possibility to customize the framework.
Thanks for this PR!
|
|
||
| // scss-docs-start navbar-breakpoints | ||
| $navbar-breakpoints: $grid-breakpoints !default; | ||
| // scss-docs-end navbar-breakpoints |
There was a problem hiding this comment.
This is never used, should be removed or used imo.
There was a problem hiding this comment.
Hello @louismaximepiton. The extra sass var is used to keep the two breakpoints separate. If we alter the $grid-breakpoints map before the inclusion of _maps we implicitly alter the $navbar-breakpoints as well. If we remove only from $navbar-breakpoints we can keep the $grid-breakpoints variable as defined inside the configuration file. e.g.
- Define before the default value
// @import...
$navbar-breakpoints: map-remove($grid-breakpoints, "xl", "xxl");
@import "../node_modules/bootstrap/scss/maps";
// @import...- Override (after) the default value
// @import...
@import "../node_modules/bootstrap/scss/maps";
$navbar-breakpoints: map-remove($navbar-breakpoints, "xl", "xxl");
// @import...Now that I think about it, maybe it would be better to define the $navbar-breakpoints: $grid-breakpoints !default; in the main variables configuration file, like I did in the first version. Otherwise I would need to change the added documentation, that could lead to confusion.
kind regards
There was a problem hiding this comment.
Yes I'm a bit confused,
I think I didn't explain well my thought on this.
I mean the change you mention is totally fine. My comment was more about the // scss-docs* thing which isn't used in the doc. Or did I miss something ?
Sorry for the confusion and thanks again for the appreciated contribution.
There was a problem hiding this comment.
@louismaximepiton Ah, ok. :-) I thought you meant the actual variable...not the docs. But I think it would make more sense either:
- to move the extra variable into the main configuration file, or
- to simply keep only the instruction to define the navbar-breakpoints using the grid-breakpoints var (as in my previous example 1) according to bootstraps docs that recommend to add/remove to/from maps before including the
_maps.scssfile.
kind regards
maps file Signed-off-by: pine3ree <pine3ree@gmail.com>
|
Added to v6-dev in #42048. Will backport to v5.4.0 when ready. |
4 participants
Description
Add the ability to remove navbar breakpoints
(Rebased PR replacing #36521)
Motivation & Context
We usually do not need so many breakpoints for navbars as for grids: see #36517 for feature description.
Type of changes
Checklist
npm run lint)Related issues
Related issue/discussion