Skip to content

[4.0] Remove deprecated code in Joomla\CMS\Menu\MenuItem#26537

Merged
wilsonge merged 33 commits intojoomla:4.0-devfrom
SharkyKZ:j4/deprecated/MenuItem
Nov 30, 2019
Merged

[4.0] Remove deprecated code in Joomla\CMS\Menu\MenuItem#26537
wilsonge merged 33 commits intojoomla:4.0-devfrom
SharkyKZ:j4/deprecated/MenuItem

Conversation

@SharkyKZ
Copy link
Contributor

@SharkyKZ SharkyKZ commented Oct 10, 2019

Pull Request for Issue # .

Summary of Changes

Removes deprecated code from Joomla\CMS\Menu\MenuItem.

Testing Instructions

Install sample data or create some menu items.
Navigate frontend and backend.
Inspect menu items.

Expected result

Works like before.

Documentation Changes Required

Some methods removed.
Added Joomla\CMS\Menu\AdministratorMenuItem class.

@amit4106udale
Copy link

I have tested this item 🔴 unsuccessfully on 9137ad0

Error: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params: Cannot access protected property Joomla\CMS\Menu\MenuI


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26537.

@SharkyKZ
Copy link
Contributor Author

On which page are you getting this?

@amit4106udale
Copy link

On which page are you getting this?

I was testing this on cloudaccess. And applied the patch. Once applied site became black.

@SharkyKZ
Copy link
Contributor Author

Do you have any 3rd party extensions installed? Maybe using Cloudbase template?

@amit4106udale
Copy link

Do you have any 3rd party extensions installed? Maybe using Cloudbase template?

No pure joomla 4 nightly build

@SharkyKZ
Copy link
Contributor Author

Should work now but don't test yet. Performance could be bad at the moment.

@SharkyKZ
Copy link
Contributor Author

OK, this can be tested now.

@Quy
Copy link
Contributor

Quy commented Nov 11, 2019

Go to Components Dashboard.

Notice: Undefined property: Joomla\CMS\Menu\MenuItem::$permission in \administrator\modules\mod_submenu\Menu\Menu.php on line 56

Notice: Undefined property: Joomla\CMS\Menu\MenuItem::$icon in \administrator\modules\mod_submenu\tmpl\default.php on line 22

@SharkyKZ
Copy link
Contributor Author

Though maybe there's a better way to do this. browserNav is a numeric value from database. In some places in backend it's overwritten with the string used in markup. target is only used in backend and it also holds same string. Do we need both of these properties? Maybe remove target and use only browserNav?

In frontend it's much simpler. Only browserNav is used and it does not get overwritten.

@isidrobaq
Copy link

I have tested this item ✅ successfully on 0460dd6


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26537.

1 similar comment
@cheiff
Copy link
Contributor

cheiff commented Nov 15, 2019

I have tested this item ✅ successfully on 0460dd6


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26537.

@alikon
Copy link
Contributor

alikon commented Nov 15, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26537.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 15, 2019
@wilsonge
Copy link
Contributor

Though maybe there's a better way to do this. browserNav is a numeric value from database. In some places in backend it's overwritten with the string used in markup. target is only used in backend and it also holds same string. Do we need both of these properties? Maybe remove target and use only browserNav?

If that works - and they are exactly the same thing (struggling to tell from a quick code review - I'd probably go down that route. Maybe revert the rename here https://github.com/joomla/joomla-cms/pull/26537/files#diff-1b5557519e9638a5bde91cf3432456dfR403 and then make that change in a separate PR?

@SharkyKZ
Copy link
Contributor Author

I think it's best to subclass MenuItem for administrator items and add missing properties there.

@wilsonge wilsonge merged commit e1dc420 into joomla:4.0-dev Nov 30, 2019
@wilsonge
Copy link
Contributor

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 30, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Nov 30, 2019
@xillibit
Copy link
Contributor

xillibit commented Dec 2, 2019

I have this kind of call in my component :
$params = Factory::getApplication()->getMenu()->getActive()->params; with this change it display : Cannot access protected property Joomla\CMS\Menu\MenuItem::$params

Can-i call instead $params = Factory::getApplication()->getMenu()->getActive()->getParams(); ? getParams() isn't deprecated anymore ?

@SharkyKZ
Copy link
Contributor Author

SharkyKZ commented Dec 2, 2019

Yes, use getParams() to access params.

@bugsmafia
Copy link

Error: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params

Are these changes due to a bug on the frontend?

@SharkyKZ
Copy link
Contributor Author

@bugsmafia Are you using any 3rd party extensions?

@bugsmafia
Copy link

bugsmafia commented Dec 16, 2019

@bugsmafia Are you using any 3rd party extensions?

no, clean joomla 4 dev.
Files were updated (developer build of December 16) (with full replacement) after 1 month of development (build of November 7, 2019).

@SharkyKZ
Copy link
Contributor Author

On which pages does this error occur?

@bugsmafia
Copy link

On which pages does this error occur?
Any articles, including the default Featured Articles page
(whether a problem occurs with other components is unknown)

@bugsmafia
Copy link

/index.php?option=com_content&view=article&id=2&Itemid=103
Build on November 7th. on the links from the menu to any article gave an error 404.
after updating to the build on December 16th.
"Error: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params"

@SharkyKZ
Copy link
Contributor Author

I can't replicate this issue on current nightly (https://developer.joomla.org/nightly-builds.html) and on my git clone.

If you upload a zip of your installation (only files, no database), I could take a look.

@bugsmafia
Copy link

bugsmafia commented Dec 20, 2019

I can't replicate this issue on current nightly (https://developer.joomla.org/nightly-builds.html) and on my git clone.

If you upload a zip of your installation (only files, no database), I could take a look.

write me bugsmafia@gmai.com or Telegram @bugsm

yep, update only file (full pack),
update structure DB does not fix the error

@bugsmafia
Copy link

I Found the source of the problem in my template.
$pageclass = $menu->params->get('pageclass_sfx');
or
$pageclass = $menu->getParams()->get('pageclass_sfx');

@wilsonge
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.