|
Description
Nick Clemens (kidclamp)
2021-02-08 12:48:41 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 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> 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> 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. Owen, do you have an idea, how we could set this individuallyfor those 2 tables (acq home page and funds table) or handle otherwise? 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 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 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 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 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. Created attachment 116690 [details]
Display problem
(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. 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> Thanks Katrin. I re-retested and everything worked great. 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> Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.03 Missing dependency for 20.05, not backporting. If this is needed, please rebase bug 26602, thereby allowing backport of bug 27646. Pushed to 20.05.x for 20.05.09 Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. |