Bug 21645 - Clean up library groups template
Summary: Clean up library groups template
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL: /cgi-bin/koha/admin/library_groups.pl
Keywords:
Depends on: 15707
Blocks: 20654
  Show dependency treegraph
 
Reported: 2018-10-24 13:56 UTC by Owen Leonard
Modified: 2019-10-14 19:56 UTC (History)
3 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 21645: Clean up library groups template (18.08 KB, patch)
2018-10-24 14:18 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21645: Clean up library groups template (25.29 KB, patch)
2018-10-24 17:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21645: Clean up library groups template (25.35 KB, patch)
2018-10-24 17:59 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 21645: Clean up library groups template (25.42 KB, patch)
2018-10-25 20:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21645: e.preventDefault on click (2.20 KB, patch)
2018-10-25 20:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21645: Fix focus when modal is shown (2.36 KB, patch)
2018-10-25 20:21 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-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.