? ? Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
2 Feb 2020

PR for #27770

Before this PR In the dashboards of joomla 4 we have a situation where sometimes there is an icon and sometimes not.

For all the static preset dashboards the icon was coded in the preset
For the dynamic dashboard for components there was no icon

With this PR then the components module will display icons now. For core components this is done by updating rhe menu table.

Any extension that already defines an icon eg com_pathctester will also work as this is fully backwards compatible

Extension developers can still add either an image or an icon via their components xml

eg
<menu img="class:icon_name">COM_MYCOMPONENT</menu> or
<menu img="pathto/image_name">COM_MYCOMPONENT</menu> or
<menu img="image:pathto/image_name">COM_MYCOMPONENT</menu>

This is a DRAFT. It is ready for testing but I haven't done the install sql or updated the core component xml yet until after feedback.

Documentation Changes Required

Hopefully none as the menu aspect of the xml should already be documented

avatar joomla-cms-bot joomla-cms-bot - change - 2 Feb 2020
Category SQL Administration com_admin Modules
avatar brianteeman brianteeman - open - 2 Feb 2020
avatar brianteeman brianteeman - change - 2 Feb 2020
Status New Pending
avatar Bakual
Bakual - comment - 2 Feb 2020

Looks like a good approach to me.
I guess the hardcoded stuff in the menu code can then be removed as well?

avatar brianteeman
brianteeman - comment - 2 Feb 2020

I guess the hardcoded stuff in the menu code can then be removed as well?

Actually not as those components do not have an entry in the _menus table

avatar Bakual
Bakual - comment - 2 Feb 2020

Aww, crap. That's true.
Maybe taking it from the menu preset could work? As I understood those presets there is one for the menu and then several for the dashboards. But I may be wrong, I still don't understand those presets.

avatar brianteeman
brianteeman - comment - 2 Feb 2020

Let's tackle one thing at a time ;)

If you could be so kind as to test what I have done so far then I can finish it

  • sql installs
  • postgresql updates
  • component xml (not used but would be good to be consistent)

Note one thing about my implementation is that it does allow for dashboards to be created when using the alternate template as it uses the same field and formatting for the icon/image (assuming someone creates the dashboard code.

There were many ways to achieve this but I really tried for the most compatible method - not necessarily the cleverest.

avatar Bakual Bakual - test_item - 3 Feb 2020 - Tested successfully
avatar Bakual
Bakual - comment - 3 Feb 2020

I have tested this item successfully on 9b2a716


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

avatar Bakual
Bakual - comment - 3 Feb 2020

I even tried adding img="class:comment" to my component menu declaration and it now shows an icon for it in the "Components Dashboard".
So that imho is a great solution which also works for 3rd parties.

avatar brianteeman
brianteeman - comment - 3 Feb 2020

Thanks @Bakual - I will complete the PR now

avatar joomla-cms-bot joomla-cms-bot - change - 3 Feb 2020
Category SQL Administration com_admin Modules SQL Administration com_admin Postgresql Modules
avatar brianteeman brianteeman - change - 3 Feb 2020
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 3 Feb 2020
Category SQL Administration com_admin Modules Postgresql SQL Administration com_admin Postgresql com_associations com_banners com_contact com_finder com_newsfeeds Modules
avatar joomla-cms-bot joomla-cms-bot - change - 3 Feb 2020
Category SQL Administration com_admin Modules Postgresql com_associations com_banners com_contact com_finder com_newsfeeds SQL Administration com_admin Postgresql com_associations com_banners com_contact com_finder com_newsfeeds Modules Installation
avatar brianteeman brianteeman - change - 3 Feb 2020
The description was changed
avatar brianteeman brianteeman - edited - 3 Feb 2020
avatar brianteeman
brianteeman - comment - 3 Feb 2020

No longer a draft and ready for testing

avatar Bakual
Bakual - comment - 3 Feb 2020

Just noticed that News Feeds doesn't show the RSS icon you specified. I guess that is no longer (?) part of our FA package?
Or I did something wrong with all the composer/npm stuff

avatar brianteeman
brianteeman - comment - 3 Feb 2020
avatar Bakual
Bakual - comment - 3 Feb 2020

It may well be that I messed up something in my local installation. If it shows for you and others, then it's fine.

avatar infograf768
infograf768 - comment - 8 Feb 2020

Could you post a screenshot of the resulting modification? For example concerning Multilingual Associations.

avatar brianteeman
brianteeman - comment - 8 Feb 2020

I am not aware of any way this impacts on multi lingual associations. But try it and see.

avatar infograf768
infograf768 - comment - 8 Feb 2020

It impacts it by adding an icon in the dashboard.
For testers to know what to expect here is a screenshot after patch where the icons are present next to each component title.
Screen Shot 2020-02-08 at 11 30 05

avatar infograf768
infograf768 - comment - 8 Feb 2020

conflicts

avatar brianteeman
brianteeman - comment - 8 Feb 2020

It's not an impact it is the desired and intended behaviour as described in the pull request

avatar infograf768
infograf768 - comment - 8 Feb 2020

Cool down. By impact, i meant modify and it looks good. (I am not British).
conflicts remain to be solved.

avatar brianteeman
brianteeman - comment - 8 Feb 2020

conflicts resolved

avatar Bakual
Bakual - comment - 9 Feb 2020

@brianteeman I wanted to test your last changes again, and got a very messed up page. To verify it's not on my end, I've did a complet fresh installation and it was still the same. So probably a wrong conflict solving. Can you check if that's the same for you?
"News Feeds" is still not showing the icon - don't know what's wrong here and why it's showing for you and JM (according to his screenshot).
image

avatar brianteeman
brianteeman - comment - 9 Feb 2020

probably a conflict with the changes in #27777

avatar Bakual
Bakual - comment - 9 Feb 2020

Indeed, with this PR you're reverting some of the changes you did there. Eg the <div class="module-wrapper"> is removed again. Probably other changes as well. You need to check that layout again.

avatar sakiss
sakiss - comment - 13 Feb 2020

It breaks the layout in my case:
component_dash_before
After patch
component_dash_after

avatar brianteeman
brianteeman - comment - 13 Feb 2020

@Bakual @sakiss Please can you retest. There was an extra closing div after the merge conflict fix that was breaking the display

avatar Quy
Quy - comment - 13 Feb 2020

Still display issue on a new installation.

27773

avatar Bakual
Bakual - comment - 13 Feb 2020

Still doesn't look right to me.
Have a look at administrator/modules/mod_submenu/tmpl/default.php. If I remember right, you only changed the lines regarding the icon, but now there is a whole bunch of lines changed in the PR.
Still looks like a bad merge conflict solving.

avatar brianteeman
brianteeman - comment - 14 Feb 2020

Sorry for the bad merge conflict - now I really am sure it is correct

avatar Bakual Bakual - test_item - 14 Feb 2020 - Tested successfully
avatar Bakual
Bakual - comment - 14 Feb 2020

I have tested this item successfully on 1684df9

Works perfectly fine again!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27773.
avatar Quy Quy - test_item - 14 Feb 2020 - Tested successfully
avatar Quy
Quy - comment - 14 Feb 2020

I have tested this item successfully on 4aed3ff


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

avatar Quy Quy - change - 14 Feb 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 14 Feb 2020

RTC


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

avatar wilsonge wilsonge - change - 16 Feb 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-02-16 12:15:31
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 16 Feb 2020
avatar wilsonge wilsonge - merge - 16 Feb 2020
avatar wilsonge
wilsonge - comment - 16 Feb 2020

Thanks!

avatar brianteeman
brianteeman - comment - 16 Feb 2020

thx

Add a Comment

Login with GitHub to post a comment