includeNotInMenu for breadcrumbs

[seirerman]

We used includeNotInMenu=1 to include pages with nav_hide=1 in our old HMENU breadcrumb menus:

10 = HMENU
10 {
    special = rootline
    includeNotInMenu = 1
    ...

This doesn't seem to be possible with the menus extension (which otherwise works great, btw!). Or am I just missing something?

page.10.dataProcessing.110 = B13\Menus\DataProcessing\BreadcrumbsMenu
page.10.dataProcessing.110.as = breadcrumbNavigation
page.10.dataProcessing.110.includeNotInMenu = 1

[siwa-pparzer]

would also be nice for LanguageMenu currently, you can not switch the language, if the page is hidden in the menu

[bmack]

I think LanguageMenu has some more "problems" in general, such as handling non-translated pages properly. In any case, it would be beneficial to sort out doktypes and hide-in-menu properly with good test coverage.

[sbuerk]

After Benny suggessted us (as an option) to have a look to this extension for a project, we had some performance issues, we backported this extension to v8 for that project (and v7 for a handfull others). And we added the excludePages to the breadcrumb menu in our backport. Anyway this was added here with feature #20 / pr #52, because our goal was to get rid of our own "fork", even more because the project upgraded to v9.

Anyway, evaluationg for v9 the orignal extensions broke for us a lot of menus in that project, because of the globally introduced excluding of nav_hide = 1 pages in all menus (which was not the case when we backported it) through https://github.com/b13/menus/commit/a746bf269e41719f024a475d81a4617b1b81969b.

In our case we need, that nav_hide = 1 is not excluded by following menus:

• treeMenu
• listMenu
• breadcrumbMenu

We have menus with and withoud hiding these pages, for that pages we needed it we added a second DataProcessor as follow up which cleans nav_hide pages from that menus afterwards.

There was an issue #38 Bug: LanguageMenu excludes "nav_hide" pages, where the opener states that language menu should NOT hide nav_hide = 1 pages. But reading the answers, I thinkt that went wrong. It was mentioned that it was solved by the commit where ALL menus now hide nav_hide = 1 pages.

I can understand, that there are use cases where hiding these pages is usefull. But befor that commit, it was possible to hide them "afterwards", because the where "there". And display them if needed.

So, a746bf269e41719f024a475d81a4617b1b81969b was a full "breaking change" for us, with no way to adjust it so we can render our usecases ( without forking / using patched local copy of the extension ).

Personally I would prefer an option "respectNavHide" for all menu types, but having the option named that way, a default of "true" would be needed if not provided / set as option - which does not make any sense, so I would go for the mentioned "includeNotInMenu" option name stated in this issue for it - for all menu types.

Also @bmack is right, language menu has eventually some more problems. After taking some time I would say that regarding test coveraged there is a lot missing, not only for the language menu.

For example, listMenu has absolutly no functional tests for the processor, it is completly missing. Also I would not say that having tests with a page structure with nav_hide = 1 pages, if all menu types are hiding them should be included, but was missing also. I'm not sure, but should there not be pages for all menus with multi language tests ?

But anyway, because this would not be a simple task to get all that fixed in one step, I would leave some of these things for feature work. @bmack I would love to have a talk with you (or the person who is the maintainer of this extension ) to discuss some of these extended/further tasks, maybe came up with parts / steps - and I would take the time to help with them.

So for now, as a first round I prepared a pull request with some work regarding this "issue", but more for all menu types, not only for breadcrumbs.

I have included also a lot of further testcases in this PR, not only introducing the "includeNotInMenu" option. For that I have extended the tests for the menus which was there, and adding nav_hide = 1 pages to the generel pages fixture for functional tests. And also added tests for not not tested listMenu also.

What is still not included, are real tests with multilanguage pages fixture structure for all menu types.

But for now at the next step we would need this part with this option first to use the raw extension again in our biggest project.

On our plan is, to use this extension in another customer installation (v9 with +10 languages, with connected and free mode languages ). So I would go for the language things than for that, and help improve the extension through that.

Regarding the listMenu there is another point which could be discussed: for list menu you select specific pages, which means that the pages are selected. So it is arguable, that the listMenu should not ignore nav_hide pages, if they are specified ? Making includeNotInMenu absolete for that. But for now I have added the includeNotInMenu option for it.