[4.0] Hide disabled components in mod_submenu by alikon · Pull Request #32555 · joomla/joomla-cms

alikon · 2021-03-01T18:41:41Z

Pull Request for Issue #32456.

Summary of Changes

don't show items from disabled component in mod_submenu

Testing Instructions

see #32456.

Actual result BEFORE applying this Pull Request

see #32456.

Expected result AFTER applying this Pull Request

no items from disabled components are shown

alterrnative to #32539

let's see how many 👎 i got 🤣

Fedik · 2021-03-01T18:53:49Z

this actually looks much better than #32539
because ComponentHelper already have a pre-loaded list of available components.

Fedik · 2021-03-01T18:55:52Z

administrator/modules/mod_submenu/src/Menu/Menu.php


 		foreach ($children as $item)
 		{
+			if (!ComponentHelper::isEnabled($item->element))


you also need to check the $item type to be a component, because there may be a spacers or something else.
if I right remember

well, I hope 😄
need to test

I thought about if ($item->type === 'component' ...
but I do not remember how it exactly there, need to check, later

joomdonation · 2021-03-02T06:42:43Z

administrator/modules/mod_submenu/src/Menu/Menu.php


 		foreach ($children as $item)
 		{
+			if (!empty($item->element) && (!ComponentHelper::isEnabled($item->element)))


Could you change the code to

if ($item->element && !ComponentHelper::isEnabled($item->element))

It is a bit easier to read, same with the code in mod_menu https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/modules/mod_menu/src/Menu/CssMenu.php#L313

No, it is not done yet. You still have unnecessary ( and ). Please change it exactly to:

if ($item->element && !ComponentHelper::isEnabled($item->element))

it's already like that
https://github.com/joomla/joomla-cms/pull/32555/files#diff-4e78a9a79d9c5aa5ff67fbe2aaaebbee6e6ef7d90de830da7c7987d543edf919R54

@alikon You still have unnecessary bracket ( and ) in the code as I said. The code should be

if ($item->element && !ComponentHelper::isEnabled($item->element))

The current code is:

if ($item->element && (!ComponentHelper::isEnabled($item->element)))

ah my bad didn't look at bracket

Fedik · 2021-03-02T08:54:40Z

btw, we have similar in mod_menu also 😉

joomla-cms/administrator/modules/mod_menu/src/Menu/CssMenu.php

Lines 312 to 317 in fa6cc5d

    
           // Exclude item if is not enabled 
        
           if ($item->element && !ComponentHelper::isEnabled($item->element)) 
        
           { 
        
           	$parent->removeChild($item); 
        
           	continue; 
        
           }

so all should be good

richard67 · 2021-03-02T12:15:25Z

@alikon I was so free to change the title for you so we later have a better commit message when merging the PR.

particthistle · 2021-03-02T12:23:08Z

I have tested this item 🔴 unsuccessfully on abd61d4

Before applying patch, "Manage" list on System Dashboard had 9 items:
Extensions, Languages, Content Languages, Language Overrides, Content Security Policy, Plugins, Redirects, Site Modules, Administrator Modules

After applying patch, without turning off any components, some items disappeared:
Languages, Content Languages, Language Overrides

It seems these three Language related items are not following the same configuration as the other items in the list.

As Language component is a protected one, it can't be disabled, but I'm not sure why it's not otherwise appearing.

Aside from the Language items, the PR works as expected when I turned off either CSP or Redirects as two items available to disable in that dashboard module.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32555.}

joomdonation · 2021-03-02T12:46:08Z

@particthistle I think the error here is not related to this PR. I don't understand how menu presets work, but someone familiar with that concept should check https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_menus/presets/system.xml#L127-L149 to see why element="com_language", not element="com_languages" (I think it is typos). One that issue is fixed, the error you reported here won't happen.

richard67 · 2021-03-02T17:34:29Z

@particthistle I think the error here is not related to this PR. I don't understand how menu presets work, but someone familiar with that concept should check https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_menus/presets/system.xml#L127-L149 to see why element="com_language", not element="com_languages" (I think it is typos). One that issue is fixed, the error you reported here won't happen.

@joomdonation @particthistle Who wants to make a PR?

not com_language

alikon · 2021-03-02T19:09:13Z

better to do it in this pr imho
please test again

particthistle · 2021-03-03T04:16:29Z

I have tested this item ✅ successfully on 9511451

All good this time around.

Glad it was something easy for you to resolve @alikon as I wouldn't have known where to start to fix that up.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32555.}

toivo · 2021-03-03T14:38:35Z

I have tested this item ✅ successfully on 9511451

Tested successfully in Beta8-dev of 3 March in Wampserver 3.2.4 using PHP 8.0.2.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32555.}

richard67 · 2021-03-03T14:40:03Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32555.}

wilsonge · 2021-03-05T12:38:26Z

LGTM. Nice work everyone!

less code

b131a45

joomla-cms-bot added the PR-4.0-dev label Mar 1, 2021

alikon mentioned this pull request Mar 1, 2021

fix(mod_submenu): hide disabled components in mod_submenu #32539

Closed

Fedik reviewed Mar 1, 2021

View reviewed changes

better ?

6c2c13a

joomdonation reviewed Mar 2, 2021

View reviewed changes

easy read

fa6cc5d

unnecessary bracket

abd61d4

richard67 changed the title ~~less code~~ [4.0] Hide disabled components in mod_submenu Mar 2, 2021

com_languages

9511451

not com_language

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 3, 2021

wilsonge merged commit 5e57dae into joomla:4.0-dev Mar 5, 2021

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 5, 2021

wilsonge added this to the Joomla 4.0 milestone Mar 5, 2021

alikon deleted the patch-81 branch March 5, 2021 14:48

dgrammatiko pushed a commit to dgrammatiko/joomla-cms that referenced this pull request Mar 17, 2021

[4.0] Hide disabled components in mod_submenu (joomla#32555)

b2a7b31

Uh oh!

Conversation

alikon commented Mar 1, 2021

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

alterrnative to #32539

Uh oh!

Fedik commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fedik Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fedik commented Mar 2, 2021

Uh oh!

richard67 commented Mar 2, 2021

Uh oh!

particthistle commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joomdonation commented Mar 2, 2021

Uh oh!

richard67 commented Mar 2, 2021

Uh oh!

alikon commented Mar 2, 2021

Uh oh!

particthistle commented Mar 3, 2021

Uh oh!

toivo commented Mar 3, 2021

Uh oh!

richard67 commented Mar 3, 2021

Uh oh!

wilsonge commented Mar 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Fedik commented Mar 1, 2021 •

edited

Loading

Fedik Mar 1, 2021 •

edited

Loading

particthistle commented Mar 2, 2021 •

edited

Loading