just going to repeat what was said before - some of these changes are creating a dependency on fontawesome in the front end as well as the admin

just going to repeat what was said before - some of these changes are creating a dependency on fontawesome in the front end as well as the admin

I'm more then open to suggestions, but I think part of it is probably a PLT issue. We have that same dependency on icomoon right now.

IIRC @wilsonge said the dependency was ok in the admin but not in the site

We have that same dependency on icomoon right now.

Technically yes but as their code is "icon-file" instead of "fas fa-fw fa-file-0" it is much easier to override the more generic icomoon syntax

IIRC @wilsonge said the dependency was ok in the admin but not in the site

We have that same dependency on icomoon right now.

Technically yes but as their code is "icon-file" instead of "fas fa-fw fa-file-0" it is much easier to override the more generic icomoon syntax

am I actually changing front end icons?
unless I'm sadly mistaken, all icon- will still map. Just our core files will use fa-

@N6REJ @brianteeman no Font Awesome is not ok for the front end:

joomla/40-backend-template#441 (comment)

So in short:

• Fields
• Toolbar
• etc
  that are shared between FE/BE need to cleaned up from those classes and have the respected icon inlined or in some class in the css. I had Prs for some eg password field...

This should fix all icon to fa issues.

This is breaking the star icon for menu items.
You can first apply my patch:
#28131
Look at the screenshots.

But it is the same result without my patch...

look at the star for main menu default home set to All Languages.

After your patch we get

and

I have both patches applied - no problem

@brianteeman
I posted the errors above.

See https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_menus/presets/menus.xml#L39

for example

@wilsonge can I ask why is this even a valid PR (I guess you've already merged couple PRs like this as this is pt3)?
I've created the bridge for the old classes (mooicons) so people wouldn't have to change all the markup. This just renders that useless and if the aim is to remove that you're heading to a quite inconvenient B/C break and the worst part you're following the wrong path once again with dependencies (hardcoded everything to FA is not the way forward, esp with all these leaking to the front end, meaning you'll never be able to ship a site without also including FA in every page, although all that might be needed probably will be a few icons only!)
Also the final code is kinda unattractive:

I've been saying that since the beginning

dependencies (hardcoded everything to FA is not the way forward,

@dgrammatiko
while I can understand what you mean about hard coding your implying the hard coding icon- is fine.

This is breaking the star icon for menu items.

@infograf768 your 2 reverts, put the star back. Thanks.

Also the final code is kinda unattractive:

@dgrammatiko I'll own that. I did the best I knew how with what little I had to work with.
I don't pretend to be as good at coding as most of you.
The way the icons are created now are VERY confusing. Most of the time it seems to be using "name" which isn't always the same as what you want the icon to be in fa which this does address.

your implying the hard coding icon- is fine.

It's not fine but was there since 2.5 also if the project wants to move forward should drop the font implementation and just adopt plain svgs

I did the best I knew how with what little I had to work with.

It's not personal and it's about the code: even if you wrote that inn another fashion (egg: switch) the code will still be ugly to my eyes. It's not something that you did wrong there...

Most of the time it seems to be using "name"

Yes that was reason I created that scss bridge and to keep B/C...

In short I thought that you were removing the deprecated fa and replacing it with whatever is the current one

Yes that was reason I created that scss bridge and to keep B/C...

In short I thought that you were removing the deprecated fa and replacing it with whatever is the current one

that was the first part yes. The we thought, ok.. .we're already deep into this, lets go ahead and just replace the icon- while we're at it so that its consistently .fas fa-xxx
no b/c break because we're keeping the mapping you made.

no b/c break because we're keeping the mapping you made.

B/C is one part of the equation here the other one is that you are hardcoding Font Awesome to the markup which will be a B/C for the next generation contributors and believe me they will blame you big time for that!
FWIW joomla/40-backend-template#441 would be the best way for the icons but George killed it early (and probably without even getting some frondenders advice). Icon fonts are not recommended for some years now for all the very well documented reasons (google it)...

@dgrammatiko if I make J! worse I apologize. Trying to be helpful.

@N6REJ The conflicts reported here come from following recently merged PR: #27915 .

File build/media_source/com_newsfeeds/js/categories-default.es6.js has been removed completely, and file build/media_source/com_contact/js/categories-default.es6.jshas been moved to build/media_source/com_categories/js/shared-categories-accordion.es6.js.

If you want I can solve these conflicts for you.

if I make J! worse I apologize

Already @brianteeman told you that this is wrong #28075 (comment). I just tried to explain the why but you're welcome to pursue your ideas. We're not gatekeepers here I guess...

I have tested this item ✅ successfully on 8289367

I have tested this item ? unsuccessfully on 8289367

No change

@Razzo1987 can you retest please because thats not what I'm seeing.

@N6REJ I think it may be a problem on CI Integration.
I try to test it from Prebuilt packages

If using patchtester 4 rc-2: CI integration is switched off in that version by default because it made problems with some PR's, but these problems did only result in alerts and not in any kind of crash, so it can be switched on again in the patchtester 4 options.

@Razzo1987 If you use patchtester 4 latest rc 2 check my previous comment.

I have tested this item ✅ successfully on 8289367

Re-tested: work!

thanks @richard67

2 good tests ... so RTC.

Hmm, wait, I see label "Conflicting Files" is set .. have to check.

There are no conflicts, I've just made a clean update to the latest changes in the base branch.

Appveyor will fail, that's a know issue currently. But drone should pass.

@N6REJ I've removed RTC for following reason: It seems this PR reverts some changes done in the recently merged PR #28589 . E.g. here: https://github.com/joomla/joomla-cms/pull/28075/files#diff-fab202d130ff6570c9e945edeed1ffe6L87-L91 .

The other PR changed it like you can see here: https://github.com/joomla/joomla-cms/pull/28589/files#diff-fab202d130ff6570c9e945edeed1ffe6R87-R90.

Could you check that? You can see the changes done by the other PR here: https://github.com/joomla/joomla-cms/pull/28589/files.

P.S.: I have no idea why I haven't seen any merge conflict about that when updating your branch to latest 4.0-dev just before.

Added the "Updates Requested" label for reasons stated in my previous comment.

I THINK it was only header.css that was goofed? I'll talk to you about it in the morning if your around. Thanks for the help.

@N6REJ I've fixed the header scss now. Could you just check if the changes of your PR here https://github.com/joomla/joomla-cms/pull/28075/files show what you expect in the following 2 files:

• administrator/templates/atum/scss/blocks/_toolbar.scss
• administrator/templates/atum/scss/template-rtl.scss

If all changes you can see are those changes which you wanted to make with this PR, then all is ok.

But if it shows more changes then that has to be fixed.

@richard67 looks good

I have tested this item ✅ successfully on 8289367

Tested Again and the class is correct

I have tested this item ✅ successfully on 8289367

good for me

@N6REJ I see still a problem. Could you change the following diff for atum's _toolbar.scss?

https://github.com/joomla/joomla-cms/pull/28075/files#diff-570b3b990aae8886b1d92cb9a7f7ef43

It seems you are adding back lines 1 to 19 which have been removed by #28607 . Maybe also something which went wrong when I was updating your branch to latest changes in 4.0-dev?

@Razzo1987 @faustonenci due to recent merge in 4.0-dev I had to make a small update.. could you both please test again.. :(

Looks good to me now. @Razzo1987 @faustonenci Sorry for the inconvenience. That can happen when several contributors work in parallel at the same files. Now it is solved. Could you test again one last time? As soon as it has 2 good tests I will set it to RTC (Reado to Commit). Thanks in advance.

I have tested this item ✅ successfully on 8289367

I tested briefly.

1. git fetch origin pull/28075/head:icon-
2. git checkout icon-
3. npm ci

The buttons in the toolbar now use fa instead of icon.

<button class="button-apply  btn btn-sm btn-success" type="button">
	<span class="fas fa-save" aria-hidden="true"></span>
	Save</button>

I have tested this item ✅ successfully on 8289367

Correct!

@Razzo1987 I hope this was not the only icon you have tested ;-)

I have tested this item ✅ successfully on 8289367

ok, good

@richard67 There are 34 occurrences of "icon" on the page. Most are DIV classes. The only icon is "icon-arrow-down-3"

How, there are 84 "fas" icon now in the page ;)

Maybe I'm doingh something wrong here, but the "Close" buttons here still have "icon-cancel" class, while other buttons related to saving have the right FA class,

Update a few minutes later: False alarm, seems the patch was not applied correctly here.

@N6REJ But now I have really an issue: The icon is missing for the "Associations" button in edit views, e.g. article edit:

It has class="fas fa-contract", but on fontawesome I don't find "contract", I only find things like "file-contract" or "file-signature": https://fontawesome.com/icons?d=gallery&q=contract&m=free.

Could you check?

@Razzo1987 @faustonenci Can you confirm the finding in my previous comment?

@astridx Can you confirm my finding abouve with the associations button?

@richard67 yes, you are right :(
the icon fa-contract doesn't exist in font awesome 5
I suggest to use fa-compress: https://fontawesome.com/icons/compress?style=solid

fa-purge --> fa-trash: https://fontawesome.com/icons/trash?style=solid (Multilingual Associations)
fa-options --> fa-cog https://fontawesome.com/icons/cog?style=solid
fa-refresh --> fa-sync https://fontawesome.com/icons/sync?style=solid (Patch tester)
fa-expired --> fa-minus-circle https://fontawesome.com/icons/minus-circle?style=solid (Patch tester)

fa-save-new --> fa-plus https://fontawesome.com/icons/plus?style=solid (Save & New)
fa-save-copy -> fa-copy https://fontawesome.com/icons/copy?style=solid (Save as Copy)
fa-checkin --> fa-check-square https://fontawesome.com/icons/check-square?style=solid (Article > Actions)

@Razzo1987 Hmm, that looks very different to me to what we have without this PR. I'll set the "Updates Requested" label meanwhile so no other maintainer accidently sets it to RTC.

For icon-contract we can use https://fontawesome.com/icons/compress?style=solid .. is the same as it looked before for the "Associations" button.

@richard67 yes, you are right :(
the icon fa-contract doesn't exist in font awesome 5
I suggest to use fa-compress: https://fontawesome.com/icons/compress?style=solid

fa-purge --> fa-trash: https://fontawesome.com/icons/trash?style=solid (Multilingual Associations)
fa-options --> fa-cog https://fontawesome.com/icons/cog?style=solid
fa-refresh --> fa-sync https://fontawesome.com/icons/sync?style=solid (Patch tester)
fa-expired --> fa-minus-circle https://fontawesome.com/icons/minus-circle?style=solid (Patch tester)

Confirmed, they look right compared to the old ones (I haven't checked the Patchtester Icons though, but from memory this seems to me to be right, too).

=> Ping @N6REJ

To be changed in addition to those reported above:

fa-save-new --> fa-plus https://fontawesome.com/icons/plus?style=solid (Save & New)
fa-save-copy --->fa-copy https://fontawesome.com/icons/copy?style=solid (Save as Copy)
fa-checkin --> fa-check-square https://fontawesome.com/icons/check-square?style=solid (Article > Actions)

Thanks @Razzo1987 .

I don't find these when searching dev nor my pr. I'll work with richard later today to try to figure things out.

Concerning the Associations toolbar, we have in 4.0-dev

.fa-compress::before, .icon-contract::before {
    content: "\f066";
}

code uses for article

$toolbar->standardButton('contract')
						->text('JTOOLBAR_ASSOCIATIONS')
						->task('article.editAssociations'

and elsewhere
ToolbarHelper::custom('item.editAssociations', 'contract', 'contract', 'JTOOLBAR_ASSOCIATIONS', false, false);

That last one uses the method

	public static function custom($task = '', $icon = '', $iconOver = '', $alt = '', $listSelect = true, $formId = null)
	{
		$bar = Toolbar::getInstance('toolbar');

		// Strip extension.
		$icon = preg_replace('#\.[^.]*$#', '', $icon);

		// Add a standard button.
		$bar->appendButton('Standard', $icon, $alt, $task, $listSelect, $formId);
	}

So basically, if we add

if ( $icon === 'contract' )
{
	$icon = 'compress';
}

if ( $icon === 'purge' )
{
	$icon = 'trash';
}
[...] //others

in .../layouts/joomla/toolbar/iconclass.php
we do solve the issue.

@Razzo1987 Luca, can you look again please..
Regarding patch tester, I'm not sure what is going on. the cross reference was and is there yet it's not being used. And I couldn't see "expired" to test it..

if ( $icon === 'refresh' )
{
	$icon = 'sync';
}

@Razzo1987 found the patch tester cause. It's in patch tester itself.
joomla-extensions/patchtester#277

@Razzo1987 @faustonenci @astridx could you guys retest please.. everything should be stellar now.

I find another problem ?

fas fa-joomla install --> fab fa-joomla (administrator/index.php?option=com_joomlaupdate)

Not Solid Icon, but Brand Icon

Also:
fas fa-loop --> fas fa-sync (index.php?option=com_joomlaupdate)
fas fa-stack --> fas fa-copy (index.php?option=com_content&view=articles)
fas fa-address --> fas fa-file-alt (index.php?option=com_workflow&view=workflows&extension=com_content)
fas fa-puzzle --> fas fa-puzzle-piece (index.php?option=com_fields&view=fields&context=com_content.article & index.php?option=com_fields&view=groups&context=com_content.article & index.php?option=com_fields&context=com_contact.contact)
fas fa-zoom-in --> fas fa-search-plus (index.php?option=com_finder&view=index & index.php?option=com_finder&view=maps)

@Razzo1987 @faustonenci @astridx ok, I think I got all of luca's findings done.

icon-equalizer --> fas fa-sliders-h (index.php?option=com_config)
icon-lightning --> fas fa-bolt (index.php?option=com_cache)
icon-checkin --> fas fa-check-square (index.php?option=com_checkin)
icon-comments-2 --> fas fas-comments (index.php?option=com_languages&view=installed)
icon-power-cord --> fas fas-plug (index.php?option=com_plugins)
icon-generic --> fas fa-dot-circle (index.php?option=com_postinstall)
icon-info-2 --> fas fa-info-circle (index.php?option=com_admin&view=sysinfo)

ready to test again

I have not tested this item.

Reset all tests as these are for older commits and need retested.

icon-chevron-down -> fa-angle-down -> administrator/index.php?option=com_content&view=article&layout=edit

icon-publish -> fa-check -> administrator/index.php?option=com_categories&view=categories&extension=com_content

icon-arrow-down-3 -> (dont know the correct fa) -> administrator/index.php?option=com_content&view=articles

icon-arrow-up-3 -> (dont know the correct fa) -> administrator/index.php?option=com_categories&view=categories&extension=com_content

icon-publish -> fa-check -> administrator/index.php?option=com_fields&view=groups

icon-unpublish -> (not sure of the fa) -> administrator/index.php?option=com_fields&view=groups

icon-archive -> fa-folder -> administrator/index.php?option=com_fields&view=groups&context=com_content.article

icon-trash -> fa-trash -> administrator/index.php?option=com_fields&view=groups&context=com_content.article

icon-checkedout -> (Do not know the fa) -> administrator/index.php?option=com_fields&view=groups&context=com_content.article

icon-arrow-down-3 -> (dont know the correct fa) -> administrator/index.php?option=com_content&view=articles

icon-arrow-up-3 -> (dont know the correct fa) -> administrator/index.php?option=com_categories&view=categories&extension=com_content

fas fa-sort
fas fa-sort-up
fas fa-sort-down

https://fontawesome.com/icons/sort?style=solid

From @jwaisner commets:
icon-unpublish --> fas fa-times
icon-checkedout --> fas fa-lock

Others:
icon-new --> fas fa-plus

Also:
Remove tab space before " fas fa-cog" and ""

icon-chevron-down -> fa-angle-down -> administrator/index.php?option=com_content&view=article&layout=edit

should be fa-chevron down. FIXED

@Razzo1987 I could use some help finding this one...
icon-publish -> fa-check -> administrator/index.php?option=com_categories&view=categories&extension=com_content

I could swear this was fixed before. checking the code it appears it should be.

@Razzo1987 @richard67 @wilsonge @jwaisner
fixing

icon-publish -> fa-check -> administrator/index.php?option=com_categories&view=categories&extension=com_content

in commit 7fe5814 changes the unpublish icon color to red, as well as the "pinned" icon color ( see banners ). While this matches scss pre-commit it doesn't match the previous behavior ( grey ) as shown here.

.icon-publish,
.fa-check {
  color: theme-color("success");

  .dropdown-status-group & {
    color: inherit;
  }
}

.icon-unpublish,
.fas.fa-times {
  color: theme-color("danger");

  .dropdown-status-group & {
    color: inherit;
  }
}

I don't know wether I should alter the scss to match the gray OR leave that for UI folks to figure out.

icon-publish -> fa-check -> administrator/index.php?option=com_fields&view=groups

@jwaisner what am I missing? This is default "testing" dataset

@N6REJ Looks like the changes made in your latest commit fixed it. I also didn't provide the correct URL apparently.

There is something odd though with the latest commit. When I "archive" an item in the field groups (administrator/index.php?option=com_fields&view=groups&context=com_content.article) I get the fa-times instead of fa-folder.

@jwaisner did the last pr break that or was it broken before?

@richard67 still showing a conflict after it's been fixed.
0220621

@N6REJ Because you have not really solved the conflict, or at least not all of them.

This time I've solved it for you, but you should sooner or later learn how to do that yourself if you really want to do development in a project managed by a version control, e.g. git, and not only one person = you working on a project but a group of people.

Here what you should know:

Conflicts are marked in the usual way by git in following way:

Between the "<<<<<<<" and the "=======" os what you would see at the left hand side in a diff, where the left hand side shows you branch.

Below, between the "=======" and the ">>>>>>>" is the right hand side, 4.0-dev branch.

You have to decide for one of these 2, or if it is necessary make a kind of merge, bringing both changes together in one of them and remove the other. At the no "<<<<<<<" and "=======" and ">>>>>>>" is left, only one of the 2 variants or a mix of them.

In the case shown above your branch was the right one, so the solution was to remove all what is stiked through with a thick red line in the next screen shot:

As I said, this is basic knowledge when working on a software project where it can happen that more than 1 person works on a file.

administrator/index.php?option=com_fields&view=groups&context=com_content.article
NOTE:
libraries->src->HTML->Helpers->JGrid.php needs to have lines 86 & 102 adjusted so reflect the proper possible states.

$html[] = '<span class="fas fa-' . ($active_class === 'publish' ? 'check' : 'times') . '" aria-hidden="true"></span>';

for line 86 won't work as it causes archive to be times as well.
line 86 resolved in 4bb293f Needs peer review
line 115, Not sure what the possible $inactive_class possibilities would be.

fas fa-puzzle -> fas fa-puzzle-piece -> administrator/index.php?option=com_fields&view=group&layout=edit&id=1&context=com_content.article in title bar
Can't find "puzzle" anywhere.

icon-arrow-down-3 -> (dont know the correct fa) -> administrator/index.php?option=com_content&view=articles

icon-arrow-up-3 -> (dont know the correct fa) -> administrator/index.php?option=com_categories&view=categories&extension=com_content

@jwaisner Can you show me where your seeing this please?

Also:
Remove tab space before " fas fa-cog" and ""

@Razzo1987 where?

administrator/index.php?option=com_content&view=articles

icon-arrow-down-3 -> (dont know the correct fa) -> administrator/index.php?option=com_content&view=articles
icon-arrow-up-3 -> (dont know the correct fa) -> administrator/index.php?option=com_categories&view=categories&extension=com_content

@jwaisner Can you show me where your seeing this please?

Icon-arrow-down-3

Icon-arrow-up-3

administrator/index.php?option=com_content&view=articles

icon-arrow-down-3 -> (dont know the correct fa) -> administrator/index.php?option=com_content&view=articles
icon-arrow-up-3 -> (dont know the correct fa) -> administrator/index.php?option=com_categories&view=categories&extension=com_content

@jwaisner Can you show me where your seeing this please?

Icon-arrow-down-3

Icon-arrow-up-3

fixed in 4b67610

fas fa-puzzle -> fas fa-puzzle-piece -> administrator/index.php?option=com_fields&view=group&layout=edit&id=1&context=com_content.article in title bar
Can't find "puzzle" anywhere.
fixed in 0cf4c46

@Razzo1987 @jwaisner @richard67 unless I missed something we're finally ready for retesting.

@N6REJ The icons in articles (and probably elsewhere) lack the color or correct image. I have ran npm ci See screenshot below:

Also drone has a lot of errors....

@jwaisner I think @N6REJ should update his branch by the lates changes in the 4.0-dev branch of the cms.

I'll look again tomorrow.. this did NOT show up when I was looking.

@jwaisner I don't see that

@N6REJ Icons look fine after latest commit to PR. I will do some testing tonight.

icon-eye -> fa-eye -> administrator/index.php?option=com_content&view=article&return=featured&layout=edit&id=24&return=featured

icon-publish -> fa-check -> administrator/index.php?option=com_workflow&view=workflows&extension=com_content

Also looks as if the featured/unfeatured icons are not displaying correctly.

icon-featured -> fa-star -> administrator/index.php?option=com_menus&view=items&menutype=

fa-starstar -> fa-star -> administrator/index.php?option=com_contact&view=contacts

icon-eye -> fa-eye -> administrator/index.php?option=com_content&view=article&return=featured&layout=edit&id=24&return=featured

I could use some help tracking this one down. I can't find anywhere where 'eye' is used solo. No idea how this icon-eye is being generated.

@jwaisner all but eye should be fixed.

@Razzo1987 @jwaisner @richard67 READY FOR TESTING

icon-publish -> fa-check -> administrator/index.php?option=com_workflow&view=workflows&extension=com_content

Also seems to be something is not right with the default icons in the workflows

icon-featured ->fa-star -> administrator/index.php?option=com_menus&view=items&menutype=

@Razzo1987 @jwaisner @richard67 READY FOR TESTING
JGrid.php changed so we shouldn't have any problems anymore.

@N6REJ Please fix conflicts.

@N6REJ See N6REJ#11 for fixing the conflicts and an error you made with missing ?> at the end of file layouts/joomla/button/iconclass.php.

fa-times is now set to var(--danger) colour, i.e. red in

diff --git a/administrator/templates/atum/scss/pages/_com_privacy.scss b/administrator/templates/atum/scss/pages/_com_privacy.scss
index 9db4407917e8..64a2387825ad 100644
--- a/administrator/templates/atum/scss/pages/_com_privacy.scss
+++ b/administrator/templates/atum/scss/pages/_com_privacy.scss
@@ -1,7 +1,8 @@
 // com-privacy
 
 .tbody-icon {
-  .icon-download {
+  .icon-download,
+  .fas fa-download {
     color: var(--success);
     border-color: var(--success);
   }
@@ -11,7 +12,8 @@
     border-color: var(--primary);
   }
 
-  .icon-delete {
+  .icon-delete,
+  .fa-times {
     color: var(--danger);
     border-color: var(--danger);
   }

That is wrong as it is also used for Unpublished generally speaking and TFA in user manager.

Similar issue I guess for Cassiopea.

@infograf768 that is in core.

I'm not going to change icon styling in this pr. I mentioned it before #28075 (comment) and got zero feedback.

@N6REJ See N6REJ#11 for fixing the conflicts and an error you made with missing ?> at the end of file layouts/joomla/button/iconclass.php.

thanks @richard67 , really appreciate all the work you've put into this.

@N6REJ when I toggle the show password during the user creation process it does not work after applying your PR. The icon color changes but it does not show the password and the icon does not change as expected.

administrator/index.php?option=com_users&view=user&layout=edit

I do get the following error in the browser inspector:

EXPECTED:

ACTUAL:

Sounds as if some js needs love, too.

More JS errors:

administrator/index.php?option=com_fields&view=fields&context=com_users.user

administrator/index.php?option=com_fields&view=groups&context=com_users.user

@jwaisner These JS errors may not be related to this PR. Please enable Global Configuration > System > Debug System and review errors.

@N6REJ when I toggle the show password during the user creation process it does not work after applying your PR. The icon color changes but it does not show the password and the icon does not change as expected.
I see the icon problem.. aside from icon-eye not belonging there the lack of a space is messing things up.. I'll work on it.

@jwaisner when I test it ( after sharky's fixes ) the icon is fine now but doesn't toggle at all. Thats not an issue I can address in this pr as I don't think it's related and i know jack about js.

@N6REJ did you run node build.js --compile-js after applying my suggestions?

I have tested this item ✅ successfully on 8289367

After an extensive run through of all icons, I was not able to find any instances of Icon and all Icons are displaying and functioning correctly.

#29016 changed the default icon from star to home. Update it in this PR?

#29016 changed the default icon from star to home. Update it in this PR?

@jwaisner hate to bother you after your wonderful test but could you please verify the last 2 commits that address the above?

@Quy thanks for the catch. Definitely needed to be changed in this pr.

I have tested this item ✅ successfully on 8289367

Test is successful. Icon changed to HOME from STAR

@N6REJ
Since #29050 has been merged, you need to correct the conflicts and make sure what is done in that PR is not lost.

npm ci is required due to merging #29050 changes.

administrator/components/com_media/package-lock.json created by mistake?

wth

administrator/components/com_media/package-lock.json created by mistake?

wth... that is not even possible with npm ci anymore
i'll purge it.. weird!..

@N6REJ Please fix #29188.

@Quy I guess you mean he shall fix that issue in a separate PR, not in this one here, right?

It can be in this PR or in a separate PR.

Separate PR could be faster because easier to test.

merge this one first.

I have tested this item ✅ successfully on 8289367

RTC

Restarted drone

YAY

#29188 is not solved by this PR here.

@richard67 nope, I'll address it in another pr the minute this one is merged.

I have a problem here for a third party component where my Jgrid is using icomoon and therefore icon and not fa
For example
I use normally icon-16-inlanguage
But with this patch it gives fas fa-16-inlanguage and I lose my icons.

This means I have to modify all my css and code.
Is icomoon definitely out of J4?

yes, as an extension developer you can include the icons with your extension

yes, as an extension developer you can include the icons with your extension

This does not solve my issue.
Indeed I see that icomoon is loaded by default with fontawsome

// B/C for Icomoon
@import "../../../../../../build/media_source/system/scss/icomoon";

but this PR modifies the grid as stated above in such a way that I am forced to modify all my css.

No that is not loading the icomoon font. If you actually look at the file you will see what it is

but this PR modifies the grid as stated above in such a way that I am forced to modify all my css.

This an unneeded B/C break introduced by this PR, but hey people tested it and hey ho it's ready to commit ?

there should be no break afaik... icons still function just fine, its just that they are switched to FA.

there should be no break afaik

If there wasn't any then @infograf768 wouldn't had to to post this:

This means I have to modify all my css and code.

The code in Joomla is not to be tested only with the core, it needs to also work with 3rd PD code. Most of us on our code have the old IcoMoon classes (thus I create the bridge scss). What you're doing might seems fine but you're asking everybody to redo their part with FA. That's not fine, it's the definition of breaking B/C.

it's the definition of breaking B/C.

Seriously Dmitris you're talking about code changing in J4 breaking B/C

Seriously Dmitris you're talking about code changing in J4 breaking B/C

YES. I created that file so those classes won't change because there is more than some classes changed. Mind you that people will have to maintain both J3 and J4 versions of their extensions for at least 2 years. The more useless breaks introduced the more stupid work all of us will get. Also this is the backend and you're already loading both css files so what's the gain to move from the well known classes to the new ones?
I'm still trying to understand what's the gain of this transition? Give up a more than 10 years contract (anything that is published from Joomla is essentially a contract with the devs) for what?
You still have FA classes, why do you need to break things?

If you look back at the history of @N6REJ work on this you will see I was not in favour ofr getting rid of the fa classes

I have a problem here for a third party component where my Jgrid is using icomoon and therefore icon and not fa
For example
I use normally icon-16-inlanguage
But with this patch it gives fas fa-16-inlanguage and I lose my icons.

This is probably some of the stuff we need to roll back. I'm not fussed about converting the code where we're just outright hardcoding properties - which is the majority of places in this PR - but the toolbar section with the multi-if statements is probably taking this too far. If people need a font-awesome explicit class there it's probably better for them to override the layout instead of that thing. The JGrid if/else is also the source of JM's problems. My suggestion would be to drop those changes from this PR. Happy to merge the conversion of the stuff we have. and if we have a handful of small places in the library code where we've still got icon references then that's probably fine - the sass code is going to handle that for the forseeable.

@wilsonge the way things were written, both the conversion tables are mandatory if there is going to be ANY translation at all.
It was a long journey to understand how most things work.

toolbar & buttons:
those files are called MOST of the time to apply the class format. This is as before. The numerous if/thens were required to translate ANY of the icons to FA's.
I was not able to locate the EXACT source of the calls.

jgrid:
Jgrid & jtoolbar are gorgeous imo. Simply pass an icon name to it and viola`. There is a few oddities in that it's inconsistent in how the icons are named but other then that it works very well. The if/thens were necessary to make the conversion happen.

If we rollback the conversion system then 99.9% of the icons WILL be icon- and frankly there is no need to have fontawesome in the first place as it just messes things up since there is no clear consistent method to use an icon. Icons are defined in several different methods instead of a consistent method like jtoolbar uses.

the fa system was never built properly imo. A good example of this is

In some places .fa is properly attached to its icon- in .scss in others not at all ( this was to be addressed in another pr since it's related to styling which is constantly changing )

There is currently ZERO method to call a .fas fa- icon. ALL icon calls are icon-
I fully understand the problem with jm's issue and why it's a problem. I'm simply laying things out that I've discovered in the 100+ hours I've spent on this.

I think I can think of a simple method for jtoolbar that will allow both by simply prefacing the icon name with fa when you want fa but I'll have to check.
Since I don't know where the icon name is being generated in jgrid, and the toolbar/icon.php(s) I can't do the same there.

Please let me know what you want me to do.
Do you want to abandon this pr completely?

@infograf768 jm, what is the call(?) your using to jgrid?

FYI, before that patch, I had already modified my css 1 year and a half ago for this J4 only component to use

.icon-16-global:before,
.iconlist-16-global:before {
	content: "";
	color: #796D67;
}

.icon-16-local:before,
.iconlist-16-local:before {
	content: "";
	color: #EF8F00;
}

.icon-16-core:before,
.iconlist-16-core:before {
	content: "";
	color: #000000;
}
etc.

instead of these in J3

.icon-16-global:before,
.iconlist-16-global:before {
	content: "!";
	color: #796D67;
}
.icon-16-local:before,
.iconlist-16-local:before {
	content: "-";
	color: #EF8F00;
}
.icon-16-core:before,
.iconlist-16-core:before {
	content: "\e200";
	color: #000000;
}

which was/is working fine.

I use for example the code
<?php echo HTMLHelper::_('jgrid.action', $i, '', array('tip'=>true, 'inactive_title'=>Text::_('COM_LOCALISE_TOOLTIP_TRANSLATIONS_CLIENT_'.$item->client), 'inactive_class'=>'16-'.$item->client, 'enabled' => false, 'translate'=>false)); ?>
To get with the css

.icon-16-site:before,
.iconlist-16-site:before {
	content: "";
	color: #2A7BA1;
}

With this patch, I have to use

.fa-16-site:before,
.iconlist-16-site:before {
	content: "";
	color: #2A7BA1;
}

But, as the classes of type .icon-16-site are also used in other parts of the component, I therefore have to modify all.
Including some like

span.icon-16-notice-note {
    background: url("../../../media/com_localise/images/icon-16-notice-note.png");
    background-repeat: no-repeat;
    height: 16px !important;
    vertical-align: text-top;
    width: 16px !important;
}

It is indeed possible. But a PITA to redo again all this.

One may say that anyway any component has to be heavily modified for J4 and the notion of B/C here is similar to the 3% deficit limit supposedly imposed until now to the European Union countries, but imho J4 should not be compared to a pandemia. ?

if we add another variable to the function(s) we could open it up so that ANY icon class could be used by simply stating the family type.
public static function action($i, $task, $prefix = '', $active_title = '', $inactive_title = '', $tip = false, $active_class = '', $inactive_class = '', $enabled = true, $translate = true, $checkbox = 'cb', $formId = null
might become
public static function action($i, $task, $prefix = '', $active_title = '', $inactive_title = '', $tip = false, $active_class = '', $inactive_class = '', $enabled = true, $translate = true, $checkbox = 'cb', $formId = null, $icon_family
you could then use
$html[] = '<span class="'$icon_family . $active_class . '" aria-hidden="true"></span>';
the question at that point would be what should the default family be.

In my specific case, I can solve it with
$html[] = '<span class="fas fa-' . $inactive_class . ' icon-' . $inactive_class . '" aria-hidden="true"></span>';

In my specific case, I can solve it with
$html[] = '<span class="fas fa-' . $inactive_class . ' icon-' . $inactive_class . '" aria-hidden="true"></span>';

is adding icon- to your $inactive_class a valid solution? so your $inactive class would be the full class.

@infograf768 could you please retest with your original configuration?

@wilsonge assuming it works for jm, I believe I have it configured now where it will work for all 3rd party extensions & is more flexible. As I stated before if you really want the if's removed I'll revert jgrid & both iconclass.php's to their current 4.0 files. Pretty sure it will break things but...........

It now works for me.
I suggest someone tests with another 3rd party component to be sure.

@N6REJ Your conflict fixes look wrong to me because they seem to undo changes done in the 4.0-dev branch recently for the new workflows.

Please either make sure you update your branch either to the 4.0-dev branch of the CMS repository, or if you update it to your 4.0-dev branch in your repository then make sure that that one is up to date with the 4.0-dev branch of the CMS.

To me it looks as if you have done your conflicts resolution nased on an outdated 4.0-dev branch of your repository.

You can verify that in the changes of this PR: It shows for the files where you just have solved the conflicts more than only the changes you want to make.

reverted commit fix to double check things per @richard67 statement

@N6REJ I think it looks better now but still I see something which looks as if it had been removed by the new workflows version and now your PR adds it back here:

grrrrrr it was supposed to remove it.

Looks ok now regarding being up to date with latest 4.0-dev and conflicts.

@Razzo1987 @Quy @jwaisner Could you give it again a test? Thanks in advance.

@wilsonge Can we get this merged now please?