Date picker formslib element popup doesn't work when dateselector (a group of elements) is added within another group.

Ok, so after investigating it seems this is an issue in all prior Moodle versions. The only reason we are noticing this now is because the calendar icon is displaying next to the date selector and is not triggering the pop-up. In previous Moodle versions (where there is no icon) the date selector does not actually pop-up when using the select boxes on this page. The reason is because on this form the function createElement is used, rather than addElement. The reason why it differs between the two I have yet to investigate.

We have two options now -

1. Fix it so that the pop-up now displays on this form.
2. Hide the icon when we are using createElement.

I am going to try for option 1 for now.

Ok, so it seems the date initialiser JS is never executed on this form which is why the pop-up does not happen.

The JS loop in dateselector.js "Y.all('fieldset.fdate_time_selector').each(function(){" does not actually loop through anything in this case. The reason for this is because the dateselector (which is a group of select boxes and an image) is being added within another group, in this case by the function setupForm in user/filters/date. This means the CSS selectors are not the same, meaning the JS is not working.

s/not working/not being executed/g.

Wow, that was a difficult issue. My first aim was to change the forms library so that when a date_selector (or date_time_selector) was generated within a group I would wrap a fieldset with the class 'fdate_selector' around each date_selector so that the JS was triggered, the only issue with this was it did not display the group as expected, which would have meant CSS changes etc, which was not ideal as it would break any custom themes. I then created another CSS class to attach to a group fieldset that contained any date_selectors which triggered some new JS that looped through the DOM elements to determine how many unique date_selectors there were, then got the necessary elements associated with each (ie. day, month, year, checkbox and calendar image) and put JS events on them.

Ok, ended up coming up with a much easier solution after I had a think about it tonight. It's now past 1am and I think I am going to read, enough coding for today.

Alt method: https://github.com/markn86/moodle/compare/master...MDL-39187_master_alt

I now wrapping the date selectors in a span element with either the class fdate_selector or fdate_time_selector depending on the element. They are only ever wrapped in this span if they are added to a group, so in Moodle's case this is only in user/filters/date.php and mod/feedback/mod_form.php. However, I have changed mod/feedback/mod_form.php in MDL-39463 to use addElement, so this will not be an issue. This means for 99% of the cases a date_selector or date_time_selector is used the HTML generated on the page will not differ when this patch is applied.

Thanks Mark,

Patch looks good although you can reduce some code by considering "Add event" which is only called when element is added, so you don't have to add if in createElement and updatevalue element.

1. Add flag update in default which is called at AddElement, or

   _usedcreateelement = true; by default
   ...
   ...
   case 'AddElement:
      $this->_usedcreateelement = false;
      parent::onQuickFormEvent($event, $arg, $caller);
      break;
   

2. Not sure why you changed _wrap from protected to public. Probably best to keep it as protected.
3. Rather then using start_tag and end_tag and using 2 if's, you might want to use html_writer::tag() to do that job for you.

Rest all looks grt. As per JS performance, it seems it's better to use selectors without tags, so all good.

Thanks Raj, very useful points. Much appreciated. Pushing to integration now.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Your wish is my command, rebasing done.

Thanks Mark,

This looks like a nasty issue - thanks for working on it. The changes look reasonable and work for me. Integrated to master.

Tested on Chrome, and passed. Thanks!

Did you think this day was not going to arrive ever?

Your patience has been rewarded, yay, sent upstream, thanks!

Closing...ciao