Skip to content
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

navbar-collapse used with dropdown-toggle #41049

Open
3 tasks done
CJ-Pav opened this issue Nov 22, 2024 · 5 comments · May be fixed by #41081
Open
3 tasks done

navbar-collapse used with dropdown-toggle #41049

CJ-Pav opened this issue Nov 22, 2024 · 5 comments · May be fixed by #41081

Comments

@CJ-Pav
Copy link

CJ-Pav commented Nov 22, 2024

Prerequisites

Describe the issue

Using a drop down navbar item within a navbar collapse seems to break bootstraps styling. Specifically, the shrink to fit breaks, and expands to the full screen width (with some padding). I tested in 5 different screen sizes and found it looks fantastic on larger screens. The issue I am noticing really only impacts medium sized screens (tablets and small laptops). See the screenshots below. On a larger screen the dropdown box shrink wraps the button options within. On a medium sized screen, it blows up across the whole screen. This makes sense for phones, but on tablets over 60% of the screen width is wasted.

Large screens look great:
image-6
Medium screens not so much (looks fine on small using these same styles):
image-5

Reduced test cases

https://codepen.io/CJ-Pav/pen/mdNNPJy

What operating system(s) are you seeing the problem on?

Windows, macOS, Linux

What browser(s) are you seeing the problem on?

Chrome, Firefox

What version of Bootstrap are you using?

v5.2.3

@antoinevilain001
Copy link

Hi! I'm not from the dev team but just took a bit of a look to see how this menu can be changed. I agree that having the big white rectangle appear isn't necessarily the nicest. From tinkering around a bit with it so far one option could be to just remove the border and decrease the padding a little bit like so:
image
Another option could be to have it appear as a pop-out like for the large screens:
image
I imagine there must be a reason why they chose to have it how it is, but potentially something like one of the above could make it look better in this particular scenario?

@CJ-Pav
Copy link
Author

CJ-Pav commented Dec 6, 2024

The second image you posted looks great to me. I'll have to see if I can reproduce it, thanks for sharing! I can get close by using the built in navbar-expand-md instead of lg but I really would prefer this type of hybrid option. The navbar expand with the separated dropdown looks clean to me and still allows the navbar to handle the additional navbar options that come with production environments that otherwise wouldn't fit on medium screens.

@CJ-Pav
Copy link
Author

CJ-Pav commented Dec 6, 2024

I actually think this would look better on small screens as well, comparing our result to that on my phone. That said, I'll stick to the initial plan and share my fix for in-between sized screens (smaller and larger screens use previous behavior). Here's a demo of the fix, I added the fix to the navbar.scss file and it compiles with your linting software to product the css in the demo.

I read that we could make pull requests without being on the core team, I'd be happy to continue working on this outside of this pull request/issue. Just let me know if you are looking for the contribution. I'd love to become part of your team or continue as a volunteer if that is an option. I have created a branch and am unable to push it for review.

https://codepen.io/CJ-Pav/pen/bNbpVKG

image

Added code:
image

@antoinevilain001
Copy link

Hi again, glad to see you managed to get the navbar working to something you like better! I myself have also never contributed to this project but after your comments I decided to give it a shot and was able to successfully open a pull request with the changes I had when showing you those screenshots, so I am surprised that you weren't able to.

I know your approach was slightly different than mine so I still think it could be worth it for you to submit a pull request (obviously I'm not an owner of this project so I have no clue whether this is even a change they would want). Following the steps in https://github.com/twbs/bootstrap/blob/ec96eacd0e6f297a64ee058b22ce9f567c0860e3/.github/CONTRIBUTING.md worked for me, I'll try and describe what I did in a little more detail though in case that solves your problem of being unable to push.

Personally what worked for me was:

  1. fork the repository, clone into this (was awhile ago so don't remember the exact commands)
  2. git checkout -b
  3. git add, git commit to the new branch in my own repository
  4. git remote add upstream https://github.com/twbs/bootstrap.git
  5. git push origin
  6. now if I go to my own forked repository on github and click on my branch it prompts me to create a pull request and this pull request by default is into the main of this original bootstrap repository.

Hope this helps!

P.S. If you're curious the only thing I had changed was to make the thing absolute, seems like you have some more ideas to make it better though: #41080

@CJ-Pav CJ-Pav linked a pull request Dec 6, 2024 that will close this issue
10 tasks
@CJ-Pav
Copy link
Author

CJ-Pav commented Dec 6, 2024

#41081

I'm not sure what I did the first time but thanks for encouraging a second attempt! Re-cloned and fresh keys and it worked fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants