Bug 21409 - Add column configuration to course reserves
Summary: Add column configuration to course reserves
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 19282
  Show dependency treegraph
 
Reported: 2018-09-25 18:39 UTC by Owen Leonard
Modified: 2020-01-06 20:14 UTC (History)
4 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:


Attachments
Bug 21409: Add column configuration to course reserves (7.38 KB, patch)
2018-09-25 18:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21409: Add column configuration to course reserves (7.45 KB, patch)
2018-10-01 15:24 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 21409: Add column configuration to course reserves (7.55 KB, patch)
2018-10-02 12:24 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2018-09-25 18:39:39 UTC
Add column configuration and export options the table of course reserves and to the table of titles reserved for a course.
Comment 1 Owen Leonard 2018-09-25 18:52:21 UTC
Created attachment 79383 [details] [review]
Bug 21409: Add column configuration to course reserves

This patch adds columns configuration and print/export options to two
tables: The table listing courses and the table listing titles reserved
for a course.

To test you should have more than one course and multiple title reserved
for at least one course.

- Apply the patch and go to Course reserves
  - In the table of courses, confirm that all DataTables controls work
    correctly: paging, filter, column visibility, export, etc.
  - View a course with multiple titles reserved. Test all DataTables
    controls on this table too.
- Go to Administration -> Columns settings
  - Change the settings for the courses and reserves table and confirm
    that these changes are applied correctly.
Comment 2 Caroline Cyr La Rose 2018-10-01 15:24:52 UTC
Created attachment 79737 [details] [review]
Bug 21409: Add column configuration to course reserves

This patch adds columns configuration and print/export options to two
tables: The table listing courses and the table listing titles reserved
for a course.

To test you should have more than one course and multiple title reserved
for at least one course.

- Apply the patch and go to Course reserves
  - In the table of courses, confirm that all DataTables controls work
    correctly: paging, filter, column visibility, export, etc.
  - View a course with multiple titles reserved. Test all DataTables
    controls on this table too.
- Go to Administration -> Columns settings
  - Change the settings for the courses and reserves table and confirm
    that these changes are applied correctly.

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 3 Jonathan Druart 2018-10-01 15:50:05 UTC
I have a small concern: we remove the table's caption.
We should use it instead of moving it to a h1 (and it's too late for a h1 here, it must be h2 or h3).

I tried to move the caption on top of the DT paging/filtering/buttons block, but failed. It seems that it's not feasible.
Maybe we should restyle the caption to reduce the space between the "pager" and the table (I guess it's why you move it out).
Comment 4 Jonathan Druart 2018-10-01 15:54:14 UTC
Caption - https://snag.gy/mikSJI.jpg
h1- https://snag.gy/sLIK3u.jpg
Caption adjusted (text-align: center; font-size: inherit; margin: inherit;) - https://snag.gy/GYahyl.jpg
Comment 5 Owen Leonard 2018-10-01 16:58:37 UTC
(In reply to Jonathan Druart from comment #3)
> I have a small concern: we remove the table's caption.

We have neither a coding guideline regarding captions nor one defining how to use heading tags, so the templates are pretty random with regard to these elements. I think most if not all tables with DataTables toolbars use a heading instead of a caption.
Comment 6 Jonathan Druart 2018-10-02 12:24:26 UTC
Created attachment 79820 [details] [review]
Bug 21409: Add column configuration to course reserves

This patch adds columns configuration and print/export options to two
tables: The table listing courses and the table listing titles reserved
for a course.

To test you should have more than one course and multiple title reserved
for at least one course.

- Apply the patch and go to Course reserves
  - In the table of courses, confirm that all DataTables controls work
    correctly: paging, filter, column visibility, export, etc.
  - View a course with multiple titles reserved. Test all DataTables
    controls on this table too.
- Go to Administration -> Columns settings
  - Change the settings for the courses and reserves table and confirm
    that these changes are applied correctly.

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2018-10-02 12:28:48 UTC
(In reply to Owen Leonard from comment #5)
> (In reply to Jonathan Druart from comment #3)
> > I have a small concern: we remove the table's caption.
> 
> We have neither a coding guideline regarding captions nor one defining how
> to use heading tags, so the templates are pretty random with regard to these
> elements. I think most if not all tables with DataTables toolbars use a
> heading instead of a caption.

https://www.w3.org/TR/WCAG20-TECHS/H39.html

"""
The objective of this technique is to programmatically associate captions for data tables where captions are provided in the presentation. The caption for a table is a table identifier and acts like a title or heading for the table.

The caption element is the appropriate markup for such text and it ensures that the table identifier remains associated with the table, including visually (by default). In addition, using the caption element allows screen reading software to navigate directly to the caption for a table if one is present.
"""

We should use captions :)
Comment 8 Nick Clemens 2018-10-15 17:05:18 UTC
Awesome work all!

Pushed to master for 18.11
Comment 9 Martin Renvoize 2018-10-17 11:00:17 UTC
Enhancement will not be backported to 18.05.x series.