Bug 21645

Summary: Clean up library groups template
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: martin.renvoize, mtompset, nick
Version: Main   
Hardware: All   
OS: All   
URL: /cgi-bin/koha/admin/library_groups.pl
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 15707    
Bug Blocks: 20654    
Attachments: Bug 21645: Clean up library groups template
Bug 21645: Clean up library groups template
Bug 21645: Clean up library groups template
Bug 21645: Clean up library groups template
Bug 21645: e.preventDefault on click
Bug 21645: Fix focus when modal is shown

Description Owen Leonard 2018-10-24 13:56:41 UTC
The library groups template has several issues which need to be cleaned up:

- Bootstrap grid
- JavaScript to the footer
- Markup corrections
Comment 1 Owen Leonard 2018-10-24 14:18:14 UTC
Created attachment 81069 [details] [review]
Bug 21645: Clean up library groups template

This patch modifies the library groups administration page to help
compliance with several coding guidelines:

 - Move JavaScript to the footer
 - Convert to Bootstrap grid
 - Other markup corrections

To test, apply the patch and go to Administration -> Library groups.

 - Confirm that the page how has a "Library groups" heading.
 - Confirm that action buttons in the table of library groups are styled
   correctly.
 - Test both adding and editing groups. The "Required" label on required
   fields should be styled correctly.
 - Test all JavaScript-driven interactions on the page:
   - Adding, editing, and deleting groups
   - Removing libraries from groups
   - Expanding and collapsing group hierarchies
 - Validate the page's HTML and confirm that there are no errors found
   in the main body of the page.
Comment 2 Mark Tompsett 2018-10-24 16:17:33 UTC
edit-group classes are twice in some tags
delete-group classes are twice in some tags
<span>? -- probably should be a closed span.
-- the type stuff is in an include file.
recheck the div match ups, particularly across include files.
Comment 3 Owen Leonard 2018-10-24 17:08:36 UTC
Created attachment 81078 [details] [review]
Bug 21645: Clean up library groups template

This patch modifies the library groups administration page to help
compliance with several coding guidelines:

 - Move JavaScript to the footer
 - Convert to Bootstrap grid
 - Other markup corrections

To test, apply the patch and go to Administration -> Library groups.

 - Confirm that the page how has a "Library groups" heading.
 - Confirm that action buttons in the table of library groups are styled
   correctly.
 - Test both adding and editing groups. The "Required" label on required
   fields should be styled correctly.
 - Test all JavaScript-driven interactions on the page:
   - Adding, editing, and deleting groups
   - Removing libraries from groups
   - Expanding and collapsing group hierarchies
 - Validate the page's HTML and confirm that there are no errors found
   in the main body of the page.
Comment 4 Mark Tompsett 2018-10-24 17:58:55 UTC
Header looks better in the case of having library groups, but looks out of place when there are none. Perhaps a pretty shaded box telling us how lonely it is with no library groups would be better, but this isn't a blocker for me.

The HTML clean up is awesome (96 -> 5 errors, of which the remainder are include files and have nothing to do with this page specifically).
Comment 5 Mark Tompsett 2018-10-24 17:59:31 UTC
Created attachment 81081 [details] [review]
Bug 21645: Clean up library groups template

This patch modifies the library groups administration page to help
compliance with several coding guidelines:

 - Move JavaScript to the footer
 - Convert to Bootstrap grid
 - Other markup corrections

To test, apply the patch and go to Administration -> Library groups.

 - Confirm that the page how has a "Library groups" heading.
 - Confirm that action buttons in the table of library groups are styled
   correctly.
 - Test both adding and editing groups. The "Required" label on required
   fields should be styled correctly.
 - Test all JavaScript-driven interactions on the page:
   - Adding, editing, and deleting groups
   - Removing libraries from groups
   - Expanding and collapsing group hierarchies
 - Validate the page's HTML and confirm that there are no errors found
   in the main body of the page.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 6 Jonathan Druart 2018-10-25 20:19:27 UTC
Created attachment 81197 [details] [review]
Bug 21645: Clean up library groups template

This patch modifies the library groups administration page to help
compliance with several coding guidelines:

 - Move JavaScript to the footer
 - Convert to Bootstrap grid
 - Other markup corrections

To test, apply the patch and go to Administration -> Library groups.

 - Confirm that the page how has a "Library groups" heading.
 - Confirm that action buttons in the table of library groups are styled
   correctly.
 - Test both adding and editing groups. The "Required" label on required
   fields should be styled correctly.
 - Test all JavaScript-driven interactions on the page:
   - Adding, editing, and deleting groups
   - Removing libraries from groups
   - Expanding and collapsing group hierarchies
 - Validate the page's HTML and confirm that there are no errors found
   in the main body of the page.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2018-10-25 20:21:08 UTC
Created attachment 81198 [details] [review]
Bug 21645: e.preventDefault on click

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2018-10-25 20:21:13 UTC
Created attachment 81199 [details] [review]
Bug 21645: Fix focus when modal is shown

First the event must be defined in the document.ready block, then, since
bootstrap 3 the event is 'shown.bs.modal', not 'shown'

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Nick Clemens 2018-11-05 14:56:11 UTC
Awesome work all!

Pushed to master for 18.11
Comment 10 Martin Renvoize 2018-11-16 12:30:13 UTC
Enhancement, will not be backported to 18.05.x series.