User tests: Successful: Unsuccessful:
Pull Request for Issue # .
With this PR merged all hard coded icons <span class="fas fa-some-kind-of-icon" aria-hidden="true" and-more-attributes></span>
are being replaced by the JLayout implemented with #30470
The reason behind this is to make Joomla more flexible to switch to other icon libraries, or to implement your own.
With this PR merged the declaration of icons is only be done in https://github.com/joomla/joomla-cms/blob/4.0-dev/layouts/joomla/icon/iconclass.php
All icons are hard coded.
Most of all icons are set through the LayoutHelper('joomla.icon.iconclass', ['icon' => 'some-kind-of-icon-type'])
I don't know if there is documentation about the present LayoutHelpers and how they are / should be used.
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_admin com_associations com_banners com_cache com_categories com_config com_contact com_content com_contenthistory com_cpanel com_csp com_fields com_finder |
Labels |
Added:
?
|
Title |
|
what about the old style of icons used by extensions - I assume they still work?
I'm not changing the look and feel of rendering the HTML output for example<span class="fas fa-spinner" aria-hidden="true"></span>
. Only the way Joomla core adds an icon in the core by using a JLayout.
there are some places where the icon is set/changed in js eg show password
I know... haven't found out how to tackle that part, so I skipped it for a later change.
Category | Administration com_admin com_associations com_banners com_cache com_categories com_config com_contact com_content com_contenthistory com_cpanel com_csp com_fields com_finder | ⇒ | Administration com_admin com_associations com_banners com_cache com_categories com_config com_contact com_content com_contenthistory com_cpanel com_csp com_fields |
That PR looks like an performance nightmare and makes templates much more complex.
If someone want's to replace the Icons I would suggest to override the template or the css classes.
I'm not convinced by this in the admin - I'm not so fussed about coupling to font awesome there. Can see the potential win in frontend however.
@HLeithner is there a test that can be run to measure performance other then on a page by page basis?
I huge part of the problem is the restriction that we must maintain b/c.
That PR looks like an performance nightmare and makes templates much more complex.
If someone want's to replace the Icons I would suggest to override the template or the css classes.
Wasn't this the whole purpose of JLayouts? To code DRY. Replacing hard coded icons by a JLayout will make it DRY. One place where you control the icons used, instead of scattered around the entire project.
I'm not convinced by this in the admin - I'm not so fussed about coupling to font awesome there. Can see the potential win in frontend however.
Since you're the one with the force to merge it branch 4.0-dev there is no use to continue with this PR. I'll close it, but will create a new PR where the JLayout is extended for the use of frontend
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-09-15 06:12:29 |
Closed_By | ⇒ | hans2103 |
not at my pc yet so quick questions
what about the old style of icons used by extensions - I assume they still work?
there are some places where the icon is set/changed in js eg show password