django-sitetree icon indicating copy to clipboard operation
django-sitetree copied to clipboard

Breadcrumbs in admin

Open mkurek opened this issue 7 years ago • 2 comments

Why admin app has special treatment for breadcrumbs?

In breadcrumbs method there is such piece of code:

current_item = self.get_tree_current_item(tree_alias)

get_tree_current_item returns None for admin app, which leads to empty breadcrumbs here.

We've build our whole app at the top of django admin and we want to use breadcrumbs here too (in fact, we're using sitetree's breadcrumbs in our app using sitetree in version 1.5.0*, where everything is working fine). I don't see any point in not providing breadcrumbs for admin app - sitetree could return list of breadcrumbs, but until sitetree_breadcrumbs is called, nothing happen.

I could provide PR with fix/change for this, but first I want to discuss the idea behind current solution.

*In version 1.5 there was simple check if app is admin app (maybe it wasn't perfect, but it was working in our case) - this simple check was as follows:

current_app = getattr(
        self._global_context.get('request', None), 
        'current_app', 
        self._global_context.current_app
)
return current_app == 'admin'

Since then, in newest version, additional check was added (notice checking app name in resolver):

current_app = getattr(
    # Try from request.resolver_match.app_name
    getattr(context.get('request', None), 'resolver_match', None), 'app_name',
    # Try from global context obj.
    getattr(context, 'current_app', None))

if current_app is None:  # Try from global context dict.
    current_app = context.get('current_app', '')

is_admin = current_app == 'admin'

mkurek avatar Dec 30 '16 17:12 mkurek

Thank you for the report.

This was a compatibility shim — reported in #201, addressed in #204 Right now I can't remember why there is no bypass for breadcrumbs, so I should take a pause for one day.

It might be somehow related to #84 Custom admin related theme was also raised in #185

idlesign avatar Dec 30 '16 17:12 idlesign

It seems we need some tests to check breadcrumbs in conjunction with dropdowns (e.g. TreeItemChoiceField). After that it'll probably be safe to bypass admin check for breadrumbs. Tests and pull request are welcome.

idlesign avatar Dec 31 '16 08:12 idlesign