[#31211] - [4.0] Deprecate unused $iconOver
- Fixed in Code Base
- 14 Nov 2020
- Medium
- Build: 4.0-dev
- # 31211
- Diff
- N6REJ:iconover-4
User tests: Successful: Unsuccessful:
Pull Request for Issue #30211
Summary of Changes
Removes unused $iconOver param.
In 1.5 days this was used as an override ( not hover as stated now ) but has never been implemented in 4.+ so might as well just remove it. If its determined later that its useful we can always create it because as it is now it does NOTHING.
Testing Instructions
notice that toolbar icons display.
apply pr.
make sure toolbar icons still display.
hover over icons notice the icons remain.
Actual result BEFORE applying this Pull Request
nothing noted
Expected result AFTER applying this Pull Request
no visual changes.
Documentation Changes Required
Deprecate $iconOver in 5.x
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
@SharkyKZ well, the thing is its not even used!!!
joomla-cms/libraries/src/Toolbar/ToolbarHelper.php
Lines 102 to 111 in 425de68
if you add $iconOver to the appendButton function it becomes a text value in place of the normal text.. i.e. everything is out of sync.
in 3.x it an override NOT hover ( which makes sense since it would be damn hard to create a hover I think )
if you look @ the function its is brought in in the function variables but then does nothing.
I'm open as to what to do because clearly I'd have TONS of icon,icon to change to icon, if I'm to do it 100% correct.
As it stands its totally broken.
in 1.5x you'll see its used as an override
#30211 (comment)
In 3.x you'll see its being used as ??
THAT would be extremely easy to implement though I would still have tons of icon,icon to clean up.
| Labels |
Added:
?
|
||
| Category | Libraries | ⇒ | Administration com_associations com_cache com_categories com_checkin com_contact com_finder com_installer com_joomlaupdate com_languages com_menus com_messages com_newsfeeds com_templates com_users Libraries |
People might call for JToolbarHelper::custom('something', 'something', '', 'alt-text');. Removing it now will break b/c.
To make it deprecate is a good point. You can safely remove it in the next J version.
This should be closed #31211 (comment) and a new pr opened for the deprecation
This should be closed #31211 (comment) and a new pr opened for the deprecation
Thats what this is!
| Title |
|
||||||
It is now you changed the title :)
| Labels |
Added:
Conflicting Files
|
||
| Labels |
Removed:
Conflicting Files
|
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31211.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31211.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31211.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31211.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-11-14 13:25:44 |
| Closed_By | ⇒ | richard67 |
Thanks!
This can't be removed without rewriting all uses of the method (B/C break) or adding B/C layer based on argument count.