If a user scrolls down onto a page, clicks a link that opens a modal window and resizes the browser, the "top" and "left" style values can be incorrect.

For example, I opened a login modal (from the modal_forms project) which is towards the bottom of a page. Then I ran the following debugging information in the chrome console.

jQuery("#modalContent").position().top; // returns 7901
jQuery(window).resize();
jQuery("#modalContent").position().top; // returns 29

The above code shows that the modal window is at the top of the page. Now a user needs to scroll up to see the modal window.

The code is wrong because it ignores that a user could have scrolled to a different portion of the page.

The code that is wrong is

      var mdcTop = wt + ( winHeight / 2 ) - (  modalContent.outerHeight() / 2);
      var mdcLeft = ( winWidth / 2 ) - ( modalContent.outerWidth() / 2);

It can be changed to factor in the user's scrolling. This also checks for a negative left, which would be caused by a large modalContent.outerWidth() and a small winWidth.

      var mdcTop =  $(document).scrollTop() + ( winHeight / 2 ) - ( modalContent.outerHeight() / 2); 
      var mdcLeft = $(document).scrollLeft() + ( winWidth / 2 ) - ( modalContent.outerWidth() / 2);

      if(mdcLeft < 0) { mdcLeft = 0; }
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

roberttstephens’s picture

Status: Active » Needs review
FileSize
744 bytes

Attached a patch.

roberttstephens’s picture

Issue summary: View changes

Typo

roberttstephens’s picture

Issue summary: View changes

Slight change in "code that's wrong"

fox_01’s picture

Issue summary: View changes

I got the problem if the modal window is opened and i resize the browser it will jump to the top of the page.

Patch #1 works for me.

mpotter’s picture

I don't think this is quite right. There is already code earlier in modal.js to calculate the "wt" variable to add to mdcTop. The problem here is that the code in modalContentResize() doesn't match the code above it used to originally position the modal. So any module that calls resize() will cause the modal to jump off the top of the page if the page is scrolled.

Rather than just adding scrollTop() as in patch #1, my patch just copies the "wt" logic into modalContentResize() so the modal doesn't jump at all.

(using this patch in Open Atrium)

dsnopek’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +panopoly

I was able to reproduce and the patch fixed the problem! We're going to start using this in Panopoly.

japerry’s picture

Status: Reviewed & tested by the community » Fixed

Patch tested, looks good. Fixed!

  • japerry committed 0ba18f6 on 7.x-1.x authored by mpotter
    Issue #2055785 by roberttstephens, mpotter, dsnopek: Modal window top...
dsnopek’s picture

Woohoo! Thanks. :-)

Status: Fixed » Closed (fixed)

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