User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Icon array is titled "image". This changes it to icon to be more consistent with its function.
inspect 4.0-dev dashboard. Verify quickicons are showing.
apply pr.
verify icons still display properly.
b/c as it may require third party devs to change the $displayData['image'] to $displayData['icon']
Status | New | ⇒ | Pending |
Category | ⇒ | Modules Administration Layout |
I have tested this item
@brianteeman i'll check on that after the doctors in about 12hrs. Sorry about that.
The name is inherited from J3. 3rd party developers can add fotos or vectors to their quickicons.
@brianteeman @chmst I see what you guys mean.
$ret = [
'link' => 'index.php?option=com_akeeba&view=Backup&autostart=1&returnurl=' . base64_encode($url) . '&profileid=' . $profileId . "&$token=1",
'image' => 'akeeba-black',
'text' => Text::_('PLG_QUICKICON_AKEEBABACKUP_OK'),
'id' => 'plg_quickicon_akeebabackup',
'group' => 'MOD_QUICKICON_MAINTENANCE',
];
if (version_compare(JVERSION, '3.0', 'lt'))
{
$ret['image'] = $url . '/../media/com_akeeba/icons/akeeba-48.png';
}
HOWEVER,
in the core quickicon plugins we have both icon & image and Icon is never used in the layout.
SO, we have a b/c either way ?
@nikosdion as a major developer, is it reasonable to replace the array item name? Or am I barking up the wrong tree?
Labels |
Added:
?
|
Category | Modules Administration Layout | ⇒ | Modules Administration Language & Strings Layout Front End Plugins |
Can you please stop changing random things just for change's sake? Every time something like that is changing with no rhyme or reason us 3PDs have to waste our time writing workarounds.
And please stop at-mentioning me. It's annoying and a waste of my time. As long as certain people are in the project who consider my every opinion wrong by default -- even when they end up doing what I suggested, promoting it as their own superbly novel and fantastic idea -- there's no point in me participating. I have better things to do.
@nikosdion don't think the sarcasm was warranted. I asked out of respect for your efforts. You shouldn't imply mine have less value or intent.
@brianteeman I agree with you that its trivial. We're discussing this in the team and have a solution that will work. We're working on the entire icon system that is a huge mess right now, so there are going to be changes that might seem trivial but their goal is to make the entire system make sense.
Leave it as image please.
An icon is an image as well - so it isn't wrong. And it was named like this for a long time and nobody had problems understanding what it is used for.
If its not broken dont improve it so that you can break it
Leave it as image please.
An icon is an image as well - so it isn't wrong. And it was named like this for a long time and nobody had problems understanding what it is used for.
The main issue is, there is both image and icon and yet icon is never used. Makes no sense to have both.
The main issue is, there is both image and icon and yet icon is never used. Makes no sense to have both.
Just delete the never used icon
one.
everywhere else we use $displayData['icon'], its very inconsistent to not do so here.
But we'll address this another way.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-09-12 01:11:01 |
Closed_By | ⇒ | N6REJ | |
Labels |
Added:
?
|
Status | Closed | ⇒ | New |
Closed_Date | 2020-09-12 01:11:01 | ⇒ | |
Closed_By | N6REJ | ⇒ |
Status | New | ⇒ | Pending |
Title |
|
As nicholas already said, don't change 'names' only because they maybe look wrong. We would need to deprecated the old one and force 1000 of developers to change there components without any gain.
PS: this switch statement looks really weird
This really should be closed.
@brianteeman This is going to be rewritten in a way that fully supports both methods. Which is why its in draft mode.
The fact that we are too lazy to fully implement a method that was created 8yrs ago makes us look horrible.
With any luck the old method will be deprecated for J5.
So close this and create a new one or reopen this when you have another attempt.
Just delete the icon and use the image. There is nothing wrong with that, it's clear what it does and nobody had an issue with it for years. The code is not a novel where the choice of words matter. This is code - it's far more important to not touch it when it's not broken,
@N6REJ Please don't get me wrong, but in this case, for this PR here, not for the other icon stuff, I agree with what was said above, that it is change for the sake of change.
You are right that "icon" would be better than "image" to see what it is good for, but that doesn't mean "image" is wrong. An icon is a special kind of image, so "image" is just less precise, but it's not wrong.
Of course my understanding might be wrong because I'm missing something.
But I'd like to ask you to think over if this PR here is really necessary and helpful, or if it might cause more trouble than it solves, and close this PR if you come to the conclusion that it's really not needed.
Alternatively enlight me and maybe otherse so we can better understand why this PR is needed and not just a nice to have for formal or esthetic reasons.
our hands are being tied by b/c constraints so we're closing all pr's for now.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-09-18 06:58:05 |
Closed_By | ⇒ | N6REJ |
After applying the patch the Update checks block is missing any icon