django-cms / djangocms-frontend

django CMS frontend is a plugin bundle for django CMS providing several components for the frontend, currently implemented with the popular Bootstrap 5 framework.
Other
51 stars 21 forks source link

fix: Added template field to nav container #23

Closed marksweb closed 2 years ago

marksweb commented 2 years ago

The nav container template can't currently be changed from the default. This adds the template field to the form.

The nav link also has this problem.

codecov[bot] commented 2 years ago

Codecov Report

Merging #23 (1f81891) into master (b32148c) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #23   +/-   ##
=======================================
  Coverage   78.50%   78.50%           
=======================================
  Files         128      128           
  Lines        3945     3946    +1     
  Branches      790      790           
=======================================
+ Hits         3097     3098    +1     
  Misses        748      748           
  Partials      100      100           
Impacted Files Coverage Δ
...angocms_frontend/contrib/navigation/cms_plugins.py 98.68% <ø> (ø)
djangocms_frontend/contrib/navigation/forms.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b32148c...1f81891. Read the comment docs.

fsbraun commented 2 years ago

@marksweb I was just thinking about the navigation templates: Do set the template for each element can be quite cumbersome and if you have many navigation elements changing it would mean to edit each element. Since all navigation plugins inherit from NavigationPlugin shouldn't all other navigation plugin also inherit its template selection? Then you would only need to set and change it once for a navigation.

Also, I am considering to change the default to navbar for the classical to navbar. If needed users could add off-canvas or sidebar or whatever.

What do you think?

marksweb commented 2 years ago

@fsbraun yeah it makes sense for the parent container to set the template path for the children then to inherit. I considered doing this last night but didn't have time so took this easier interim approach!

fsbraun commented 2 years ago

@marksweb I'll go for it!