Breadcrumb not showing categories on course page

I wouldn't use the word "stupid", but I really need my breadcrumb to work, and my technician cannot find the problem, I hope somebody can help us here... or at the Canadian Moodlemoot next week. Have a nice day!

I think you will find this is an unintended consequence of something Vadim, and is not intentional.

But I am interested in any developer comment. Please ignore the slightly emotive and negative tone of the issue description.

-Derek

This is the NEW 2.4 breadcrumbs

The courses link in the breadcrumbs was always not clickable (this is a feature, not a bug) but tere were links to categories visible. These are the links that have gone.

This patch partially resolves problem, but it remove current course item in navigation

Anne-Marie, i've attached simply patch - it partly resolves problem.
Currently breadcrumb way is taken from navigation block. So, if you see course under short "current course" path in navigation block, breadcrumb will show short way. Disabling current_course item leads to long path in navigation and in breadcrumb, like in 2.3. But long way in navigation is not very good too.
Ability to have different ways in navigation block and in breadcrumb needs deep changes in navigation core. Also, such changes may contradict with moodle navigation philosophy - different ways may delude users.

Bump. Has anything happened with this? Maybe a duplicate tracker item?

-Derek

Updated title and description to make its tone more neutral. Pull details added - it's a pretty quick attempt at this, so the peer reviewer will hopefully have a better understanding of the navigation system than I do and be able to confirm whether it's a sensible approach. My commit message is also a one-liner; I think it's reasonably self-explanatory, and I'm already late starting my weekend, so I don't have time to write anything longer

I also removed the part of the description about the "current course" node - it's a separate issue, and should be submitted as a new ticket (if there isn't already one for that issue).

We used to have a discussion about breadcrumbs in usability forum.
https://tracker.moodle.org/browse/MDL-34487
Perhaps it's time to provide more options to configure breadcrumbs.

Re-requesting peer review with Sam as reviewer - I couldn't find a way to set the reviewer while it was awaiting peer review.

Hi Paul,

I've just being looking at your changes now.
In general the code is good thanks. I've produced a couple of branches based upon your code just making a few tweaks.
Would you mind having a look at them and seeing what you think:

• https://github.com/samhemelryk/moodle/compare/master...wip-MDL-37762-m25
• https://github.com/samhemelryk/moodle/compare/MOODLE_24_STABLE...wip-MDL-37762-m24

Really the only noteable change is that I've added a condition before we add categories to ensure that $CFG->navshowcategories is enabled.

If you're happy with the tweaking I've done I think this can be pushed up for integration.

Many thanks
Sam

Hi Sam,
That looks good (and makes sense) to me. Hopefully it'll help address some of the requests for more flexibility - though tying it to the existing $CFG->navshowcategories option might not please everyone (I'm not really sure why anyone would want categories in the breadcrumbs but not the navigation itself - and I don't think it's worth adding yet more settings for such minute details; if anyone's really that bothered by it, it'll be easy enough for them to make a very minor change to this new code to do what they want).

Please go ahead and push it up for integration (I don't have permission to do so).

-Paul

Thanks Paul, this is up for integration now.

Thanks Sam and Paul,

I've integrated this now for master and 24. I had a quick look to see if a unit test could be added for it - but lib/tests/navigationlib_test.php is suffering from multiple test classes in a single file issue.

If you think a unit test could be added without an unreasonable amount of effort please add an issue Sam.

Reported by Dan,

Clicking on a category in the navbar (master only) gives warning:

Please use URL /course/index.php?categoryid=XXX instead of /course/category.php?id=XXX

(will add patch)

I've put a fix here:
git pull git://github.com/danpoltawski/moodle.git MDL-37762-category-fixup

Pulled that in - thanks Dan.

Please also note that all course categories are already loaded in moodle_page for the current course, you do not need to load them again using coursecat, just loop through

$this->page->categories

Also this line
https://github.com/samhemelryk/moodle/compare/master...wip-MDL-37762-m25#L0R3067

$categories[] = $this->page->navigation->get('courses'); 

must check if the node 'courses' is present, because it can be disabled in settings.

I'm failing this due to the problem caused described in MDL-39127

Thanks to Marina, it seems this:

$this->page->navigation->get('courses');

Is returning false.

MDL-39127

When logged in as student:

Fatal error: Class 'coursecat' not found in /home/marina/repositories/int_master/integration/lib/navigationlib.php on line 3066 
Call Stack: 0.0005 795784 
1. {main}() /home/marina/repositories/int_master/integration/course/view.php:0 0.4339 49922608 
2. core_renderer->header() /home/marina/repositories/int_master/integration/course/view.php:240 0.4462 51102176 
3. core_renderer->render_page_layout() /home/marina/repositories/int_master/integration/lib/outputrenderers.php:771 0.4465 51220496 
4. include('/home/marina/repositories/int_master/integration/theme/base/layout/general.php') /home/marina/repositories/int_master/integration/lib/outputrenderers.php:841 0.6811 72531648 
5. core_renderer->navbar() /home/marina/repositories/int_master/integration/theme/base/layout/general.php:77 0.6811 72531832 
6. navbar->get_items() /home/marina/repositories/int_master/integration/lib/outputrenderers.php:2638 0.6813 72532424 
7. navbar->get_course_categories() /home/marina/repositories/int_master/integration/lib/navigationlib.php:3027

well, except for all problems listed above.
The comment to $CFG->navshowcategories says:
"Show course categories in the navigation bar and navigation blocks. This does not occur with courses the user is currently enrolled in, they will still be listed under mycourses without categories."

so at least string must have been changed.

But at the same time, navbar should be the same as navigation block. It looks awkward when in navigation block course in under 'Current course' and navbar is completely different.

And also, as Martin said 7 years ago in the comment to MDL-2347 - what is convenient for admins is not convenient for students. Maybe capabilities is the way to go here.

It really needs some more thinking.

> what is convenient for admins is not convenient for students
But one thing has changed from 1.4 - now it's more difficult to determine, who is admin and who is teacher and who is student.
We've got teachers that have access to more then 100 courses, they preffer (like admins) full path in breadcrumbs.
I think some way to determine who is who, is to check mycourses count - if the're less than 20 (or other configurable value), we show short path, otherwise we show fullpath. This logic seems very pretty for me also on frontpage - if less then 20 - show mycourses list, if more - show courses three. With ability to code frontpage plugin (MDL-37731) the behavior will be more customizable.
Maybee we should make ability to make breadcrumb and navigation tree plugins? Such plugin can contain some code, that looks at breadcrumb or navtree, and expand or remove some items depending on custom logic. It can also determine, how to trim long names and so on

Alrighty, MDL-39127 has a patch up now to clean things up.
It uses $this->page->categories now instead (yay for that being there).
Shame the coursecat class isn't usable be surely we can work on that in the future.

In regards to the discussion about the navbar not matching the navigation, that is an interesting one.
When I first saw this issue I had the same reaction that Marina had.
The navigation was designed to illustrate the structure of a Moodle site. In planning that we decided that the navbar should only ever show the path highlighted in the navigation. If something was missing from the navbar, it would be missing from the navigation and we'd need to either fix the structure of the navigation or make an exception when displaying the page.
Back to this issue however I decided that this change was a worthwhile change. Not necessarily the "most accurate" of changes, but a good change necessitated because of the damned navigation settings.
If I were able to re-do the navigation I would have generated the navigation consistently without any of the navigation settings that we have now.
Then within the navigation block I'd introduce some means to fully control how things are displayed, allowing admins to configure things to their likings.
The navbar could become a block allowing control of that as well.
However as we have these navigation settings and I don't have time to rebuild things I think this is the next best approach.
Certainly as well as this issue several people have mentioned to me they wanted this change made.

Many thanks and sorry letting those issues slip in.
Sam

Created a follow-up issue MDL-39142 to consider more settings in hacking navbar

Passing, as tested in MDL-39127.

Blooming Marvelous! It's time for a knees up - your changes are upstream!

Thanks for making Moodle better!

Toodle pip