I have tested this item ✅ successfully on aaf4ce3

I have tested this item ✅ successfully on aaf4ce3

RTC

@pe7er look at https://github.com/pe7er/joomla-cms/pull/1it use the prepared statement for the new where clause

Wouldn't it be more obvious to filter by menu item type than component?

I have tested this item ✅ successfully on d951e96

Back to pending after recent change for prepared statements. One more tester needed.

I have tested this item ✅ successfully on d951e96

I have tested this item ✅ successfully on d951e96

Back to RTC after 2 good tests. But could you check @wilsonge 's comment "Wouldn't it be more obvious to filter by menu item type than component?"? Seems to be a valid question to me.

@pe7er Could you check George's comment above? I think it is a valid question. When fitering by the type of menu item we may achieve nearly the same result, because the item types are connected to the component, and it might also be easier.

@richard67 @wilsonge I have changed my PR:

• added a new Field type MenuItemByComponent to filter by menu item type (instead of component)

Before the change the dropdown would be:

After the change the dropdown looks like:

Note: I've added a ->where($db->quoteName('menu.client_id') . ' = 0') to the query.
Otherwise it would also show menu items from the admin menu in the dropdown:

@pe7er Should the label then still be "- Select Component -"?

@richard67 yes, I think so. Or do you have a suggestion for something better?

@pe7er Well, when you create a new menu item, it is called "Menu Item Type":

So maybe we should use the same for the new filter? "- Select Menu Item Type -" ... ok, maybe that's a bit long, but would be consistent.

That's because it contains the views for each component. If I understand correctly this pr just lists the components not all their views

I see.

@pe7er Other question. In your 2nd screenshot, banners are missing. Is that expected?

@brianteeman Yes, that's right. I wanted to filter on View and analyzed the #__menu table to see if it's easy to filter on views: it is not. The only field that has the view, has it in a larger string of text.

@richard67 good point! Yes, the filter only shows the components for which a menu item has been created. The banner component does not have a front-end view (only modules that display the banners).

In the 1st screenshot, the banners is present because I filtered on components table. Now only the components of created front-end menu items are in the drop down list.

The banner component does not have a front-end view (only modules that display the banners).

Ah, sure, I should have known that.

The banner component does not have a front-end view (only modules that display the banners).
Ah, sure, I should have known that.

I would love to have a front-end view for the banner component though. I've had use cases in the past: a page that list all the banners of sponsors. Because of the the missing banner view I've solved that with adding an empty article + loaded the banner module. :-)

I've fixed the issues in MenuItemByComponentField that @SharkyKZ mentioned.

Thank you both for your improvements @Quy + @SharkyKZ

@Quy @SharkyKZ @richard67 Do I need two new tests before getting it to RTC?

@pe7er I would say yes, because changes after the previous tests were not cosmetic only. I'll see if I can find time for testing tomorrow.

I have tested this item ✅ successfully on fb902a4

The behavior of the new filter is a bit different to other filters, but I think that's intended.

Other filters always show all possible options, regardless if there are menu items using it or not, while the new filter added with this PR only shows those components in the selection forn which there is a menu item, i.e. e.g. if you don't have any menu item for com_contact, the "Contacts" is not available in the dropdown list.

@pe7er Let me know if that's not intended, then I can change back my test result.

Other filters always show all possible options, regardless if there are menu items using it or not, while the new filter added with this PR only shows those components in the selection forn which there is a menu item, i.e. e.g. if you don't have any menu item for com_contact, the "Contacts" is not available in the dropdown list.

I hope its the intended behaviour. Look at the module filter in com_modules

I hope its the intended behaviour. Look at the module filter in com_modules

Seems so. Module positions are only shown in the dropdown if used. But languages are always shown all possible. So it seems we have that inconsistentcy everywhere, and maybe those other filters need to be fixed?

Anyway, this PR here is fine for me.

I have tested this item ? unsuccessfully on fb902a4

Almost but not quite ;)

The list of components that you can select from does not take into account the menu in use.

Example with two or more menus create a new menu item for the wrapper component in just one of the menus

Now check the component filter in that menu and you see the wrapper component - all good

Now check the component filter in the other menu. I expect not to see the wrapper component being listed but it is ;(

Thanks @brianteeman
I've added the currentMenuType context to the component filter.
Now the - Select Component - filter only shows the component views that are used in that specific menu.

I have tested this item ✅ successfully on 585c185

Thanks for rewriting the code using a prepared statement @Quy !

Could two of you please re-test this PR again? @sere-b @brianteeman @ChristineWk @richard67

I have tested this item ✅ successfully on 6e60bd9

Thanks @ChristineWk !

@pe7er Now the filter offers only those components which are actually used in a menu. But now there is something not nice: When filtering for a component and having selected a menu which contains a menu item for this component, I see a result. When then selecting another menu which doesn't contain that component, I see "No matching results", which is correct. Up to here all fine.

But I don't see that there is a filter applied, because the dropdown value for the component has been cleared, so there is no visual hint showing there is still a filter applied and that I have to use the "Clear" button.

Example with testing sample data: Select the "Fruit Shop" menu, then filter by the "Content" component, then change menu selection to "Australian Parks". One could think that the "Australian Parks" menu is empty when proceeding in this order of processing.

@pe7er Sorry, have forgotten to add the question to the end of my previous comment: Is that ok, or would it be better and possible to clear the filter automatically in such case?

Maybe it would be the best to clean all filter selections when the selected menu changes?

Thank you for your test @richard67 & for spotting that annoying bug.

Yes, I agree that filters that depend on a selected menu, should be cleaned after the selected menu changes.

Yes, I agree that filters that depend on a selected menu, should be cleaned after the selected menu changes.

@pe7er Should that be done with this PR here because this PR makes that annoying thing more visible, or do you wanna make a new separate PR for that?

@richard67 I think that I should do it in this PR for the "component view" filter.

Did you notice this annoying behavior after switching selected menu with other filters as well?

Did you notice this annoying behavior after switching selected menu with other filters as well?

No, but that's just because the others don't depend on the selected menu, and so if a filter option is selected and then you change the menu seletion, the selection in the filter is not removed.

In case of the new filter added by this PR, you lose the selection when changing to a menu where the selected value is not available.

If we now clear the filter after a change of the menu selection - which from an UX point makes sense because the menu selection is at the very top, superior to all others, then we can in future PRs pimp the other filters, too, so they only show relevant values for the selected menu (or all) like the new filter does.

IMHO we should only clear the filters that are dependent on the menu.

From experience I know that I sometimes use some filters (eg trashed items), then decide to look in other menus. Having to re-select those filters again would be annoying. So I would add the clean filter option only for filters that are dependent on the selected menu.

So I would add the clean filter option only for filters that are dependent on the selected menu.

Sure. Is there any other beside the new filter? I don't remember right now. If we later see that other filters which currently are not dependent on the selected menu should be changed to that, we can do the clear filter for those then in the future PR which changes these filters.

I've tried some things to reset the filter when the menu changes, but so far unsuccessfully...

In administrator/components/com_menus/src/Model/ItemsModel.php the method
protected function populateState($ordering = 'a.lft', $direction = 'asc')
I've changed line 166 to

// If menutype changed reset pagination and reset componentName filter
if ($menuType != $currentMenuType)
{
	$app->input->set('limitstart', 0);
	$this->setState('filter.componentName', null);
}
else
{
	$componentName = $this->getUserStateFromRequest($this->context . '.filter.componentName', 'componentName');
	$this->setState('filter.componentName', $componentName);
}

but that does not work. It keeps receiving the form input with the previous componentName filter.

Can someone please give me a hand with this?

I have tested this item ✅ successfully on 6e60bd9

I've tried some things to reset the filter when the menu changes, but so far unsuccessfully...

In administrator/components/com_menus/src/Model/ItemsModel.php the method
protected function populateState($ordering = 'a.lft', $direction = 'asc')
I've changed line 166 to

// If menutype changed reset pagination and reset componentName filter
if ($menuType != $currentMenuType)
{
	$app->input->set('limitstart', 0);
	$this->setState('filter.componentName', null);
}
else
{
	$componentName = $this->getUserStateFromRequest($this->context . '.filter.componentName', 'componentName');
	$this->setState('filter.componentName', $componentName);
}

but that does not work. It keeps receiving the form input with the previous componentName filter.

Can someone please give me a hand with this?

@SharkyKZ Could you have a look on that? Do you have an idea? Thanks in avdance.

I am moving this to 4.1, we are in beta and this is a new feature. Thank you all who have been working on this and bringing it to the state it is now. But if we want to get 4.0 out of the door we need to make a cut.

No prob, I understand. Thanks @rdeutz !

@PhilETaylor Thanks for your improvements!

Hi @pe7er, would you like to make this PR ready for 4.2? Then we can finally merge this nice feature :)

I have tested this item ✅ successfully on 00b60d9

Tested in 4.2-dev - also checked only components present in the selected menu are listed in the select drop-down.

I have tested this item ✅ successfully on 00b60d9

It worked as described.

@pe7er for fix the update requested pe7er#2

@pe7er for fix the update requested pe7er#2

I think it will not need new human tests when these changes based on Quy‘s review suggestions will be implemented.

this pr is in it's second edition is still pending from ie 2020 2 years ago
can we speed up the process a little bit ? merge this as is for now ?

It needs to be rebased to 4.2-dev, and it needs to apply Quy‘s review suggestions. If @pe7er is ok with it I can do both.

Ideally it also needs to be applied to admin menu items

@richard67 Thanks!
I updated my PR with the 4.1-dev branch, but I do not know how to rebased to 4.2-dev.
If you would take care of the rebase to 4.2-dev, please do.
Thanks!

It's really easy to rebase

I've just tried to rebase, but that was not good, it has shown too many changes, so I have reverted that. I think we have to wait with that until the next upmerge from 4.10-dev to 4.2-dev has taken place.

@brianteeman Thanks for showing me how to rebase.
@richard67 Thanks!
If there are any issues with my PR during the "upmerge" then I can redo my PR as a new PR to the 4.2-dev branch.

@brianteeman @richard67 Thanks! If there are any issues with my PR during the "upmerge" then I can redo my PR as a new PR to the 4.2-dev branch.

@pe7er I don't think there will be issues. The problem right now is just that the 4.1-dev branch and also your PR are in advance to the 4.2-dev branch, so the rebase here currently shows all these other changes, too. We just have to wait for the next regular update of the 4.2-dev branch to the 4.1-dev branch (that's the "Upmerge"), and after that the rebase will show the right result.

Back to pending

I've added back the previous test results in the issue tracker since the change which has invalidated it was just a backslash for the global namespace and short array syntax and after that just a clean branch update and finally a clean rebase.

I'll do a quick test soon and then will set RTC if ok.

I have tested this item ✅ successfully on 11bca7e

RTC

Thank you all for testing, comments and improvements!

@pe7er Thank you for this addition but I see some unwanted behavior when testing.

Removed extensions

When I first opened the filters I see this:

Notice the empty dropdown. This is what I see opening the dropdown:

The reason this happens is because I have a menu entry for an extension that has been removed after I created the menu item. You get an empty entry because your query links to the extensions table that no longer has the name in it. Perhaps we can have a fallback to the option value in the menu link?

Missing translation

In the above screenshot you see HikaShop, however when I filter on Articles the entry HikaShop becomes COM_HIKASHOP.

Can you have a look at these 2 issues please? Other than that this looks good. Thank you.

The solution for @roland-d 's issue with a menu item for an uninstalled extension can be solved by changing the JOIN from an outer (left) JOIN to an INNER JOIN here: https://github.com/joomla/joomla-cms/pull/28832/files#diff-ddcad41745a23f26d77ffc97f8ac6382c68172412523042358d8c4a5aa93b3c2R52

@richard67 In that case you get rid of the empty entry but you cannot filter the menu items then on that particular extension.

@richard67 In that case you get rid of the empty entry but you cannot filter the menu items then on that particular extension.

@roland-d Then add a ->where($db->quoteName('extensions.extension_id') . ' IS NOT NULL') to the query. Does that help?

I've added @richard67 's database improvement to fix the issue with uninstalled components reported by @roland-d
Thank you both!

Before applying the PR, the Filter Options has no - Select Component - option, allowing filtration of menu items on component. After applying the PR and following @pe7er's instructions, I was able to filter on various component types, including com_contact, showing (just) the Single Contact menu item.

After uninstalling a component with an existing menu item, there were no empty entries in the - Select Component - select box. However, the uninstalled component did not show up either, although the menu item still exists. So it appears @roland-d's latest remark has not been addressed (properly) yet.

This pull requests has been automatically converted to the PSR-12 coding standard.

I have tested this item ✅ successfully on 40e3f99

Everything looks good, even after extension uninstall

I would like to add that when a component is uninstalled, the menu items created with that component become 'orphan' and are no longer 'filtrable'. I would assume this as 'normal' behavior.

@pjdevries would you mind recording your test in the issue tracker? Thank you.

This PR requires updated documentation for the online help such as the one found at https://help.joomla.org/proxy?keyref=Help42:Menus:_Items

Thank you!

I have tested this item ✅ successfully on 40e3f99

RTC

Great PR, thank you Peter @pe7er for this feature and for all to comment and help make this a good addition. Peter, do you need help with documentation?

@jwaisner @obuisard Could at least one of you redo your test because there have been some changes? Just to see if it still works, so we can keep RTC. Thanks in advance.

I have tested this item ✅ successfully on 40e3f99

2nd testing successful

Hello Peter @pe7er, do you need help with documentation? Would love to sere this PR merged soon.

Documentation added ✔️ (https://docs.joomla.org/Help4.x:Menus:_Items)

Thank you Peter @pe7er for this PR!