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

          Attachments

            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