Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-30627

M.core.dialogue allows for a negative top location

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            dobedobedoh 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
            dobedobedoh 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
            dobedobedoh Andrew Nicols added a comment -

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

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

            Changes look good thanks Andrew!

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

            This patch cherry-picks cleanly to:

            • master
            • MOODLE_22_STABLE
            • MOODLE_21_STABLE
            Show
            dobedobedoh Andrew Nicols added a comment - This patch cherry-picks cleanly to: master MOODLE_22_STABLE MOODLE_21_STABLE
            Hide
            nebgor 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
            nebgor 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
            abgreeve 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
            abgreeve 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
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  9/Jan/12