elky / django-flat-theme

A flat theme for Django admin interface. Modern, fresh, simple.
Other
413 stars 46 forks source link

Providing `base_site.html` breaks compatibility with end-user projects. #30

Open mrmachine opened 8 years ago

mrmachine commented 8 years ago

As mentioned here: https://github.com/elky/django-flat-theme/commit/49c9b059de3398577417234399a94b5a6ece4c30

This change probably breaks compatibility with any existing project that defines their own admin/base_site.html template, which I believe is the recommended way for people to extend the Django admin template without duplicating base.html.

Changing the order of INSTALLED_APPS won't help. It's either our base_site.html or yours.

Is there another way you can implement django-flat-theme without adding the flat-theme class directly to the body tag in this template? Maybe django-flat-theme can just live without it? Or you could duplicate the base.html template. That would be ideal for people wanting to use django-flat-theme with their own projects, as it would remain more of a drop in replacement.

elky commented 8 years ago

The other way is to add template loader in settings.py. I think it's the best way to avoid base_site.html issues. I'll implement is soon.

elky commented 8 years ago

@mrmachine yeah you probably right -- I didn't find any good solution how to fix this problem.

@andybak seems I have to roll back latest change about adding css class to the admin body because it breaks user's custom base_site.html. So if you want to customize flat-theme's style -- just override base_site.html in your project manually.

andybak commented 8 years ago

I might be missing something but what's the issue here?

If you're using Flat theme and not overriding base_site.html: no problem

If you're using Flat theme and you are overriding base_site.html: Ensure correct ordering in INSTALLED_APPS and include {% block bodyclass %}flat-theme{% endblock %} in your template.

mrmachine commented 7 years ago

@andybak The issue is that flat-theme is no longer a drop in replacement for the default admin theme. It requires you to alter your existing base_site.html (if you have one). The docs don't mention this, and claim it is as simple as adding flat to INSTALLED_APPS (in the right order).

We have a pluggable app that provides a base_site.html file (a customised admin dashboard) which we would like to be usable both with and without flat. But we can't. So we have pinned to flat<1.1.3.

It's not possible for two pluggable apps to override the same Django template even if each only overrides different blocks.

andybak commented 7 years ago

I've got myself a touch confused here.

Is this correct?:

"The only reason flat requires base_site.html is to add a class to the body tag" (as requested in #26 )

If so - then I agree - it's more trouble than it's worth and users can add their own class. HOWEVER - some consensus is required on the correct class to add so that 3rd party apps can all use the same class to target the flat theme. So - would it be acceptable to add a note to the docs to this effect?:

Some 3rd Party Apps might want their CSS to target only flat admin. If so they use should use the class .flat-theme and users should add this class to the body tag in their own admin/base_site.html if needed.

mrmachine commented 7 years ago

That sounds right to me.