idlesign / django-sitetree

Reusable application for Django introducing site tree, menu and breadcrumbs navigation elements.
http://github.com/idlesign/django-sitetree
BSD 3-Clause "New" or "Revised" License
347 stars 132 forks source link

add bootstrap5 templates #304

Closed PetrDlouhy closed 3 years ago

PetrDlouhy commented 3 years ago

I have added templates for Bootstrap 5. The only difference, that is really needed is renaming of data-toggle to data-bs-toggle.

But I also added the {{ extra_class}} attribute which I found very handy. It can be used this way:

{% with extra_class="flex-wrap flex-row" %}
   {% sitetree_menu from "footer_3" include "trunk,topmenu" template "sitetree/menu_bootstrap5.html" %}
{% endwith %}
coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 95.827% when pulling d3fe2d41e2a0bd0a554d0994f0c1221f6f2f17b0 on PetrDlouhy:bootstrap5 into 86853693d2983bb43b2f408ff98da4b7d2b75e62 on idlesign:master.

idlesign commented 3 years ago

Thank you.

This class thing could be useful indeed, so I'd like to ask you also to update the docs: to mention this extra class and new templates. https://github.com/idlesign/django-sitetree/blob/master/docs/source/templatesmod.rst

And another thing: it'd probably more flexible if we call that extra_class_ul. This way we'll be able to add extra_class_li etc. in future if decided.

PetrDlouhy commented 3 years ago

@idlesign I have changed the PR to your suggestions.

BTW There is 1 more thing, for which I have to override the whole template. I am using font-awesome icons, so I need to make the item.title_resolved safe (see https://github.com/idlesign/django-sitetree/issues/91#issuecomment-870688223). I don't think, that is wise to add something like that to templates generally (in some systems users could have access to the sitetree editing). But I can either do something like

{% block title %}{{ item.title_resolved }}{% endblock %}

so that only this part of the template needs to be overriden, or it can be done by some settings (like SITETREE_FORCE_SAFE=True). What do you think is better?

idlesign commented 3 years ago

@idlesign I have changed the PR to your suggestions.

Thank you. There's the pending issue though — https://github.com/idlesign/django-sitetree/pull/304#discussion_r683518130

I don't think, that is wise to add something like that to templates generally (in some systems users could have access to the sitetree editing).

This has no connection to Bootstrap template. Let's open another issue to discuss it, if it's required. Yet, I'd rather try to play with mark_safe() somehow before anything else.

PetrDlouhy commented 3 years ago

@idlesign Sorry about the pending issue - I didn't push it properly yesterday.

The issue regarding HTML in title_resolved is here: https://github.com/idlesign/django-sitetree/issues/305

idlesign commented 3 years ago

Thank you. Merged.