Bug 16239

For anyone in fear of Bootstrap v3 not working with older browsers, I just want to add that there are third-party polyfills/shims available online, which allow Bootstrap v3 to work more or less with older browsers like IE7 and IE8. 

DSpace JSPUI uses Bootstrap v3, and I've utilized these other tools to make DSpace work correctly for older browsers (which are sadly still required by corporate/government IT departments).

While I don't think it's Koha's responsibility to deal with that third-party support at all, I just wanted to put it on the record that individuals should be able to get it work on their own, if they require it.

In other words, hurray for Owen being awesome!

(In reply to David Cook from comment #1)
> In other words, hurray for Owen being awesome!

Seconded! Bootstrap in the Staff client would be awesome as well. :)

I am pretty sure it's not perfect yet, but I'd like others to have a look at this...

Created attachment 56201 [details]
Radioboxes, checkboxes and a label do not dieplay as expected


First findings: It seems that radio boxes and checkboxes are moved after applying the patch.
See attached PDF (PDF bookmarks = comments)

Thanks for testing Marc!

I hope I've addressed the issues you point at and rebased on master.

Josef did you use the Bootstrap download customizer? Our 2.x Bootstrap js is limited to these plugins:

bootstrap-transition.js, bootstrap-modal.js, bootstrap-dropdown.js, bootstrap-tooltip.js, bootstrap-popover.js, bootstrap-affix.js, bootstrap-alert.js, bootstrap-button.js

I'm pretty sure the 2.x Bootstrap CSS is limited too, but unfortunately the file doesn't contain a list of the included components. I'll have to check my notes at home.

It think it would be worthwhile to continue to use the Bootstrap download customizer so that we're not including functionality we're not using and thus keeping JS and CSS file size smaller.

(In reply to Owen Leonard from comment #18)
> Josef did you use the Bootstrap download customizer? Our 2.x Bootstrap js is
> limited to these plugins:

I didn't, but I agree I should ;)

I'll try it and submit followup, thanks!

> It think it would be worthwhile to continue to use the Bootstrap download
> customizer so that we're not including functionality we're not using and
> thus keeping JS and CSS file size smaller.

Do we need tab, affix, and collapse? tab and collapse are currently being provided by jQueryUI. We should pick one or the other.

Overall everything looks great, but I found some issues:

- <caption> inheriting the color #777 from the default Bootstrap CSS (should be black).
- The MARC preview modal on the detail page is broken. There is some in-page CSS which changes the width of that modal (and some others -- cataloging merge for instance), and the update has broken it.
- DataTables "Processing" box which shows while tables are rendering looks bad.
- If you have translations installed with sub-languages (like en + en-GB), the menu link color on hover is white and thus unreadable.
- Radio button alignment on the Calendar entry form is off
- Split buttons wrapping badly on saved reports page and serials search results

(In reply to Owen Leonard from comment #51)
> 
> Do we need tab, affix, and collapse? tab and collapse are currently being
> provided by jQueryUI. We should pick one or the other.

I agree that we shouldn't duplicate the functionalities, so I removed these 3 modules and bootstrap js has 7 kB less in size now

> 
> Overall everything looks great, but I found some issues:
> 
> - <caption> inheriting the color #777 from the default Bootstrap CSS (should
> be black).

Fixed

> - The MARC preview modal on the detail page is broken. There is some in-page
> CSS which changes the width of that modal (and some others -- cataloging
> merge for instance), and the update has broken it.

There were some other I fixed before, but I missed this one. Now fixed.

> - DataTables "Processing" box which shows while tables are rendering looks
> bad.

Should be fixed.

> - If you have translations installed with sub-languages (like en + en-GB),
> the menu link color on hover is white and thus unreadable.

Fixed, look at it please, if it's ok or need adjusting more

> - Radio button alignment on the Calendar entry form is off

Fixed

> - Split buttons wrapping badly on saved reports page and serials search
> results

It's not as easy as it could looks like... I tried solution from here: https://github.com/twbs/bootstrap/issues/9939, another possibility is using flex, if we decide we can adapt it, see http://caniuse.com/#search=flexbox for browser support. 

I also squashed the patches again.

I hope I haven't break anything.

Thanks for testing!

Created attachment 57420 [details]
Config of bootstrap customization

Rebased on master
 
> > - Split buttons wrapping badly on saved reports page and serials search
> > results
> 
> It's not as easy as it could looks like... I tried solution from here:
> https://github.com/twbs/bootstrap/issues/9939, another possibility is using
> flex, if we decide we can adapt it, see http://caniuse.com/#search=flexbox
> for browser support. 
> 

I added more specifity for the split button style just to affect only the buttons in tables, not other (like in toolbars for example)

CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc

Created attachment 58678 [details] [review]
Bug 16239: Update Bootstrap to 3.3.7

Customization tool used.

Javascript modules:
    alert
    button
    dropdown
    modal
    tooltip
    popover
    transition

CSS modules:
    type
    code
    grid
    tables
    forms
    buttons
    responsive-utilities
    button-groups
    input-groups
    navs
    navbar
    breadcrumbs
    pagination
    pager
    labels
    badges
    thumbnails
    alerts
    progress-bars
    media
    list-group
    panels
    responsive-embed
    close
    component-animations
    dropdowns
    tooltip
    popovers
    modals

Test plan:
1) Apply the patch
2) Try to go through interface to find problems, inconsistencies, etc.
3) Read the diff

Migration guide for bootstrap: http://getbootstrap.com/migration/

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 58679 [details] [review]
Bug 16239: Update templates

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 58680 [details] [review]
Bug 16239: Update css files

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 58681 [details] [review]
Bug 16239: Update javascript files

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 58682 [details] [review]
Bug 16239: Use inline-block for split buttons only in tables

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 58683 [details] [review]
Bug 16239 [CSS Follow-up] Upgrade Bootstrap in the staff client

This followup tweaks some CSS and markup in order to correct conflicts
between default Bootstrap styles and the expected (previous) style of
the staff client. Included:

- Corrections to the widths of some modals.
- Corrections to the style of the <pre> tag, seen often in MARC preview
  modals.
- Corrections to the default active and hover states for dropdown menus.

To test, apply the patch and view various pages which use Bootstrap
modals, confirming that they look correct.

- Add order from staged file MARC preview
- Add order from external source MARC preview
- Cataloging record merge MARC preview
- Batch record modification MARC preview
- Authorities Z39.50 search MARC preview

Created attachment 58691 [details] [review]
Bug 16239 [CSS Follow-up] Upgrade Bootstrap in the staff client

This followup tweaks some CSS and markup in order to correct conflicts
between default Bootstrap styles and the expected (previous) style of
the staff client. Included:

- Corrections to the widths of some modals.
- Corrections to the style of the <pre> tag, seen often in MARC preview
  modals.
- Corrections to the default active and hover states for dropdown menus.

To test, apply the patch and view various pages which use Bootstrap
modals, confirming that they look correct.

- Add order from staged file MARC preview
- Add order from external source MARC preview
- Cataloging record merge MARC preview
- Batch record modification MARC preview
- Authorities Z39.50 search MARC preview

Signed-off-by: Claire <claire_gravely@hotmail.com>

Created attachment 58697 [details] [review]
Bug 16239 [CSS Follow-up] Upgrade Bootstrap in the staff client

This followup tweaks some CSS and markup in order to correct conflicts
between default Bootstrap styles and the expected (previous) style of
the staff client. Included:

- Corrections to the widths of some modals.
- Corrections to the style of the <pre> tag, seen often in MARC preview
  modals.
- Corrections to the default active and hover states for dropdown menus.

To test, apply the patch and view various pages which use Bootstrap
modals, confirming that they look correct.

- Add order from staged file MARC preview
- Add order from external source MARC preview
- Cataloging record merge MARC preview
- Batch record modification MARC preview
- Authorities Z39.50 search MARC preview

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>

Some quick remarks and questions:
- Is it expected to have these "big" buttons everywhere? Why do we want to have bigger buttons?
Note that some buttons have not been converted.
- Some JS code have been commented in cateditor-ui.inc. I did not get why it is for. We should either add a comment to explain why it is commented, remove the code or let it as before.

Otherwise everything looks fine, great job!

Created attachment 58722 [details] [review]
Bug 16239: Update Bootstrap's license on the about page

Thanks for testing and feedback Jonathan!

(In reply to Jonathan Druart from comment #85)
> Some quick remarks and questions:
> - Is it expected to have these "big" buttons everywhere? Why do we want to
> have bigger buttons?

The buttons in bootstrap 3 are a bit bigger than in version 2 by default. So button "levels" in Koha remains the same, but the absolute size of button is different. We can shift it down in Koha if we really wan't, but I don't think it is neccesary - from my point of view it more comfortable to use bigger buttons, but it's just my opinion, I don't have any data about that for now...

> Note that some buttons have not been converted.

Many buttons weren't transformed to bootstrap ever yet, I plan to make followup (omnibus) bug for using bootstrap everywhere where it makes sense.

Mayby some buttons which are new due to continuing the work on Koha master, I updated this patchset about a month ago - I can make a followup, or we can decide to make it on another bug, what do you think?

> - Some JS code have been commented in cateditor-ui.inc. I did not get why it
> is for. We should either add a comment to explain why it is commented,
> remove the code or let it as before.
> 

Yep, we should remove this code, it was for styling of modals, which is now made differently. I'll add the followup patch

> Otherwise everything looks fine, great job!

Thanks! ;)

Created attachment 58729 [details] [review]
Bug 16239: Followup - remove unusable js from cateditor-ui.inc

(In reply to Josef Moravec from comment #87)
> 
> Many buttons weren't transformed to bootstrap ever yet, I plan to make
> followup (omnibus) bug for using bootstrap everywhere where it makes sense.
> 

Bug 17873

Created attachment 58731 [details] [review]
Bug 16239: Update Bootstrap to 3.3.7

Customization tool used.

Javascript modules:
    alert
    button
    dropdown
    modal
    tooltip
    popover
    transition

CSS modules:
    type
    code
    grid
    tables
    forms
    buttons
    responsive-utilities
    button-groups
    input-groups
    navs
    navbar
    breadcrumbs
    pagination
    pager
    labels
    badges
    thumbnails
    alerts
    progress-bars
    media
    list-group
    panels
    responsive-embed
    close
    component-animations
    dropdowns
    tooltip
    popovers
    modals

Test plan:
1) Apply the patch
2) Try to go through interface to find problems, inconsistencies, etc.
3) Read the diff

Migration guide for bootstrap: http://getbootstrap.com/migration/

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Created attachment 58732 [details] [review]
Bug 16239: Update templates

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Created attachment 58733 [details] [review]
Bug 16239: Update css files

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Created attachment 58734 [details] [review]
Bug 16239: Update javascript files

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Created attachment 58735 [details] [review]
Bug 16239: Use inline-block for split buttons only in tables

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Created attachment 58736 [details] [review]
Bug 16239 [CSS Follow-up] Upgrade Bootstrap in the staff client

This followup tweaks some CSS and markup in order to correct conflicts
between default Bootstrap styles and the expected (previous) style of
the staff client. Included:

- Corrections to the widths of some modals.
- Corrections to the style of the <pre> tag, seen often in MARC preview
  modals.
- Corrections to the default active and hover states for dropdown menus.

To test, apply the patch and view various pages which use Bootstrap
modals, confirming that they look correct.

- Add order from staged file MARC preview
- Add order from external source MARC preview
- Cataloging record merge MARC preview
- Batch record modification MARC preview
- Authorities Z39.50 search MARC preview

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Created attachment 58737 [details] [review]
Bug 16239: Update Bootstrap's license on the about page

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Created attachment 58738 [details] [review]
Bug 16239: Followup - remove unusable js from cateditor-ui.inc

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Josef,
Yes, everything else on another bug report :)

(In reply to Jonathan Druart from comment #98)
> Josef,
> Yes, everything else on another bug report :)

;)

I started working on it ;)

Pushed to master for 17.05, thanks Josef, Owen!

Obviously: This won't get ported back to 16.11.x as it is an enhancement.

Update your QA tests repo!

commit 89a86cceb79b412a26b68cd8c2e4907060af2f6d
Author: Jonathan Druart <jonathan.druart@gmail.com>
Date:   Wed Jan 18 13:16:57 2017 +0100

    Add checkts for Bootstrap v3 (bug 16239)

(arf typo)

(In reply to Jonathan Druart from comment #102)
> Update your QA tests repo!
> 
> commit 89a86cceb79b412a26b68cd8c2e4907060af2f6d
> Author: Jonathan Druart <jonathan.druart@gmail.com>
> Date:   Wed Jan 18 13:16:57 2017 +0100
> 
>     Add checkts for Bootstrap v3 (bug 16239)
> 
> (arf typo)

Great, thank you Jonathan!