We should be using Font Awesome for our icons instead of Glyphicons, for the reasons discussed on bug 13696.
Created attachment 42925 [details] [review] 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
Created attachment 42929 [details] [review] 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
Created attachment 42943 [details] [review] [Signed-off] 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>
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.
Created attachment 43275 [details] [review] 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>
Created attachment 43277 [details] [review] Bug 14915 [QA Followup] - Tweak '+' icon for serials-home
(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.
(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
(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!
(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!
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?
Created attachment 43959 [details] [review] [PASSED QA] Bug 13696 [QA Followup] - Add Font Awesome license to the about page Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
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.
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>
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.
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.
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.
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.
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.
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...
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.
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.
I've added a followup to Bug 13696 to upgrade FA to 4.4.0!
Created attachment 44014 [details] [review] Bug 14915 [QA Followup] - Switch recent commits from Glyphicons to Font Awesome
Patches pushed to master. Thanks Kyle and Katrina!
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.