Bug 28901 - showCart incorrectly calculates position if content above navbar
Summary: showCart incorrectly calculates position if content above navbar
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on: 28910
Blocks:
  Show dependency treegraph
 
Reported: 2021-08-26 03:02 UTC by David Cook
Modified: 2021-11-05 14:46 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.05


Attachments
Bug 28901: showCart incorrectly calculates position if content above navbar (3.30 KB, patch)
2021-09-20 15:43 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 28901: showCart incorrectly calculates position if content above navbar (3.30 KB, patch)
2021-10-27 10:58 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 28901: showCart incorrectly calculates position if content above navbar (3.15 KB, patch)
2021-10-27 16:03 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 28901: showCart incorrectly calculates position if content above navbar (3.20 KB, patch)
2021-10-27 19:30 UTC, David Nind
Details | Diff | Splinter Review
Bug 28901: showCart incorrectly calculates position if content above navbar (3.26 KB, patch)
2021-10-31 00:39 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2021-08-26 03:02:26 UTC
We like to insert content like banners above the navbar. 

In 20.11, I notice that this causes the "Your cart is currently empty" message to appear way lower than it should.

It looks like that's due to the .navbar class being relatively positioned instead of statically positioned, which looks like it's probably a Bootstrap 4 upstream change.

I think the most obvious change is to update the algorithm used in "function showCart".
Comment 1 Jonathan Druart 2021-08-26 08:04:25 UTC
Not affecting master? Steps to recreate?
Comment 2 David Cook 2021-08-26 23:45:10 UTC
(In reply to Jonathan Druart from comment #1)
> Not affecting master? Steps to recreate?

It'll probably be affecting master. It might affect other versions after 19.11, but I haven't gotten around to looking yet. I've just fixed it locally for 20.11.

I'll have a think about drafting up some steps. I'll probably do something with injecting a HTML element above the navbar using Javascript. 

I wasn't supposed to be working on Koha yesterday, and today after I get through my emails I'm going to work on something else. But i'll make a note to come back to this.
Comment 3 Owen Leonard 2021-08-27 11:07:16 UTC
This can be reproduced in master by adding this to OPACUserJS:

$(document).ready(function(){
  $("#wrapper").prepend('<img src="https://generative-placeholders.glitch.me/image?width=900&height=200&style=triangles&gap=30">');
});


- Search the catalog
- Scroll down the page to a later search result
- Click the "Add to cart" link
Comment 4 Owen Leonard 2021-09-20 15:43:25 UTC Comment hidden (obsolete)
Comment 5 David Cook 2021-10-08 05:43:57 UTC
Sorry, Owen, but I couldn't get your patch to work. 

I ran both "yarn build" and "yarn build --view opac" but the "Your cart is currently empty" message is still appearing way too low when an image is prepended to the wrapper.

Am I doing something wrong?

The following is the patch I used to fix it locally:

diff --git a/koha-tmpl/opac-tmpl/bootstrap/js/basket.js b/koha-tmpl/opac-tmpl/bootstrap/js/basket.js
index 58e77c13c5..17ff077aeb 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/js/basket.js
+++ b/koha-tmpl/opac-tmpl/bootstrap/js/basket.js
@@ -469,10 +469,8 @@ function vShelfAdd() {
 function showCart(){
     var position = $("#cartmenulink").offset();
     var scrolld = $(window).scrollTop();
-    var top = position.top + $("#cartmenulink").outerHeight();
-    if( scrolld > top ){
-        top = scrolld + 15;
-    }
+    //BZ 28901
+    var top = $("#cartmenulink").outerHeight() + 10;
     var menuWidth = 200;
     var buttonWidth = $("#cartmenulink").innerWidth();
     var buttonOffset = menuWidth - buttonWidth;
Comment 6 David Cook 2021-10-08 05:55:23 UTC
Maybe it didn't work because I didn't actually apply the patch. You can tell it's 4:55pm on a Friday...
Comment 7 David Cook 2021-10-08 06:00:34 UTC
Btw reset back to Needs Signoff, and will have to have a crack another time, as it's the weekend!
Comment 8 David Nind 2021-10-27 05:48:42 UTC
Patch no longer applies 8-(..

Applying: Bug 28901: showCart incorrectly calculates position if content above navbar
Using index info to reconstruct a base tree...
M	koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss
M	koha-tmpl/opac-tmpl/bootstrap/js/basket.js
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/bootstrap/js/basket.js
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/js/basket.js
Auto-merging koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss
error: Failed to merge in the changes.
Patch failed at 0001 Bug 28901: showCart incorrectly calculates position if content above navbar
Comment 9 Owen Leonard 2021-10-27 10:58:15 UTC Comment hidden (obsolete)
Comment 10 David Nind 2021-10-27 15:19:27 UTC
Still can't get to apply 8-(..
Comment 11 Owen Leonard 2021-10-27 16:03:55 UTC
Created attachment 127021 [details] [review]
Bug 28901: showCart incorrectly calculates position if content above navbar

This patch makes changes to the OPAC Cart's CSS and JS so that the cart
pop-up message display more consistently. The CSS is changed to use
position "fixed" instead of "absolute." This allows the message to
display without recalculating the position every time, and keeps the
appearance we expect.

The z-index of the message is also increase to prevent it from being
hidden behind a floating toolbar.

To test, apply the patch and rebuild the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

 - Perform a catalog search in the OPAC which will return multiple
   results.
 - In the list of search results, click the "Add to cart" link next to
   several search results. Each time you should see a message appear in
   the upper left corner of the screen, "The item has been added to the
   cart."
 - The position of the messages should be consistent no matter how far
   down the page you scroll.
 - Test again after adding content to the OpacHeader region. One way to
   do this is to add the following to the OpacUserJS preference:

   $(document).ready(function(){
     $("#wrapper").prepend('<img src="https://generative-placeholders.glitch.me/image?width=900&height=200&style=triangles&gap=30">');
   });

 - Test at various browser widths, from a phone-sized screen width up to
   wide desktop-sized.
Comment 12 David Nind 2021-10-27 19:30:37 UTC
Created attachment 127022 [details] [review]
Bug 28901: showCart incorrectly calculates position if content above navbar

This patch makes changes to the OPAC Cart's CSS and JS so that the cart
pop-up message display more consistently. The CSS is changed to use
position "fixed" instead of "absolute." This allows the message to
display without recalculating the position every time, and keeps the
appearance we expect.

The z-index of the message is also increase to prevent it from being
hidden behind a floating toolbar.

To test, apply the patch and rebuild the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

 - Perform a catalog search in the OPAC which will return multiple
   results.
 - In the list of search results, click the "Add to cart" link next to
   several search results. Each time you should see a message appear in
   the upper left corner of the screen, "The item has been added to the
   cart."
 - The position of the messages should be consistent no matter how far
   down the page you scroll.
 - Test again after adding content to the OpacHeader region. One way to
   do this is to add the following to the OpacUserJS preference:

   $(document).ready(function(){
     $("#wrapper").prepend('<img src="https://generative-placeholders.glitch.me/image?width=900&height=200&style=triangles&gap=30">');
   });

 - Test at various browser widths, from a phone-sized screen width up to
   wide desktop-sized.

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 Katrin Fischer 2021-10-31 00:39:52 UTC
Created attachment 127120 [details] [review]
Bug 28901: showCart incorrectly calculates position if content above navbar

This patch makes changes to the OPAC Cart's CSS and JS so that the cart
pop-up message display more consistently. The CSS is changed to use
position "fixed" instead of "absolute." This allows the message to
display without recalculating the position every time, and keeps the
appearance we expect.

The z-index of the message is also increase to prevent it from being
hidden behind a floating toolbar.

To test, apply the patch and rebuild the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).

 - Perform a catalog search in the OPAC which will return multiple
   results.
 - In the list of search results, click the "Add to cart" link next to
   several search results. Each time you should see a message appear in
   the upper left corner of the screen, "The item has been added to the
   cart."
 - The position of the messages should be consistent no matter how far
   down the page you scroll.
 - Test again after adding content to the OpacHeader region. One way to
   do this is to add the following to the OpacUserJS preference:

   $(document).ready(function(){
     $("#wrapper").prepend('<img src="https://generative-placeholders.glitch.me/image?width=900&height=200&style=triangles&gap=30">');
   });

 - Test at various browser widths, from a phone-sized screen width up to
   wide desktop-sized.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Jonathan Druart 2021-11-02 15:53:23 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 15 Kyle M Hall 2021-11-05 14:46:48 UTC
Pushed to 21.05.x for 21.05.05