Skip to content

[5.2] Remove Toolbar::getInstance() from views#43512

Merged
laoneo merged 7 commits intojoomla:5.2-devfrom
Hackwar:5.2-toolbar
Aug 14, 2024
Merged

[5.2] Remove Toolbar::getInstance() from views#43512
laoneo merged 7 commits intojoomla:5.2-devfrom
Hackwar:5.2-toolbar

Conversation

@Hackwar
Copy link
Member

@Hackwar Hackwar commented May 22, 2024

Summary of Changes

Toolbar::getInstance() has been deprecated for quite some time now, but the alternative wasn't really clear yet. This PR replaces all occurences with $this->getDocument()->getToolbar() instead. This leaves the Toolbar::getInstance() in the ToolbarHelper class.

Testing Instructions

Codereview

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@HLeithner
Copy link
Member

What the issue @brianteeman ?

@brianteeman
Copy link
Contributor

What the issue @brianteeman ?

  1. obvious errors due to blindly search/replace (picked up by drone)
  2. stating that no documentation is required.

@HLeithner
Copy link
Member

I checked all files but can't find a search replace issue. if you mean the conversation from Toolbar::getInstance('toolbar'); to $this->getDocument()->getToolbar(); without the parameter toolbar this is ok because the default toolbar is toolbar.

on the side I checked some file for missing removal of the use statement for the toolbar which can be removed if not used, as it has been done for some view.

@brianteeman
Copy link
Contributor

Sorry no idea as its been 10 weeks but based on my comment it must have been something shown up in drone so maybe retrigger drone and see

@pe7er
Copy link
Contributor

pe7er commented Jul 16, 2024

@Hackwar Could you please remove the unused use Joomla\CMS\Toolbar\Toolbar; statements?

@pe7er pe7er self-assigned this Jul 16, 2024
@Hackwar
Copy link
Member Author

Hackwar commented Jul 16, 2024

Done

@Quy
Copy link
Contributor

Quy commented Aug 10, 2024

Done

I flagged some. There are more to remove.

@Hackwar
Copy link
Member Author

Hackwar commented Aug 10, 2024

Done

I flagged some. There are more to remove.

No, I thoroughly went through them all at that time and all other instances are still in use.

@laoneo
Copy link
Member

laoneo commented Aug 14, 2024

@Quy the code style checker should also throw an error when there are unused imports. So I guess we are fine here, or?

@Quy
Copy link
Contributor

Quy commented Aug 14, 2024

@laoneo Yes. Please discard my comments.

@laoneo laoneo merged commit 5015961 into joomla:5.2-dev Aug 14, 2024
@laoneo
Copy link
Member

laoneo commented Aug 14, 2024

Thanks!

@laoneo laoneo added this to the Joomla! 5.2.0 milestone Aug 14, 2024
@Hackwar Hackwar deleted the 5.2-toolbar branch June 10, 2025 17:40
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.

8 participants