Moodle
  1. Moodle
  2. MDL-39047

Date picker/File picker javascript conflict

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5
    • Labels:
      None
    • Testing Instructions:
      Hide

      Test on all supported browsers and themes (sorry)

      1. Create a new course.
      2. On the settings page click on the calendar icon next to 'Course start date'.
      3. Ensure it renders above the 'Course overview files' area (ie the dotted lines do not show on top of it).
      4. Choose a date that is hovering over the filemanager between the dotted lines and make sure the filepicker does not pop-up (it usually pops up when the filemanager is clicked).
      5. Upload a file to the file area.
      6. Click on the calendar icon and select a date on the calendar pop-up directly above that file.
      7. Ensure the file information does not pop-up.
      Show
      Test on all supported browsers and themes (sorry) Create a new course. On the settings page click on the calendar icon next to 'Course start date'. Ensure it renders above the 'Course overview files' area (ie the dotted lines do not show on top of it). Choose a date that is hovering over the filemanager between the dotted lines and make sure the filepicker does not pop-up (it usually pops up when the filemanager is clicked). Upload a file to the file area. Click on the calendar icon and select a date on the calendar pop-up directly above that file. Ensure the file information does not pop-up.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-39047_master
    • Rank:
      49166

      Description

      The new course files feature puts a file picker directly above a date picker.

      If there are files in the picker, and the calendar popup overlays the file picker, a click on the calendar will be handled by both inputs (the date is saved and then a popup for the file that was behind the calendar appears).

      My guess is the calendar is not preventing the click event from propogating.

        Issue Links

          Activity

          Hide
          Mark Nelson added a comment -

          Just a note to the developer, this was happening before the introduction of MDL-26649, so it is not a regression caused by this.

          Show
          Mark Nelson added a comment - Just a note to the developer, this was happening before the introduction of MDL-26649 , so it is not a regression caused by this.
          Hide
          Mark Nelson added a comment -

          Imo, this should be set to must fix for 2.5.

          Show
          Mark Nelson added a comment - Imo, this should be set to must fix for 2.5.
          Hide
          Rajesh Taneja added a comment -

          Thanks Mark,

          Patch looks good, pushing it for integration.

          [y] Syntax
          [y] Output
          [y] Whitespace
          [-] Language
          [-] Databases
          [y] Testing
          [-] Security
          [-] Documentation
          [y] Git
          [y] Sanity check

          Show
          Rajesh Taneja added a comment - Thanks Mark, Patch looks good, pushing it for integration. [y] Syntax [y] Output [y] Whitespace [-] Language [-] Databases [y] Testing [-] Security [-] Documentation [y] Git [y] Sanity check
          Hide
          Andrew Nicols added a comment -

          I'm going to throw a spanner in the works and say that this doesn't look okay.
          We should really be applying things like z-index in theme style sheets and not in code. Doing so can potentially cause difficulties for theme designers who have changed the stacking orders for all sorts of components.

          I'm aware that Y.Panel and Y.Overlay forcibly set a z-index of 1 (and there is a bug report in YUI3 for this). To unset it and revert back to the theme style sheet, after creating your Y.Panel, you just need to do a:

          this.panel.setStyle('zIndex', null);
          

          or

          this.panel.setStyle('zIndex', '');
          
          Show
          Andrew Nicols added a comment - I'm going to throw a spanner in the works and say that this doesn't look okay. We should really be applying things like z-index in theme style sheets and not in code. Doing so can potentially cause difficulties for theme designers who have changed the stacking orders for all sorts of components. I'm aware that Y.Panel and Y.Overlay forcibly set a z-index of 1 (and there is a bug report in YUI3 for this). To unset it and revert back to the theme style sheet, after creating your Y.Panel, you just need to do a: this .panel.setStyle('zIndex', null ); or this .panel.setStyle('zIndex', '');
          Hide
          Mark Nelson added a comment -

          Hi Andrew, I agree with you about setting it in the CSS, however setStyle is not a function for the Overlay object.

          Show
          Mark Nelson added a comment - Hi Andrew, I agree with you about setting it in the CSS, however setStyle is not a function for the Overlay object.
          Hide
          Andrew Nicols added a comment -

          Sorry Mark, you're spot on. You need to apply the style to the boundingBox:

          this.panel.get('boundingBox').setStyle('zIndex', null);

          Show
          Andrew Nicols added a comment - Sorry Mark, you're spot on. You need to apply the style to the boundingBox: this.panel.get('boundingBox').setStyle('zIndex', null);
          Hide
          Andrew Nicols added a comment -

          p.s. There are a few areas hit by that currently in Moodle which need fixing.

          Show
          Andrew Nicols added a comment - p.s. There are a few areas hit by that currently in Moodle which need fixing.
          Hide
          Mark Nelson added a comment -

          Hi Andrew, that did not work either. I ended up using this.panel.removeAttr('zIndex'); which was suggested by Raj. I have now updated my diff to include the z-index in the bootstrap and the base theme.

          Show
          Mark Nelson added a comment - Hi Andrew, that did not work either. I ended up using this.panel.removeAttr('zIndex'); which was suggested by Raj. I have now updated my diff to include the z-index in the bootstrap and the base theme.
          Hide
          Andrew Nicols added a comment -

          Thanks Mark - that looks good now.

          Show
          Andrew Nicols added a comment - Thanks Mark - that looks good now.
          Hide
          Dan Poltawski added a comment -

          Integrted to master - thanks Mark

          Show
          Dan Poltawski added a comment - Integrted to master - thanks Mark
          Hide
          Rossiani Wijaya added a comment -

          This is working as expected.

          Tested for Master.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This is working as expected. Tested for Master. Test passed.
          Hide
          Dan Poltawski added a comment -

          Blooming Marvelous! It's time for a knees up - your changes are upstream!

          Thanks for making Moodle better!

          Toodle pip

          Show
          Dan Poltawski added a comment - Blooming Marvelous! It's time for a knees up - your changes are upstream! Thanks for making Moodle better! Toodle pip

            People

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

              Dates

              • Created:
                Updated:
                Resolved: