-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Edit navbar-nav dropdown-menu appearance on condensed navbars #41080
base: main
Are you sure you want to change the base?
Edit navbar-nav dropdown-menu appearance on condensed navbars #41080
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I'm not against this change, let's wait for @mdo feedback as it's a design rendering decision to make.
Adding the "design" label to this PR and putting it in the v5.4.0 project as it would be a new rendering by default.
Anyway, if this change lands, there will be a modification to do at least in our documentation header as the current deployed version looks like this:
It should maybe left-aligned in this case.
scss/_navbar.scss
Outdated
@@ -108,7 +108,7 @@ | |||
} | |||
|
|||
.dropdown-menu { | |||
position: static; | |||
position: absolute; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried to remove entirely the .dropdown-menu
rule here? We might have this absolute position by default (haven't tried).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave this a shot and looks like you are correct, it appears to behave the exact same when removing the rule entirely. I've updated this branch accordingly with a new commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite see what should be changed about that documentation header, from what I've seen it still looks the same in the new version as in the old version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite see what should be changed about that documentation header, from what I've seen it still looks the same in the new version as in the old version?
Reference at https://getbootstrap.com:
In this branch, the dropdown menu is too close to the left edge.
…nu so removing entirely
Description
Currently the expanded navbar with a dropdown menu looks like this:
And the condensed navbar with a dropdown menu looks like this:
The proposed fix would make the condensed dropdown menu always pop out as with the expanded navbar, like so:
Motivation & Context
This is purely a cosmetic change, The intention is to make the navbar look better as the current version does not look so nice.
Type of changes
Checklist
npm run lint
)npm test currently failing with the below signature but I don't believe this is due to my changes as it occurs when I run on an unchanged main branch as well. I will open a new issue about this.
Running vnu-jar validation...
command used: java -jar "C:...my_path...\bootstrap\node_modules\vnu-jar\build\dist\vnu.jar" --asciiquotes --skip-non-html --Werror --filterpattern "Attribute “autocomplete” is only allowed when the input type is.*|Attribute “autocomplete” not allowed on element “button” at this point.|An “aria-disabled” attribute whose value is “true” should not be specified on an “a” element that has an “href” attribute." _site/ js/tests/
Live previews
https://deploy-preview-41080--twbs-bootstrap.netlify.app/docs/5.3/components/navbar/#offcanvas
Related issues
Addresses #41049