Bug 32910

Created attachment 146396 [details] [review]
Bug 32910: Adjust fontawesome icons ref

I had a quick try at upgrading to V6, but it needs more attention.

Especially a better focus on those 2 pages:
https://fontawesome.com/v5/docs/web/setup/upgrade-from-v4
https://fontawesome.com/docs/web/setup/upgrade/

I didn't manage to attach the patches here, see https://gitlab.com/joubu/Koha/-/commits/bug_32910

I have made some follow-up changes:

https://gitlab.com/koha-dev/koha-dev/-/tree/bug_32910

I tested Owens branch and couldn't find any obvious regressions, it's looking great and I can't wait to start using the new set.

I'm going to mark the bug as signed off, please add my SO line to the relevant branch when appropriate :).

Jonathan we're already installing some Font Awesome assets via npm, correct? For ERM? I'm wondering if we shouldn't also get the FA SCSS from there and add it to the SCSS build.

(In reply to Owen Leonard from comment #6)
> Jonathan we're already installing some Font Awesome assets via npm, correct?
> For ERM? I'm wondering if we shouldn't also get the FA SCSS from there and
> add it to the SCSS build.

Yes, we are using some fontawesome vue modules, but not a lot, and not for all the icons we use in vue files.

See the output of
% git grep awesome koha-tmpl/intranet-tmpl/prog/js/vue # Vue module
vs
% git grep fa- koha-tmpl/intranet-tmpl/prog/js/vue # Not vue module

At this point we should actually the vue module them from there. The problem is that we are using edit/show/delete icons in the "Actions" columns of the tables, and we cannot use them there.


So, yes, we could install them differently, but not because we are doing it differently for Vue ;)

I think it's a great idea to shift installation of these assets to the build process.. :)

Let push and see how it goes?

Signed-off-by lines and rebase at https://gitlab.com/joubu/Koha/-/commits/bug_32910

Owen, can you please rebase?

Any chance for this one still? I'd like to pick a new icon for inventory from the newer version :)

(In reply to Jonathan Druart from comment #9)
> Let push and see how it goes?

This was more than 2 months ago, to have time to fix before the release. Now we are few weeks before the release. Why did we have wait so much time to push it? 
These patches are unnecessary painful to rebase!

Are we part of a kind of masochist secret group?

Too many conflicts, I am giving up.

On top of the remote branch, I think there are some occurrences missing

% git grep '\-o"' **/*.tt **/*.inc

(In reply to Jonathan Druart from comment #14)
> (In reply to Jonathan Druart from comment #9)
> > Let push and see how it goes?
> 
> This was more than 2 months ago, to have time to fix before the release. Now
> we are few weeks before the release. Why did we have wait so much time to
> push it? 

I was willing to push it once I noticed it existed, but it already needed a rebase. I guess we've been busy with other things...

I've rebased this branch: https://gitlab.com/koha-dev/koha-dev/-/tree/bug_32910

(In reply to Owen Leonard from comment #18)
> I've rebased this branch

Before seeing comment 16, so there are still a few missing

Early push candidate for 23.11

Thanks Owen!

I think there is this one left:
koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload-images.tt:        .fa-file-archive-o {


Also, are you sure we need v4-shims?

> koha-tmpl/intranet-tmpl/prog/en/modules/tools/upload-images.tt:       
> .fa-file-archive-o {

Remote branch rebased again and updated to fix a couple more missing references and to remove unused assets from the staff interface and OPAC
 
https://gitlab.com/koha-dev/koha-dev/-/tree/bug_32910

Pushed to master for 23.11.

Nice work everyone, thanks!

I pushed a follow-up to fix Asset.t

Created attachment 152412 [details] [review]
Bug 32910: Fix 'Agreements' icon

Follow-up pushed to master. Thanks Jonathan!

*** Bug 32018 has been marked as a duplicate of this bug. ***

I would have loved to have seen this in 23.05.x, but for now, I can't justify this enhancement.

I think the screenshots will be updated as we document other things. Unless this changes something specific, I will make it resolved. Feel free to reopen with more details if I'm wrong...