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

Bootstrap 5 Renderer #84

Closed
wants to merge 5 commits into from
Closed

Bootstrap 5 Renderer #84

wants to merge 5 commits into from

Conversation

coolacid
Copy link
Contributor

@coolacid coolacid commented Jan 2, 2024

This PR adds a new BootStrapRenderer which provides the necessary render functions to work with Bootstrap 5.

This is tested using the bootstrap_flask python package.

When initializing nav, you pass a bootstrap=True to the init.

nav = Nav()
nav.init_app(app, bootstrap=True)

@coolacid
Copy link
Contributor Author

coolacid commented Jan 3, 2024

Refactored as much as I can to meet flake8 requirements. Not sure where to go from here.

@qs5779
Copy link
Contributor

qs5779 commented Jan 3, 2024

Seems like only two minor issues, left. I will see if I can tweak them. Curiously though, can you explain why this is needed a little better? I use this in a flask app of mine that also uses the flask_bootstrap package and it is working for me.

@coolacid
Copy link
Contributor Author

coolacid commented Jan 3, 2024

Thanks. If I remember correctly, the CSS for Bootstrap 5 changed a bunch of items causing the drop down sub navs to not work? I honestly started the work a while ago and only now put the final touches to get it all working as expected.

@qs5779
Copy link
Contributor

qs5779 commented Mar 6, 2024

Closing.

@qs5779 qs5779 closed this Mar 6, 2024
@coolacid
Copy link
Contributor Author

coolacid commented Mar 6, 2024

Any details on to why this was closed?

@qs5779
Copy link
Contributor

qs5779 commented Mar 6, 2024

First of all, I apologize. I thought we had discussed this earlier, but I must have confused it with another PR. I closed it mainly because I don't know off the top of my head how to implement the request you are making, however if you do I would happily accept a PR that doesn't break any existing functionality.

@coolacid
Copy link
Contributor Author

coolacid commented Mar 6, 2024

This doesn't break any current functionality. There is a configuration bool to turn on and use this code path.

@qs5779
Copy link
Contributor

qs5779 commented Mar 7, 2024

When I build with your previous PR and tested with one of my flask projects, my navigation did not work correctly regardless of whether I specified bootstrap = True, or bootstrap = False.

@coolacid
Copy link
Contributor Author

I only have the one PR. I'll test with the existing example code to see if I can see why it wouldn't be working.

Mind re-opening for work to continue?

@coolacid
Copy link
Contributor Author

So, I just tried to use the example code out of the box as it sits in this repo. There is no nav bar, just the nav objects. This is because the example code doesn't include any CSS code what so ever to provide drop down or nav bar solutions. I'll investigate getting a working example working and apply to this PR.

@coolacid coolacid mentioned this pull request Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants