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

Allow extra classes on render_icon #336

Open
ElSaico opened this issue Oct 17, 2023 · 3 comments
Open

Allow extra classes on render_icon #336

ElSaico opened this issue Oct 17, 2023 · 3 comments
Assignees

Comments

@ElSaico
Copy link

ElSaico commented Oct 17, 2023

Use case: the light/dark toggler from Bootstrap's own documentation makes use of classes such as me-2 for spacing and theme-icon-active for query selecting.

@PanderMusubi
Copy link
Collaborator

Was thinking about this earlier. Thanks for the request. Working on a PR now.

@PanderMusubi
Copy link
Collaborator

@greyli In the example, why do the links at the end of the nav menu use <li>...</li> and the render_nav_item() don't?

    <a class="nav-item nav-link"
       href="[/icon](view-source:http://127.0.0.1:5000/icon)">
        Icon 
    </a>
    <a class="nav-item nav-link"
       href="[/icons](view-source:http://127.0.0.1:5000/icons)">
        Icons 
    </a>
                <li class="nav-item"><a class="nav-link" href="[https://bootstrap-flask.readthedocs.io/](view-source:https://bootstrap-flask.readthedocs.io/)" target="_blank">Documentation</a></li>
                <li class="nav-item"><a class="nav-link" href="[https://getbootstrap.com/docs/5.1/getting-started/introduction/](view-source:https://getbootstrap.com/docs/5.1/getting-started/introduction/)" target="_blank">Bootstrap Documentation</a></li> 

See also https://github.com/helloflask/bootstrap-flask/blob/master/examples/bootstrap5/templates/base.html#L41 and https://getbootstrap.com/docs/5.3/components/scrollspy/#non-visible-elements

Both work, but what is reommended? With out without the <li>?

@greyli
Copy link
Member

greyli commented Oct 18, 2023

I'm not sure. Users can control this with _use_li parameter based on their preference, so I don't think we need to change anything.

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

No branches or pull requests

3 participants