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

Breadcrumbs in admin #226

Open
mkurek opened this issue Dec 30, 2016 · 2 comments
Open

Breadcrumbs in admin #226

mkurek opened this issue Dec 30, 2016 · 2 comments

Comments

@mkurek
Copy link

mkurek commented Dec 30, 2016

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'
@idlesign
Copy link
Owner

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
Copy link
Owner

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.

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

2 participants