Bug 27653 - Do not include 'caption' row in print/copy export of datatables
Summary: Do not include 'caption' row in print/copy export of datatables
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 27646
Blocks:
  Show dependency treegraph
 
Reported: 2021-02-08 12:48 UTC by Nick Clemens
Modified: 2021-02-23 17:27 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.03,20.05.09


Attachments
Bug 27653: Set messageTop to null for KohaTables (1.62 KB, patch)
2021-02-08 12:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27653: Set messageTop to null for KohaTables (1.67 KB, patch)
2021-02-08 22:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27653: Set messageTop to null for KohaTables (1.74 KB, patch)
2021-02-08 23:44 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 27653: [Alternate] Use toolbar elements instead of captions (2.30 KB, patch)
2021-02-09 14:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27653: [Alternate] Use toolbar elements instead of captions (2.32 KB, patch)
2021-02-09 18:40 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27653: [Alternate] Use toolbar elements instead of captions (4.27 KB, patch)
2021-02-09 19:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 27653: [Alternate][Alternate][Alternate] Use div instead of caption for controls (3.94 KB, patch)
2021-02-09 19:42 UTC, Owen Leonard
Details | Diff | Splinter Review
Display problem (31.89 KB, image/png)
2021-02-10 23:33 UTC, Barbara Johnson
Details
Bug 27653: [Alternate][Alternate][Alternate] Use div instead of caption for controls (4.00 KB, patch)
2021-02-12 17:56 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 27653: [Alternate][Alternate][Alternate] Use div instead of caption for controls (4.05 KB, patch)
2021-02-14 15:06 UTC, Katrin Fischer
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-02-08 12:48:41 UTC
The 'caption' field is used a a few dataTables as a place to store filters etc.

We do not wish this to be included when copying/printing the tables
Comment 1 Nick Clemens 2021-02-08 12:52:03 UTC
Created attachment 116495 [details] [review]
Bug 27653: Set messageTop to null for KohaTables

This patch sets messageTop to null for KohaTables. This prevents
the filter row from being included in an export.

To test:
1 - Browse to acquisitions home
2 - Click Export->Copy
3 - Paste the data to a text editor
4 - Click Export->Print
5 - Note the 'Expand all...' row is included in both of the above
6 - Apply patch
7 - Repeat
8 - The row is gone from both copy and print
Comment 2 Katrin Fischer 2021-02-08 22:05:55 UTC
Created attachment 116530 [details] [review]
Bug 27653: Set messageTop to null for KohaTables

This patch sets messageTop to null for KohaTables. This prevents
the filter row from being included in an export.

To test:
1 - Browse to acquisitions home
2 - Click Export->Copy
3 - Paste the data to a text editor
4 - Click Export->Print
5 - Note the 'Expand all...' row is included in both of the above
6 - Apply patch
7 - Repeat
8 - The row is gone from both copy and print

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 3 PTFS Europe Sandboxes 2021-02-08 23:44:16 UTC
Created attachment 116531 [details] [review]
Bug 27653: Set messageTop to null for KohaTables

This patch sets messageTop to null for KohaTables. This prevents
the filter row from being included in an export.

To test:
1 - Browse to acquisitions home
2 - Click Export->Copy
3 - Paste the data to a text editor
4 - Click Export->Print
5 - Note the 'Expand all...' row is included in both of the above
6 - Apply patch
7 - Repeat
8 - The row is gone from both copy and print

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Comment 4 Owen Leonard 2021-02-08 23:58:17 UTC
I'm a little concerned about this change because the example in the test plan isn't really good HTML. We shouldn't put controls like "expand all" links into a <caption> tag because it's not semantically correct.

Ideally the caption is designed to provide information about the contents of the table, so a good caption wouldn't be unwanted in a printout. We don't use <caption> very consistently, so I don't really know how often we have a situation where this patch would affect the output. But I am concerned about making this a global change.
Comment 5 Katrin Fischer 2021-02-09 10:58:03 UTC
Owen, do you have an idea, how we could set this individuallyfor those 2 tables (acq home page and funds table) or handle otherwise?
Comment 6 Nick Clemens 2021-02-09 14:41:46 UTC
Created attachment 116576 [details] [review]
Bug 27653: [Alternate] Use toolbar elements instead of captions

The aqbudgets and aqui-home tables use captions for additional table controls - these are
more appropriate as a toolbar

To test:
1 - View the tables on acquisitions home and budgets views
2 - Note the 'Expand all...' control row
3 - Apply patch
4 - The row remains, but is above table rows
5 - The row is not included when printing or copying the table
Comment 7 Nick Clemens 2021-02-09 18:40:55 UTC
Created attachment 116611 [details] [review]
Bug 27653: [Alternate] Use toolbar elements instead of captions

The aqbudgets and aqui-home tables use captions for additional table controls - these are
more appropriate as a DataTables custom toolbar

To test:
1 - View the tables on acquisitions home and budgets views
2 - Note the 'Expand all...' control row
3 - Apply patch
4 - The row remains, but is below the table controls
5 - The row is not included when printing or copying the table
Comment 8 Nick Clemens 2021-02-09 19:12:26 UTC
Created attachment 116614 [details] [review]
Bug 27653: [Alternate] Use toolbar elements instead of captions

The aqbudgets and aqui-home tables use captions for additional table controls - these are
more appropriate as a DataTables custom toolbar

To test:
1 - View the tables on acquisitions home and budgets views
2 - Note the 'Expand all...' control row
3 - Apply patch
4 - The row remains, but is below the table controls
5 - The row is not included when printing or copying the table
Comment 9 Owen Leonard 2021-02-09 19:42:47 UTC
Created attachment 116615 [details] [review]
Bug 27653: [Alternate][Alternate][Alternate] Use div instead of caption for controls

The funds and acquisitions home page tables use captions for additional
table controls. These are more appropriate as "toolbar" divs.

This patch converts the <caption> to <div class="toolbar btn-toolbar">
and moves it just above <table> in the markup to keep it valid.

To test:
1 - View the tables on acquisitions home and budgets views
2 - Note the 'Expand all...' control row
3 - Apply patch
4 - The row remains, but is below the table controls
5 - The row is not included when printing or copying the table
Comment 10 Barbara Johnson 2021-02-10 23:32:49 UTC
The row is not included when printing or copying the table but there's a display problem.  The 'Expand all' control row is duplicated.  Also there is extraneous data such as <<<<<<< HEAD ======= and >>>>>>> Bug 27653: [Alternate][Alternate][Alternate] Use div instead of caption for controls.  I attached a screenshot of the display I got.
Comment 11 Barbara Johnson 2021-02-10 23:33:29 UTC
Created attachment 116690 [details]
Display problem
Comment 12 Katrin Fischer 2021-02-10 23:39:00 UTC
(In reply to Barbara Johnson from comment #11)
> Created attachment 116690 [details]
> Display problem

Hi Barbara, these are called "merge markers" - they sometimes are visible when a patch hasn't applied correctly. I will obsolete the other alternates, maybe that will help.
Comment 13 ByWater Sandboxes 2021-02-12 17:56:29 UTC
Created attachment 116841 [details] [review]
Bug 27653: [Alternate][Alternate][Alternate] Use div instead of caption for controls

The funds and acquisitions home page tables use captions for additional
table controls. These are more appropriate as "toolbar" divs.

This patch converts the <caption> to <div class="toolbar btn-toolbar">
and moves it just above <table> in the markup to keep it valid.

To test:
1 - View the tables on acquisitions home and budgets views
2 - Note the 'Expand all...' control row
3 - Apply patch
4 - The row remains, but is below the table controls
5 - The row is not included when printing or copying the table

Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Comment 14 Barbara Johnson 2021-02-12 17:57:29 UTC
Thanks Katrin.  I re-retested and everything worked great.
Comment 15 Katrin Fischer 2021-02-14 15:06:18 UTC
Created attachment 116864 [details] [review]
Bug 27653: [Alternate][Alternate][Alternate] Use div instead of caption for controls

The funds and acquisitions home page tables use captions for additional
table controls. These are more appropriate as "toolbar" divs.

This patch converts the <caption> to <div class="toolbar btn-toolbar">
and moves it just above <table> in the markup to keep it valid.

To test:
1 - View the tables on acquisitions home and budgets views
2 - Note the 'Expand all...' control row
3 - Apply patch
4 - The row remains, but is below the table controls
5 - The row is not included when printing or copying the table

Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Jonathan Druart 2021-02-15 11:08:43 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 17 Fridolin Somers 2021-02-19 15:27:01 UTC
Pushed to 20.11.x for 20.11.03
Comment 18 Andrew Fuerste-Henry 2021-02-23 12:56:42 UTC
Missing dependency for 20.05, not backporting. If this is needed, please rebase bug 26602, thereby allowing backport of bug 27646.
Comment 19 Andrew Fuerste-Henry 2021-02-23 17:08:17 UTC
Pushed to 20.05.x for 20.05.09
Comment 20 Victor Grousset/tuxayo 2021-02-23 17:27:09 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.