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

date_time_selector popup is too constrained

    Details

    • Testing Instructions:
      Hide

      Test pre-requisites

      • This test has to be done on different themes (the more the better)

      Test steps

      1. Go to Navigation > Site pages > Calendar and click on 'New event'
      2. Click to edit the date
      3. Make sure the date selector does not overlap with the dropdown menu
      4. Using Firebug, edit the HTML and remove the fields 'Type of event', 'Name' and 'Description'.
      5. Click to edit the date
      6. Make sure the date selector does not overlap with the dropdown menu

      (This fix won't work when the inputs are closer to <body> but MDL-26649 will take care of it)

      Show
      Test pre-requisites This test has to be done on different themes (the more the better) Test steps Go to Navigation > Site pages > Calendar and click on 'New event' Click to edit the date Make sure the date selector does not overlap with the dropdown menu Using Firebug, edit the HTML and remove the fields 'Type of event', 'Name' and 'Description'. Click to edit the date Make sure the date selector does not overlap with the dropdown menu (This fix won't work when the inputs are closer to <body> but MDL-26649 will take care of it)
    • Workaround:
      Hide

      Change constraint from form to body:

      lib/form/yui/dateselector/dateselector.js

      fix_position : function() {
        if (this.currentowner) {
          // this.panel.set('constrain', this.currentowner.get('node').ancestor('form'));
          this.panel.set('constrain', this.currentowner.get('node').ancestor('body'));
          this.panel.set('align', {
            node:this.currentowner.get('node').one('select'),
            points:[Y.WidgetPositionAlign.BL, Y.WidgetPositionAlign.TL]
          });
        }
      },

      Show
      Change constraint from form to body: lib/form/yui/dateselector/dateselector.js fix_position : function() { if (this.currentowner) { // this.panel.set('constrain', this.currentowner.get('node').ancestor('form')); this.panel.set('constrain', this.currentowner.get('node').ancestor('body')); this.panel.set('align', { node:this.currentowner.get('node').one('select'), points:[Y.WidgetPositionAlign.BL, Y.WidgetPositionAlign.TL] }); } },
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-32297-master

      Description

      The date_time_selector has a javascript calendar popup, this works ok, unless the date_time_selector is one of the top elements of the form.
      The popup is constrained by the form area, but this is too restricted for some cases.

      I've added a picture of the form constrain, and one where it has been replaced by the body element (simple fix).

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for reporting that and providing a fix.

              Show
              salvetore Michael de Raadt added a comment - Thanks for reporting that and providing a fix.
              Hide
              fred Frédéric Massart added a comment -

              Thank you for your patch. I have applied it and created branches for 2.2, 2.3 and master.

              Show
              fred Frédéric Massart added a comment - Thank you for your patch. I have applied it and created branches for 2.2, 2.3 and master.
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Thanks for fixing this Esben and Fred.

              Patch looks good, pushing it for integration review.

              Show
              rajeshtaneja Rajesh Taneja added a comment - Thanks for fixing this Esben and Fred. Patch looks good, pushing it for integration review.
              Hide
              fred Frédéric Massart added a comment -

              Updated test instructions

              Show
              fred Frédéric Massart added a comment - Updated test instructions
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks Fred, this has been integrated now

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Fred, this has been integrated now
              Hide
              dmonllao David Monllaó added a comment -

              It passes in master and 2.3 using the standard theme and the binarius theme

              In 2.2 passes with the binarius theme but the javascript calendar always goes to the upper part of the screen using the standard theme.

              Marking as failed, but 2.3 and master are ok

              Show
              dmonllao David Monllaó added a comment - It passes in master and 2.3 using the standard theme and the binarius theme In 2.2 passes with the binarius theme but the javascript calendar always goes to the upper part of the screen using the standard theme. Marking as failed, but 2.3 and master are ok
              Hide
              fred Frédéric Massart added a comment -

              That is strange. I believe it behaves differently because we upgraded YUI from 2.2 to 2.3. It is also strange that it works with the theme Binarius...

              Show
              fred Frédéric Massart added a comment - That is strange. I believe it behaves differently because we upgraded YUI from 2.2 to 2.3. It is also strange that it works with the theme Binarius...
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Just noting that of the two "super" themes base does not support this, yet "canvas" does.
              Obviously something very minor is causing this problem.
              I do remember earlier versions of YUI having positioning quirks and perhaps we are facing one here.

              Show
              samhemelryk Sam Hemelryk added a comment - Just noting that of the two "super" themes base does not support this, yet "canvas" does. Obviously something very minor is causing this problem. I do remember earlier versions of YUI having positioning quirks and perhaps we are facing one here.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Aha I've back-tracked through the page structure to find the location of the problem.
              It's the product of a float+0height issue. The differences:

              In 23 and master where this fix works for both:

              • Canvas uses a wrapper(div.clearfix) immediately within #page to fix 0height. All layout blocks are within this.
              • Base uses a placeholder (div.clearfix) as the last element within #page to fix 0height.

              In 22 canvas is using the same solution, however in base that placeholder element is not there.
              The solution would be to add "<div class="clearfix"></div>" as the last element of #page in the base layouts (cherry-picking aa030ba)

              Fred could you please test that out and if happy let me know so that we can get this in an get Raj to test again.
              Noting if we add that then we really need to test the standard theme on several pages in 22.

              Cheers
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - Aha I've back-tracked through the page structure to find the location of the problem. It's the product of a float+0height issue. The differences: In 23 and master where this fix works for both: Canvas uses a wrapper(div.clearfix) immediately within #page to fix 0height. All layout blocks are within this. Base uses a placeholder (div.clearfix) as the last element within #page to fix 0height. In 22 canvas is using the same solution, however in base that placeholder element is not there. The solution would be to add "<div class="clearfix"></div>" as the last element of #page in the base layouts (cherry-picking aa030ba) Fred could you please test that out and if happy let me know so that we can get this in an get Raj to test again. Noting if we add that then we really need to test the standard theme on several pages in 22. Cheers Sam
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Thanks Sam,

              Will work with Fred to get this fixed in 22

              Show
              rajeshtaneja Rajesh Taneja added a comment - Thanks Sam, Will work with Fred to get this fixed in 22
              Hide
              fred Frédéric Massart added a comment -

              Good spotting Sam. Thanks for your solution! I have cherry-picked the commit and pushed my 2.2 branch.

              Show
              fred Frédéric Massart added a comment - Good spotting Sam. Thanks for your solution! I have cherry-picked the commit and pushed my 2.2 branch.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks Fred, I've merged in that change and this is back up for testing.
              All yours again Raj.

              Cheers
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Fred, I've merged in that change and this is back up for testing. All yours again Raj. Cheers Sam
              Hide
              dmonllao David Monllaó added a comment -

              2.2 version tested using the themes binarius, standard, boxxie, nonzero and splash. All ok, thanks

              Show
              dmonllao David Monllaó added a comment - 2.2 version tested using the themes binarius, standard, boxxie, nonzero and splash. All ok, thanks
              Hide
              poltawski Dan Poltawski added a comment -

              Congratulations!

              You've made it into the weekly release!

              Thanks for your contribution - here are some random drummers to keep you inspired for the next week!
              http://www.youtube.com/watch?v=_QhpHUmVCmY

              Show
              poltawski Dan Poltawski added a comment - Congratulations! You've made it into the weekly release! Thanks for your contribution - here are some random drummers to keep you inspired for the next week! http://www.youtube.com/watch?v=_QhpHUmVCmY

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    10/Sep/12