Bug 29057 - Use font awesome icons on request.pl
Summary: Use font awesome icons on request.pl
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-20 10:22 UTC by Nick Clemens
Modified: 2022-05-13 10:46 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
This enhancement updates the arrows used on the holds page for a record to change the hold priority. Instead of using images for the arrows, Font Awesome icons are now used. In addition, you can now override the icons using any Font Awesome icons in the IntranetUserCSS system preference. For an example, see https://gitlab.com/-/snippets/2319364
Version(s) released in:


Attachments
Bug 29057: Use font awesome icons on request.pl (5.75 KB, patch)
2021-09-20 10:24 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29057: Use font awesome icons on request.pl (5.64 KB, patch)
2021-09-20 13:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29057: [alternate] Use font awesome icons on request.pl (10.27 KB, patch)
2022-01-13 15:31 UTC, Owen Leonard
Details | Diff | Splinter Review
before pic (106.07 KB, image/png)
2022-03-14 11:57 UTC, Nick Clemens
Details
nick patch (74.20 KB, image/png)
2022-03-14 11:57 UTC, Nick Clemens
Details
owen patch (73.63 KB, image/png)
2022-03-14 11:57 UTC, Nick Clemens
Details
Bug 29057: (follow-up) Make arrow border hover state the correct color (933 bytes, patch)
2022-03-18 10:56 UTC, Owen Leonard
Details | Diff | Splinter Review
Screenshot of interface using angle icons (69.49 KB, image/png)
2022-03-18 11:08 UTC, Owen Leonard
Details
Bug 29057: [alternate] Use font awesome icons on request.pl (11.37 KB, patch)
2022-05-11 14:47 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 29057: [alternate] Use font awesome icons on request.pl (11.42 KB, patch)
2022-05-13 01:57 UTC, David Nind
Details | Diff | Splinter Review
Bug 29057: [alternate] Use font awesome icons on request.pl (11.48 KB, patch)
2022-05-13 10:46 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-09-20 10:22:44 UTC
We have a few images we use here, they could be replaced with font-awesome icons

It looks like the 'top'/'bottom' icons require a pro license, but the filled circle arrows seem a close equivalent
Comment 1 Nick Clemens 2021-09-20 10:24:34 UTC
Created attachment 125049 [details] [review]
Bug 29057: Use font awesome icons on request.pl

To test:
1 - Place some holds on a record
2 - View the 'Holds' tab in the staff interface
3 - Note the new icons
4 - Note their hover test
5 - Confirm the buttons still work as expected
Comment 2 Owen Leonard 2021-09-20 12:15:02 UTC
Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 29057: Use font awesome icons on request.pl
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc).
error: could not build fake ancestor
Patch failed at 0001 Bug 29057: Use font awesome icons on request.pl
Comment 3 Nick Clemens 2021-09-20 13:14:59 UTC
Created attachment 125061 [details] [review]
Bug 29057: Use font awesome icons on request.pl

To test:
1 - Place some holds on a record
2 - View the 'Holds' tab in the staff interface
3 - Note the new icons
4 - Note their hover test
5 - Confirm the buttons still work as expected
Comment 4 David Nind 2021-10-05 10:20:42 UTC
Patch no longer applies 8-(..

Applying: Bug 29057: Use font awesome icons on request.pl
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
error: Failed to merge in the changes.
Patch failed at 0001 Bug 29057: Use font awesome icons on request.pl
Comment 5 Owen Leonard 2022-01-13 15:31:08 UTC
Created attachment 129413 [details] [review]
Bug 29057: [alternate] Use font awesome icons on request.pl

This patch offers an alternate take on using Font Awesome icons on the
holds page in place of images as links.

To test:
1 - Place some holds on a record
2 - View the 'Holds' tab in the staff interface
3 - Note the new icons
4 - Note their hover test
5 - Confirm the buttons still work as expected
Comment 6 Lucas Gass 2022-01-13 21:18:54 UTC
I like Owen's patch. The only thing I would say is that the use of the using the 'fa-rotate-90 fa-fast-foward' icon in the 'Toggle set to lowest priority' <th> column looks awkward. Is it nessesary to have it in the <th>?
Comment 7 Fridolin Somers 2022-03-10 08:15:02 UTC
We should take screen shots to present the 2 solutions
Comment 8 Fridolin Somers 2022-03-10 08:16:04 UTC
And check if we can remove files img/go-xxx.png
Comment 9 Shi Yao Wang 2022-03-10 16:11:02 UTC
Patch doesn't apply.


CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc
error: Failed to merge in the changes.
Patch failed at 0001 Bug 29057: Use font awesome icons on request.pl
Comment 10 Nick Clemens 2022-03-14 11:57:11 UTC
Created attachment 131650 [details]
before pic
Comment 11 Nick Clemens 2022-03-14 11:57:31 UTC
Created attachment 131651 [details]
nick patch
Comment 12 Nick Clemens 2022-03-14 11:57:48 UTC
Created attachment 131652 [details]
owen patch
Comment 13 Nick Clemens 2022-03-14 11:58:47 UTC
Added pics of before and after with both patches

Obsoleting mine in favor of Owen's
Comment 14 Fridolin Somers 2022-03-18 07:17:04 UTC
I'm testing.

Looks good but I see some issues :

When hovering button "Move hold to top/bottom" we see that horizontal line color does not change. This is because it is not part of the icon, it is a CSS border.
Looks dangerous to me.

Why is icon for "Toggle lower priority" different from "Move hold to bottom" ?

I propose we use "angle" FA icons :
https://fontawesome.com/v4/icon/angle-down
https://fontawesome.com/v4/icon/angle-double-down
Single arrow for "Move hold up/down" and double arrow for "Move hold to top/bottom".
Comment 15 Owen Leonard 2022-03-18 10:56:52 UTC
Created attachment 131877 [details] [review]
Bug 29057: (follow-up) Make arrow border hover state the correct color
Comment 16 Owen Leonard 2022-03-18 11:08:17 UTC
Created attachment 131878 [details]
Screenshot of interface using angle icons

(In reply to Fridolin Somers from comment #14)

> Why is icon for "Toggle lower priority" different from "Move hold to bottom"
> ?

I think it should be different because it performs a different function. I think the current version with the arrow images is confusing because they're not different enough.

The angle icons are not my top choice, but it is nice that there could be three variations on the same one.
Comment 17 Katrin Fischer 2022-03-20 16:02:05 UTC
I think I like the first version from Owen the best so far, but we do have an issue with the fifth icon (and kinda always had).

I think on another bug we had discussed using something like a 'pin' to indicate that it pins the hold to the bottom. Could we do some combination of 'move to bottom' with a red pin in it or so?

https://fontawesome.com/v3/icon/pushpin

I think we need something similar but yet different (which makes it quite hard).
Comment 18 Fridolin Somers 2022-03-21 21:49:26 UTC
> I think it should be different because it performs a different function.
Ah OK I was not aware of that :)

Tooltip text is a good first help.
But a good icon would be nice.

Maybe use "angle" icons to move up/down/top/bottom :
https://fontawesome.com/v4/icon/angle-down
https://fontawesome.com/v4/icon/angle-double-down

And a "angle double down" with underline has 5th icon (toggle lowest priority).
Meaning go to bottom and stick there.
Comment 19 Aleisha Amohia 2022-04-21 04:42:05 UTC
It sounds like this is not yet ready for testing and awaiting a new patch? Adjusting bug status - feel free to change back if I'm wrong
Comment 20 Owen Leonard 2022-05-10 14:19:24 UTC
If anyone wants to leave their vote on which of the options screenshotted in the attachments looks best to them maybe this could move forward.
Comment 21 David Nind 2022-05-10 21:44:43 UTC
I like the one called "owen patch" the best, but they all nice improvements.
Comment 22 Owen Leonard 2022-05-11 14:47:52 UTC
Created attachment 134896 [details] [review]
Bug 29057: [alternate] Use font awesome icons on request.pl

This patch offers an alternate take on using Font Awesome icons on the
holds page in place of images as links.

This revised patch updates the icon implementation so that it is easier
to override the Font Awesome icon selection with CSS. Test by adding the
contents of this file to the IntranetUserCSS preference:

https://gitlab.com/-/snippets/2319364

To test:
1 - Place some holds on a record
2 - View the 'Holds' tab in the staff interface
3 - Note the new icons
4 - Note their hover test
5 - Confirm the buttons still work as expected
Comment 23 Owen Leonard 2022-05-11 15:22:04 UTC
the possibilities are endless https://snipboard.io/UCbkzr.jpg
Comment 24 David Nind 2022-05-13 01:57:20 UTC
Created attachment 134942 [details] [review]
Bug 29057: [alternate] Use font awesome icons on request.pl

This patch offers an alternate take on using Font Awesome icons on the
holds page in place of images as links.

This revised patch updates the icon implementation so that it is easier
to override the Font Awesome icon selection with CSS. Test by adding the
contents of this file to the IntranetUserCSS preference:

https://gitlab.com/-/snippets/2319364

To test:
1 - Place some holds on a record
2 - View the 'Holds' tab in the staff interface
3 - Note the new icons
4 - Note their hover test
5 - Confirm the buttons still work as expected

Signed-off-by: David Nind <david@davidnind.com>
Comment 25 David Nind 2022-05-13 02:07:15 UTC
(In reply to Owen Leonard from comment #23)
> the possibilities are endless https://snipboard.io/UCbkzr.jpg

Nice!
Comment 26 Fridolin Somers 2022-05-13 02:32:53 UTC
I agree with David choice, lets go this that 👊
Comment 27 Nick Clemens 2022-05-13 10:46:01 UTC
Created attachment 134966 [details] [review]
Bug 29057: [alternate] Use font awesome icons on request.pl

This patch offers an alternate take on using Font Awesome icons on the
holds page in place of images as links.

This revised patch updates the icon implementation so that it is easier
to override the Font Awesome icon selection with CSS. Test by adding the
contents of this file to the IntranetUserCSS preference:

https://gitlab.com/-/snippets/2319364

To test:
1 - Place some holds on a record
2 - View the 'Holds' tab in the staff interface
3 - Note the new icons
4 - Note their hover test
5 - Confirm the buttons still work as expected

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>