Problem/Motivation

This is the same problem as #3163958: Position of popup is not right when collapsed in floating mode, the fix to that really messed things up.

The current code looks like this but makes little sense:

    var checkIfPopupIsClosed = debounce(function () {
      var wrapperHeight = $wrapper.outerHeight();
      if (drupalSettings.eu_cookie_compliance.popup_position) {
        var wrapperTopProperty = parseFloat($wrapper.css('bottom'));
        if (wrapperTopProperty !== 0) {
          $wrapper.css('top', wrapperHeight * -1);
        }
      }
      else {
        var wrapperBottomProperty = parseFloat($wrapper.css('bottom'));
        if (wrapperBottomProperty !== 0) {
          $wrapper.css('bottom', wrapperHeight * -1);
        }
      }
    }, 50);

Either the variables are misnamed, and/or it's checking/setting the wrong CSS properties.

What happens is that on resize it's fetching the bottom position, names it the top position, checks if it's not zero (which it'll never be when top positioned and open), then it moves the banner to just above outside the window by setting its top position to its negative height.
This happens every resize so the banner will never be visible after a first resize.
Also, why parseFloat and not parseInt? Do we expect subpixel accuracy? In that case it's pointless to compare to exactly 0 without first rounding. ;)

CommentFileSizeAuthor
#2 eu_cookie_compliance.3221485.1.patch684 bytestwod

Comments

TwoD created an issue. See original summary.

twod’s picture

Status: Active » Needs review
StatusFileSize
new684 bytes

Here's a first patch which somewhat fixes our use case. Not tested with any other configuration.

svenryen’s picture

Thanks TwoD, we'll take a look at the patch later this summer.

svenryen’s picture

Assigned: Unassigned » svenryen

  • svenryen committed ffeb810 on 8.x-1.x authored by TwoD
    Issue #3221485 by TwoD, svenryen: Fixed top banner disappears on resize
    
svenryen’s picture

Status: Needs review » Reviewed & tested by the community

I added the patch and it seems like it does no harm on my setup.

Merging it now. Thanks for the patch!

svenryen’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.