Skip to content

Conversation

@wilsonge
Copy link
Contributor

@wilsonge wilsonge commented Jan 1, 2023

Summary of Changes

Migrate the majority of core components towards the new toolbar API (notably I'm skipping Joomla Update as part of this PR and the use of ToolbarHelper::modal in com_templates views which probably need to be moved to the popupButton - but that's a bigger piece of work for another PR)

Testing Instructions

Check all the toolbar icons work the same before and after the patch is applied across all backend components

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

@brianteeman
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on b4bab7d

changes the button id :(


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

@wilsonge
Copy link
Contributor Author

wilsonge commented Jan 1, 2023

https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/Toolbar/ToolbarButton.php#L221

IDs look fairly fixed. I can rebase this to 5.0 I guess - Depends how much we think IDs are a major b/c break in the backend. It's only the delete button that has the changed ID right?

@wilsonge wilsonge changed the title Migrate com_cache to the new toolbar api Migrate most components to the new toolbar api Jan 3, 2023
@wilsonge wilsonge changed the title Migrate most components to the new toolbar api [4.3] Migrate most components to the new toolbar api Jan 3, 2023
@carlitorweb
Copy link
Member

@wilsonge what you mean with cache toolbar in the test instructions? Is just test the toolbar of each component right? Sorry for ask, just testing this PR, and for avoid do it wrong.

@wilsonge
Copy link
Contributor Author

wilsonge commented Jan 5, 2023

Yes. Sorry originally the PR was just over com_cache but then I worked on some more components so need to update the instructions

@wilsonge
Copy link
Contributor Author

wilsonge commented Jan 5, 2023

Final batch of components migrated. I suggest we test functionality here and I'll leave it to @joomla/cms-maintainers to decide if it needs to go to 4.3 or 5.0 and I'll happily rebase it to 5.0 if the IDs are deemed a concern.

@carlitorweb
Copy link
Member

carlitorweb commented Jan 7, 2023

  • com_actionlogs ✔️
  • com_admin ✔️
  • com_associations ✔️
  • com_cache ✔️
  • com_categories ✔️
  • com_checkin ✔️
  • com_config ✔️
  • com_contact ✔️
  • com_cpanel ✔️
  • com_content ✔️
  • com_contenthistory ✔️
  • com_fields ✔️
  • com_finder ✔️
  • com_installer ✔️
  • com_languages ✔️
  • com_media ✔️
  • com_menus ✔️
  • com_messages ✔️
  • com_modules ✔️
  • com_newsfeed ✔️
  • com_plugins ✔️
  • com_postinstall ✔️
  • com_privacy ✔️
  • com_redirect ✔️
  • com_sheduler ✔️
  • com_tags ✔️
  • com_templates ✔️
  • com_users ✔️
  • com_workflow ✔️

@carlitorweb
Copy link
Member

@wilsonge com_installer -> view=manage

The button Install Extension: Undefined variable $url in ..www\bugtesting\joomla-cms\layouts\joomla\toolbar\link.php on line 34

@wilsonge
Copy link
Contributor Author

Fixed!

@carlitorweb
Copy link
Member

I have tested this item ✅ successfully on 4198356


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

obuisard and others added 4 commits January 16, 2023 13:24
@Quy
Copy link
Contributor

Quy commented Jan 18, 2023

I have tested this item ✅ successfully on 661f0b1


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

1 similar comment
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 661f0b1


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

@Quy
Copy link
Contributor

Quy commented Jan 20, 2023

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 20, 2023
@obuisard obuisard added this to the Joomla! 4.3.0 milestone Jan 21, 2023
@obuisard obuisard merged commit f05a062 into joomla:4.3-dev Jan 21, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 21, 2023
@obuisard
Copy link
Contributor

Thank you George @wilsonge for this PR!

@wilsonge wilsonge deleted the cache_toolbar_api branch January 22, 2023 12:59
@Quy Quy mentioned this pull request Feb 4, 2023
2 tasks
richard67 pushed a commit that referenced this pull request Feb 4, 2023
brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Apr 3, 2023
You can't have two buttons with the same text that do different things. I am assuming that this change was made in error by @wilsonge in joomla#39537

### Joomla 4.29

### Joomla 4.3 before this PR

### Joomla 4.3 after this PR
wilsonge pushed a commit that referenced this pull request Apr 3, 2023
You can't have two buttons with the same text that do different things. I am assuming that this change was made in error by @wilsonge in #39537

### Joomla 4.29

### Joomla 4.3 before this PR

### Joomla 4.3 after this PR

Co-authored-by: Quy <[email protected]>
@heelc29 heelc29 mentioned this pull request Jul 24, 2023
2 tasks
brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Sep 15, 2023
The convention is that when it is a NEW item then its a cancel button and when editing an item its a close button.

During the updates in joomla#39537 it looks like @wilsonge missed a few changes etc
wilsonge pushed a commit that referenced this pull request Sep 15, 2023
The convention is that when it is a NEW item then its a cancel button and when editing an item its a close button.

During the updates in #39537 it looks like @wilsonge missed a few changes etc
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.

10 participants