Bug 9617 - Replace YUI menu on catalog pages with Bootstrap
Summary: Replace YUI menu on catalog pages with Bootstrap
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Owen Leonard
QA Contact: Jonathan Druart
URL: /cgi-bin/koha/members/detail.pl
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-15 14:47 UTC by Owen Leonard
Modified: 2014-05-26 21:04 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 9617 - Replace YUI menu on catalog pages with Bootstrap (21.09 KB, patch)
2013-02-19 20:01 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 9617 - Replace YUI menu on catalog pages with Bootstrap (21.20 KB, patch)
2013-02-21 17:24 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 9617 - Replace YUI menu on catalog pages with Bootstrap (21.25 KB, patch)
2013-02-28 15:26 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 2013-02-15 14:47:17 UTC
The toolbar include file used on catalog detail pages (detail.pl, MARCdetail.pl, etc.) uses YUI for buttons and button menus. It should use Bootstrap instead.
Comment 1 Owen Leonard 2013-02-19 20:01:43 UTC Comment hidden (obsolete)
Comment 2 Bernardo Gonzalez Kriegel 2013-02-21 17:24:38 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2013-02-28 15:26:20 UTC
QA comment:

Woow, great patch. cat-toolcar.inc burned eyes so much! :)

All works perfectly, I didn't find any regression.
I think that a form element with an id='f' and name='f' is not really a good idea since it is an include file, but it was already present before this patch.

However, one remark (because I don't know if it is wanted): the div#toolbar has a margin-top: 10px; added by the .btn-toolbar. So the toolbar is 10px under its previous position.

Marked as Passed QA.
Comment 4 Jonathan Druart 2013-02-28 15:26:45 UTC
Created attachment 15773 [details] [review]
Bug 9617 - Replace YUI menu on catalog pages with Bootstrap

This patch converts the toolbar include file used by catalog detail
pages (detail.pl, MARCdetail.pl, etc.) replacing YUI button and menu
code with Bootstrap.

Minor change to staff client CSS fixes issue with language footer.
Minor change to doc-head-close.inc corrects error in logic for handling
cart and lists js discovered in testing.

To test, view any page which uses cat-toolbar.inc. Buttons and menus
should look correct and work correctly. Functions to test include:

New record
New item
New subscription
New child record

Edit record
Edit items
Edit items in a batch
Delete items in a batch
Attach item
Edit as new (duplicate)
Replace record via Z39.50
Delete record
Delete all items

Save (various formats)

Add to cart
Add to list

Print

Place hold (including with search-to-hold active).

Testing should be done by users with varying permissions including with
and without:

CAN_user_serials_create_subscription
CAN_user_editcatalogue_edit_catalogue
CAN_user_editcatalogue_edit_items
CAN_user_tools_items_batchmod
CAN_user_tools_items_batchdel

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Comment: All buttons tested. Works as described. No Errors.
Looks good.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 5 Jonathan Druart 2013-02-28 15:29:36 UTC
(In reply to comment #3)
> However, one remark (because I don't know if it is wanted): the div#toolbar
> has a margin-top: 10px; added by the .btn-toolbar. So the toolbar is 10px
> under its previous position.

Owen, If is not wanted, we can redefine the margin-top property for div#toolbar (
margin-top: 0;). Could be done in a followup.
Comment 6 Jared Camins-Esakov 2013-03-02 17:03:56 UTC
This patch has been pushed to master.