Moodle
  1. Moodle
  2. MDL-30627

M.core.dialogue allows for a negative top location

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.3, 2.2, 2.3
    • Fix Version/s: 2.1.4, 2.2.1
    • Component/s: General
    • Labels:
    • Testing Instructions:
      Hide
      • Add the 'Community finder' block to a page and select 'Search'
      • Submit the form to search for courses
      • find a course with a comment
      • make your window small
      • click the comments button

      This is much more pronounced on non-core uses of M.core.dialogue which specify a larger hieght

      Show
      Add the 'Community finder' block to a page and select 'Search' Submit the form to search for courses find a course with a comment make your window small click the comments button This is much more pronounced on non-core uses of M.core.dialogue which specify a larger hieght
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30627-master-1
    • Rank:
      33437

      Description

      In enrol/yui/notification/notification.js under the centerDialogue function, the top and left values of the dialogue box are calculated based upon the bounding box for the page.

      However, these don't take into account that the dialogue could be larger than the window (e.g. if you have a very small window size set for some reason).
      If this is the case, then the dialogue disappears off the top of the screen.

      I suggest setting a minimum top/left of around 15px

        Issue Links

          Activity

          Hide
          Andrew Nicols added a comment -

          The community block was also trying to do some custom alignment which appears to override the default xy set by centerDialogue and thus breaks it slightly.

          I've removed the code which sets the alignment as part of this commit

          Show
          Andrew Nicols added a comment - The community block was also trying to do some custom alignment which appears to override the default xy set by centerDialogue and thus breaks it slightly. I've removed the code which sets the alignment as part of this commit
          Hide
          Andrew Nicols added a comment -

          Confirmed that this bug also affects the 2.1 series.
          The patch cherry-picks cleanly

          Show
          Andrew Nicols added a comment - Confirmed that this bug also affects the 2.1 series. The patch cherry-picks cleanly
          Hide
          Sam Hemelryk added a comment -

          Changes look good thanks Andrew!

          Show
          Sam Hemelryk added a comment - Changes look good thanks Andrew!
          Hide
          Andrew Nicols added a comment -

          This patch cherry-picks cleanly to:

          • master
          • MOODLE_22_STABLE
          • MOODLE_21_STABLE
          Show
          Andrew Nicols added a comment - This patch cherry-picks cleanly to: master MOODLE_22_STABLE MOODLE_21_STABLE
          Hide
          Aparup Banerjee added a comment -

          Thanks Andrew, thats been integrated.

          Tester: please check that we cannot have the dialogue top hidden beyond the window top.

          Show
          Aparup Banerjee added a comment - Thanks Andrew, thats been integrated. Tester: please check that we cannot have the dialogue top hidden beyond the window top.
          Hide
          Adrian Greeve added a comment -

          Tested in Firefox, Chrome and IE8. This patch works, but I found a problem with comments not being displayed in IE 8. I have created a new issue for that.

          Show
          Adrian Greeve added a comment - Tested in Firefox, Chrome and IE8. This patch works, but I found a problem with comments not being displayed in IE 8. I have created a new issue for that.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

          Now... disconnect, relax and enjoy the next days, yay!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: