Bug 32109 - Toolbar containing text links lacks spacing
Summary: Toolbar containing text links lacks spacing
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 30952
Blocks:
  Show dependency treegraph
 
Reported: 2022-11-04 11:41 UTC by Owen Leonard
Modified: 2023-06-08 22:28 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00


Attachments
Screenshot showing the crowded links (14.59 KB, image/png)
2022-11-04 11:41 UTC, Owen Leonard
Details
Bug 32109: Fix spacing in toolbars containing text links (7.59 KB, patch)
2022-11-04 12:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32109: Fix spacing in toolbars containing text links (7.66 KB, patch)
2022-11-07 15:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32109: Fix spacing in toolbars containing text links (10.33 KB, patch)
2022-11-14 15:22 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32109: Fix spacing in toolbars containing text links (10.38 KB, patch)
2022-11-14 18:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32109: Fix spacing in toolbars containing text links (10.45 KB, patch)
2022-11-15 07:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32109: Fix spacing in toolbars containing text links (10.56 KB, patch)
2022-11-16 14:24 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2022-11-04 11:41:37 UTC
Created attachment 143177 [details]
Screenshot showing the crowded links

In the redesign, <div id="toolbar"> has "display:flex," which works well for Bootstrap-styled buttons but creates a problem if the toolbar has only text links.
Comment 1 Owen Leonard 2022-11-04 12:00:06 UTC
Created attachment 143178 [details] [review]
Bug 32109: Fix spacing in toolbars containing text links

This patch adds a new class to toolbars in certain contexts: Where the
toolbar contains only text links (as opposed to Bootstrap-styled
controls). This lets us set some comfortable padding on the links.

To test, apply the patch and rebuild the staff interface CSS.

Test the following pages to confirm that the toolbar of selection links
("Select all," "Clear all," etc.) has legible spacing.

- Cataloging -> Batch item modification -> List of submitted barcodes
- Cataloging -> Batch item deletion -> List of submitted barcodes
- Cataloging -> Batch record modification -> List of submitted records
- Cataloging -> Batch record deletion -> List of submitted records
- Tools -> Batch extend due dates -> Preview results
- Tools -> Batch patron modification -> List of submitted patrons
Comment 2 Martin Renvoize 2022-11-07 15:34:55 UTC
I see.. this is another place we're a little inconsistent with how we mark up a 'toolbar'.

On 'Opac problem reports' and 'checkout notes' there are similar "toolbars" for filtering the datatable... they have the following marking

<fieldset class="action" style="cursor:pointer;">

Should we try to synchronize the markup of those two 'filter bars' too here?

Challenge with this is that on those two pages there are also already <div id="toolbar"> blocks on the page too.

I feel like an id of toolbar is wrong.. it should likely be a class instead.?
Comment 3 Martin Renvoize 2022-11-07 15:55:47 UTC
Created attachment 143374 [details] [review]
Bug 32109: Fix spacing in toolbars containing text links

This patch adds a new class to toolbars in certain contexts: Where the
toolbar contains only text links (as opposed to Bootstrap-styled
controls). This lets us set some comfortable padding on the links.

To test, apply the patch and rebuild the staff interface CSS.

Test the following pages to confirm that the toolbar of selection links
("Select all," "Clear all," etc.) has legible spacing.

- Cataloging -> Batch item modification -> List of submitted barcodes
- Cataloging -> Batch item deletion -> List of submitted barcodes
- Cataloging -> Batch record modification -> List of submitted records
- Cataloging -> Batch record deletion -> List of submitted records
- Tools -> Batch extend due dates -> Preview results
- Tools -> Batch patron modification -> List of submitted patrons

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2022-11-07 16:03:03 UTC
Signing off.. happy to see a follow-up here to make the other two cases I identified match the markup.. or we could submit another bug for that.. I'll defer that decision to the RM/QA
Comment 5 Owen Leonard 2022-11-14 15:22:58 UTC
Created attachment 143837 [details] [review]
Bug 32109: Fix spacing in toolbars containing text links

This patch adds a new class to toolbars in certain contexts: Where the
toolbar contains only text links (as opposed to Bootstrap-styled
controls). This lets us set some comfortable padding on the links.

To test, apply the patch and rebuild the staff interface CSS.

Test the following pages to confirm that the toolbar of selection links
("Select all," "Clear all," etc.) has legible spacing.

- Cataloging -> Batch item modification -> List of submitted barcodes
- Cataloging -> Batch item deletion -> List of submitted barcodes
- Cataloging -> Batch record modification -> List of submitted records
- Cataloging -> Batch record deletion -> List of submitted records
- Circulation -> Checkout notes
- Tools -> Batch extend due dates -> Preview results
- Tools -> Batch patron modification -> List of submitted patrons
- Tools -> OPAC problem reports
Comment 6 Owen Leonard 2022-11-14 15:29:18 UTC
I reworked this so that all instances of the "selections-toolbar" class also have the "btn-toolbar" class, and the "toolbar" ID is removed. The intention of this is to reserve the "toolbar" ID for the "main" toolbar on the page, the one containing the action buttons.
Comment 7 Katrin Fischer 2022-11-14 18:27:13 UTC
Created attachment 143858 [details] [review]
Bug 32109: Fix spacing in toolbars containing text links

This patch adds a new class to toolbars in certain contexts: Where the
toolbar contains only text links (as opposed to Bootstrap-styled
controls). This lets us set some comfortable padding on the links.

To test, apply the patch and rebuild the staff interface CSS.

Test the following pages to confirm that the toolbar of selection links
("Select all," "Clear all," etc.) has legible spacing.

- Cataloging -> Batch item modification -> List of submitted barcodes
- Cataloging -> Batch item deletion -> List of submitted barcodes
- Cataloging -> Batch record modification -> List of submitted records
- Cataloging -> Batch record deletion -> List of submitted records
- Circulation -> Checkout notes
- Tools -> Batch extend due dates -> Preview results
- Tools -> Batch patron modification -> List of submitted patrons
- Tools -> OPAC problem reports

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2022-11-14 18:27:59 UTC
Great fix, thx!
Comment 9 Martin Renvoize 2022-11-15 07:10:27 UTC
Created attachment 143890 [details] [review]
Bug 32109: Fix spacing in toolbars containing text links

This patch adds a new class to toolbars in certain contexts: Where the
toolbar contains only text links (as opposed to Bootstrap-styled
controls). This lets us set some comfortable padding on the links.

To test, apply the patch and rebuild the staff interface CSS.

Test the following pages to confirm that the toolbar of selection links
("Select all," "Clear all," etc.) has legible spacing.

- Cataloging -> Batch item modification -> List of submitted barcodes
- Cataloging -> Batch item deletion -> List of submitted barcodes
- Cataloging -> Batch record modification -> List of submitted records
- Cataloging -> Batch record deletion -> List of submitted records
- Circulation -> Checkout notes
- Tools -> Batch extend due dates -> Preview results
- Tools -> Batch patron modification -> List of submitted patrons
- Tools -> OPAC problem reports

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2022-11-15 07:10:59 UTC
Thanks Owen :)
Comment 11 Owen Leonard 2022-11-16 14:24:51 UTC
Created attachment 143957 [details] [review]
Bug 32109: Fix spacing in toolbars containing text links

This patch adds a new class to toolbars in certain contexts: Where the
toolbar contains only text links (as opposed to Bootstrap-styled
controls). This lets us set some comfortable padding on the links.

To test, apply the patch and rebuild the staff interface CSS.

Test the following pages to confirm that the toolbar of selection links
("Select all," "Clear all," etc.) has legible spacing.

- Cataloging -> Batch item modification -> List of submitted barcodes
- Cataloging -> Batch item deletion -> List of submitted barcodes
- Cataloging -> Batch record modification -> List of submitted records
- Cataloging -> Batch record deletion -> List of submitted records
- Circulation -> Checkout notes
- Tools -> Batch extend due dates -> Preview results
- Tools -> Batch patron modification -> List of submitted patrons
- Tools -> OPAC problem reports

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Tomás Cohen Arazi 2022-11-16 18:56:32 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!