Bug 14915 - Use Font Awesome instead of Glyphicons for the staff intranet
Summary: Use Font Awesome instead of Glyphicons for the staff intranet
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 13696
Blocks:
  Show dependency treegraph
 
Reported: 2015-09-28 14:41 UTC by Kyle M Hall
Modified: 2017-06-14 22:11 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14915 - Use Font Awesome instead of Glyphicons for the staff intranet (103.84 KB, patch)
2015-09-28 14:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14915 - Use Font Awesome instead of Glyphicons for the staff intranet (103.83 KB, patch)
2015-09-28 15:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
[Signed-off] Bug 14915 - Use Font Awesome instead of Glyphicons for the staff intranet (104.09 KB, patch)
2015-09-29 06:44 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14915 - Use Font Awesome instead of Glyphicons for the staff intranet (104.01 KB, patch)
2015-10-08 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14915 [QA Followup] - Tweak '+' icon for serials-home (2.88 KB, patch)
2015-10-08 18:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 13696 [QA Followup] - Add Font Awesome license to the about page (1.38 KB, patch)
2015-10-25 22:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14915 - Use Font Awesome instead of Glyphicons for the staff intranet (104.33 KB, patch)
2015-10-25 22:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14915 [QA Followup] - Tweak '+' icon for serials-home (2.96 KB, patch)
2015-10-25 22:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14915: QA Follow-up - Replace fa-remove-sign with fa-times-circle (9.45 KB, patch)
2015-10-25 22:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14915: QA Follow-up - Replace fa-hdd with fa-save (4.32 KB, patch)
2015-10-25 22:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14915: QA Follow-up - Replace fa-downlowd-alt with fa-download (4.60 KB, patch)
2015-10-25 22:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14915: QA Follow-up - Replace fa-zoom-in and fa-zoom-out (1.73 KB, patch)
2015-10-25 22:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14915: QA Folloq-up - Replace fa-merge with fa-compress (2.25 KB, patch)
2015-10-25 22:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14915: [FA 4.4] QA Follow-up - Replace fa-hold with fa-sticky-note-o (4.85 KB, patch)
2015-10-25 22:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14915: QA Follow-up - Replace fa-time with fa-clock-o (2.05 KB, patch)
2015-10-25 22:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14915 [QA Followup] - Switch recent commits from Glyphicons to Font Awesome (17.47 KB, patch)
2015-10-26 17:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14915: (follow-up) [FA 4.4] (QA followup) Replace fa-hold with fa-sticky-note-o (1.32 KB, patch)
2015-11-13 14:17 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2015-09-28 14:41:19 UTC
We should be using Font Awesome for our icons instead of Glyphicons, for the reasons discussed on bug 13696.
Comment 1 Kyle M Hall 2015-09-28 14:47:48 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2015-09-28 15:02:24 UTC Comment hidden (obsolete)
Comment 3 Marc Véron 2015-09-29 06:44:11 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-10-01 11:27:47 UTC
I think this needs small tweaks.
For instance, compare the "+" icon on /cgi-bin/koha/serials/serials-home.pl
Before this patch, it was correctly aligned, now it's not.
Comment 5 Kyle M Hall 2015-10-08 18:01:10 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2015-10-08 18:08:38 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2015-10-09 07:08:54 UTC
(In reply to Kyle M Hall from comment #6)
> Created attachment 43277 [details] [review] [review]
> Bug 14915 [QA Followup] - Tweak '+' icon for serials-home

There are certainly some others I have not found.
I also tested for a few minutes.

I think we need either another signoff (the tester must test a lot of pages and compare them with master) or wait for the beginning of the next release cycle.
Comment 8 Tomás Cohen Arazi 2015-10-21 13:11:20 UTC
(In reply to Jonathan Druart from comment #7)
> (In reply to Kyle M Hall from comment #6)
> > Created attachment 43277 [details] [review] [review] [review]
> > Bug 14915 [QA Followup] - Tweak '+' icon for serials-home
> 
> There are certainly some others I have not found.
> I also tested for a few minutes.
> 
> I think we need either another signoff (the tester must test a lot of pages
> and compare them with master) or wait for the beginning of the next release
> cycle.

Jonathan, you are so lazy :-P
Comment 9 Jonathan Druart 2015-10-21 13:16:44 UTC
(In reply to Tomás Cohen Arazi from comment #8)
> Jonathan, you are so lazy :-P

Indeed, I am a developer, I have to be lazy!
Comment 10 Tomás Cohen Arazi 2015-10-21 13:29:11 UTC
(In reply to Jonathan Druart from comment #9)
> (In reply to Tomás Cohen Arazi from comment #8)
> > Jonathan, you are so lazy :-P
> 
> Indeed, I am a developer, I have to be lazy!

Feature, not a bug!
Comment 11 Josef Moravec 2015-10-23 06:40:40 UTC
There are still some Glyphicons, probably from recent development in these files:

koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-edit-batch.tt
koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-manage.tt
koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/edit-batch.tt
koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/image-manage.tt
koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/manage.tt

koha-tmpl/intranet-tmpl/prog/en/includes/labels-toolbar.inc
koha-tmpl/intranet-tmpl/prog/en/includes/patroncards-toolbar.inc

Maybe we should update Coding Guidelines, what do you think?
Comment 12 Katrin Fischer 2015-10-25 22:17:50 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2015-10-25 22:17:55 UTC
Created attachment 43960 [details] [review]
[PASSED QA] Bug 14915 - Use Font Awesome instead of Glyphicons for the staff intranet

We should be using Font Awesome for our icons instead of Glyphicons, for
the reasons discussed on bug 13696.

Test Plan:
1) Apply this patch
2) Note all Glyphicons have been replaced with FA icons in the staff intranet
3) git grep "icon-" ./koha-tmpl/intranet-tmpl/prog/en/modules/
   should give no results
4) git grep "icon-" ./koha-tmpl/intranet-tmpl/prog/en/includes/
   should give no results

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
We need a follow-up to cover the files changes since this
patch was written. Especially to cover the changes in the
label creator modules.
Comment 14 Katrin Fischer 2015-10-25 22:17:58 UTC
Created attachment 43961 [details] [review]
[PASSED QA] Bug 14915 [QA Followup] - Tweak '+' icon for serials-home

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 15 Katrin Fischer 2015-10-25 22:18:02 UTC
Created attachment 43962 [details] [review]
Bug 14915: QA Follow-up - Replace fa-remove-sign with fa-times-circle

Noticed that an icon was missing in the templates.

Replaced fa-remove-sign with same-looking fa-times-circle.
Comment 16 Katrin Fischer 2015-10-25 22:18:05 UTC
Created attachment 43963 [details] [review]
Bug 14915: QA Follow-up - Replace fa-hdd with fa-save

Alternative patch to using hdd-o.

As the Font Awesome Icon is different to what we used before,
an alternative could be switching back to the 'classic' symbol
for save.
Comment 17 Katrin Fischer 2015-10-25 22:18:08 UTC
Created attachment 43964 [details] [review]
Bug 14915: QA Follow-up - Replace fa-downlowd-alt with fa-download

Fixes non-existing fa-download-alt by using fa-download instead.
Comment 18 Katrin Fischer 2015-10-25 22:18:11 UTC
Created attachment 43965 [details] [review]
Bug 14915: QA Follow-up - Replace fa-zoom-in and fa-zoom-out

Same icons don't appear to exist for Font awesome.
Replaced with fa-folder and fa-folder-open for now.
Comment 19 Katrin Fischer 2015-10-25 22:18:14 UTC
Created attachment 43966 [details] [review]
Bug 14915: QA Folloq-up - Replace fa-merge with fa-compress

Another icon that doesn't quite exist in Font Awesome -
this is starting to get a bit troublesome.
Comment 20 Katrin Fischer 2015-10-25 22:18:17 UTC
Created attachment 43967 [details] [review]
Bug 14915: [FA 4.4] QA Follow-up - Replace fa-hold with fa-sticky-note-o

Ok, this is a fun one: There is no icon like our current hold icon
in Font Awesome. So I picked fa-sticky-note-o for a replacement.

But since this is a new one, it doesn't exist in Koha yet.
I found no better match and the patch was already written...
Comment 21 Katrin Fischer 2015-10-25 22:18:20 UTC
Created attachment 43968 [details] [review]
Bug 14915: QA Follow-up - Replace fa-time with fa-clock-o

Icon looks almost the same, just has a different name.
Comment 22 Katrin Fischer 2015-10-25 22:28:07 UTC
Some comments about this:

- It looks like the patch assumed that the file names from Glyphicons and Font Awesome would match. They do not. This led to a bit of silliness in QA follow-ups... (7).

- Not always there is an equivalent icon available in Font Awesome. I have tried to pick an icon close to our original one or come up with a good alternative suggestion - the ones that changed most would be (Glyphicon name): hdd, zoom-in, zoom-out, hold.

- Especially hold is a hard one - the best I could come up with was sticky-note-o. But then I learned that this was only added in Font Awesome 4.4, while bug bug 13696 adds 4.3. The patch is marked - the result is the same with and without the patch currently - no icon shown.

- The git grep ... statements given in the first patch now come back 'non-empty' - as we have added more icons with some of the new features being pushed. Most of the files seem to belong to the label creators. It would be good to follow-up on those and others before release.

I like that the Font Awesome Icons look nicer - less pixelated than our current ones, especially if you zoom in. But there are some catches like that we have to replace some of the icons. Leaving to RM for final decision and review/pushing back to QA :)

My recommendation would be to update to 4.4 (makes sense to start with the newest set) and to do follow-ups once the new features have been pushed to fix the remaining or newly introduced icons.
Comment 23 Kyle M Hall 2015-10-26 16:54:26 UTC
I've added a followup to Bug 13696 to upgrade FA to 4.4.0!
Comment 24 Kyle M Hall 2015-10-26 17:16:53 UTC
Created attachment 44014 [details] [review]
Bug 14915 [QA Followup] - Switch recent commits from Glyphicons to Font Awesome
Comment 25 Tomás Cohen Arazi 2015-10-27 13:44:32 UTC
Patches pushed to master.

Thanks Kyle and Katrina!
Comment 26 Jonathan Druart 2015-11-13 14:17:47 UTC
Created attachment 44841 [details] [review]
Bug 14915: (follow-up) [FA 4.4] (QA followup) Replace fa-hold with fa-sticky-note-o

Reported on bug 15186.

Test plan:
Do a search on the staff interface
You will get an error in the console and the image + checkboxes are not
display.

With this patch it will be fixed.