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
    • Rank:
      39093

      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).

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that and providing a fix.

          Show
          Michael de Raadt added a comment - Thanks for reporting that and providing a fix.
          Hide
          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
          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
          Rajesh Taneja added a comment -

          Thanks for fixing this Esben and Fred.

          Patch looks good, pushing it for integration review.

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

          Updated test instructions

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

          Thanks Fred, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Fred, this has been integrated now
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          Rajesh Taneja added a comment -

          Thanks Sam,

          Will work with Fred to get this fixed in 22

          Show
          Rajesh Taneja added a comment - Thanks Sam, Will work with Fred to get this fixed in 22
          Hide
          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
          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
          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
          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
          David Monllaó added a comment -

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

          Show
          David Monllaó added a comment - 2.2 version tested using the themes binarius, standard, boxxie, nonzero and splash. All ok, thanks
          Hide
          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
          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: