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

Broken Admin Breadcrumb #22

Closed
lanceosborne opened this issue Oct 5, 2016 · 10 comments
Closed

Broken Admin Breadcrumb #22

lanceosborne opened this issue Oct 5, 2016 · 10 comments

Comments

@lanceosborne
Copy link

Short description of the issue

The breadcrumb link for the parent page of the current page does not link to the proper page.

In the following breadcrumb example, the "Parent Page" does not link directly to that page; it instead links to /processwire/page/

Pages > Home > [Parent Page] > [Current Page]

In the following example, the "Grandparent Page" link is correct, but the "Parent Page" link is incorrect:

Pages > Home > [Grandparent Page] > [Parent Page] > [Current Page]

Expected behavior

The "Parent Page" in the above example should link to the parent page of the current page.

Actual behavior

The "Parent Page" in the above example instead links to /processwire/page/

Steps to reproduce the issue

This occurs when editing any page in the admin. The incorrect link is always the immediate parent page of the current page.

Setup/Environment

Issue is not due to setup/environment.

  • ProcessWire version: 3.0.33 devns
@ryancramerdesign
Copy link
Member

The behavior here was an intended compromise to to ensure the fast tree gets used. Without it, things are a lot slower. The breadcrumb click is inclusive of the breadcrumb page that was clicked on, even if it might also include other pages as well. But in some cases where you've got multiple tabs open and making edits between them, then it can get mixed up. We could always go back to the old behavior of making the breadcrumb open the page editor for the page, and maybe we'll switch back to that at some point if it's a recurring issue.

@lanceosborne
Copy link
Author

With the current implementation, if I'm viewing the following page in the admin:

Pages > Home > [Grandparent Page] > [Parent Page] > [Current Page]

and click on "Parent Page", I'm returned to the list view in the admin. The downside is that if the "Grandparent Page" is collapsed, I still have to click on the "Grandparent Page" to view the "Parent Page."

@ryancramerdesign
Copy link
Member

The page tree basically remembers where it was last left off, and if used to navigate to the page edited, then there shouldn't be a collapsed grandparent page. But if you navigated to the edited page by some other means (or are working in PW with multiple tabs open to it), then it's true that something might be collapsed like that. So there is a little bit of a compromise in that respect here. In most cases it's a nice speed optimization but not for this specific case. I think for most the speed optimization is preferable, but I suppose we could always make a config option that lets you define how you want the breadcrumbs to work in the page editor.

@Toutouwai
Copy link

Toutouwai commented Oct 7, 2016

My vote would go for:
Clicking "Home", "Pages" or using Tree panel shows last tree state.
Clicking breadcrumb item besides "Home" shows tree expanded on clicked item, other branches collapsed.

@ryancramerdesign
Copy link
Member

The tree should always show where you last left it, even including what pagination you were on. So in addition to the matter of speed, this is why we make the breadcrumb for the parent page (and only that page) point to /processwire/page/. If the page you were editing happened to be on the 99th pagination among a huge set of the pages, then it's going to show that parent on the 99th pagination. But if the breadcrumb instead links to /processwire/page/?open=123 (where 123 is parent id), then there's no way to return to where you actually were (on that 99th pagination) short of manually clicking through all the paginations till you get there. This is not really an issue with grandparent, great-grandparent, etc., which is why we use the "?open=123" for those breadcrumb items. The compromise is that if you navigated to the page editor by some other means, or if you navigated in the page list via another tab/window while you were editing, then the page list is going to be where you left it, rather than the breadcrumb.

What I can do is make it revert to the "open=123" behavior for the 1 parent breadcrumb when the parent's number of children falls below the pagination threshold. There's still a compromise there in that it makes the whole page list run slower for the majority where parent breadcrumb is usually consistent with their last place in the page list, but this might at least prevent confusion for the cases identified here.

@BitPoet
Copy link

BitPoet commented Oct 13, 2016

Would it be possible to check the pagelist_open cookie and, if it was in fact open in the parent (last element is parent id with optional page number, if I read the code right), use the last open feature, otherwise fall back to the classic behavior?

@lanceosborne
Copy link
Author

I'm all for speed when it comes to my clients using the admin. From what I've noticed in training sessions, the breadcrumbs aren't used very often. So in my opinion, speed is a bigger priority for casual users of the backend.

I've kept note my own habits in the past week while developing a large site, and I've found that I use the breadcrumbs quite a lot. And when the breadcrumb link doesn't work as I expect it to, I find myself wasting time backtracking to get where I want. In this case, I'd rather see the expected behavior that I noted in my original post.

Therefore, would it be possible to keep the default breadcrumb behavior for "regular" admins, but make it customizable for superusers?

@isellsoap
Copy link
Collaborator

@lanceosborne Since this is not a bug and your last comment sounds more like a feature request, could you open one over at https://github.com/processwire/processwire-requests and close this issue? Thanks!

@processwired
Copy link

Sorry for rising this thread from the dead, but what's the solution to the originally posted issue now: "The breadcrumb link for the parent page of the current page does not link to the proper page."?

I wonder why this issue has been closed. I'm getting complaints from some editors, because they, for instance, do a search in the upper right search field, find a page, open it, use the breadcrumb (click on parent) - and then they get lost.

Is there a way to fix it on my own (hook into breadcrumb)?

@BitPoet
Copy link

BitPoet commented Jan 3, 2017

This snippet for site/ready.php restores the old behavior:

wire()->addHookBefore("Process::breadcrumb", null, "hookBefore_ChangeParentBreadcrumb");

function hookBefore_ChangeParentBreadcrumb(HookEvent $event) {
	if($event->object instanceof ProcessPageEdit) {
		$href = $event->arguments(0);
		if($href == "../") {
			$page = $event->object->getPage();
			$event->arguments(0, "../?open=$page->id");
		}
	}
}

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

6 participants