[Ideas] New interactive widget

The reason for having the widget in JS is that the form is quite interactive, and that I could reuse existing code for a jQuery-based RRULE widget (see rrule.widget.js).

Of course you're right that a new Drupal form element would be best. I don't know if I have time soon to get to it, so feel free to work on this. I wouldn't actually create the individual form elements in Drupal (due to them being highly interconnected in JS, and the JS also includes custom logic to parse out the values by their names, which correspond to the matching properties in rrule.js). Instead, I'd create an element that has all relevant options, which are then passed to JS as settings, and then adapt rrule.widget.js to behave differently (or exclude/include form fields) based on these options.

The UNTIL value doesn't get converted on storage so the conversion to the site timezone when loading the value was causing the value to increment/decrement on edit and save of content using this field depending on your site timezone settings. I've removed this conversion in this updated patch.

Patch missing added files and wouldn't apply, reroll.

Reroll to fix typo in libraries link to moment.min.js (instead of moment-min.js)

Added fix for js issue with monthly reoccuring droppdown that contains checkboxes.

Futher JS fix

@gordon looks like Victor sent me the wrong file.. will update tomorrow

update, closes the monthly dropdown when you defocus it.

hopefully this fixes that.

Lets try that one more time.

Reroll of #20 (wouldn't apply as part of composer.json for me)

@gordon any idea why the patch is drastically smaller than #19?

Have noticed a few other bugs @gordon, if you select monthly/weekly and choose some days, and save, and return to the form and select daily instead, it still applies the other rules.

they don't get posted to the server @gordon however the results do not represent the settings saved, ie. editing the page draft will display the wrong information.

Upated to add a fix for dates that were strings instead of date objects, was throwing error in line 338 of DateRecurRRule.php

I haven't tested this yet, but I had some nitpicks from just reading the code.

1. +++ b/assets/date_recur_rrule.css
   @@ -64,20 +74,13 @@
   \ No newline at end of file
   

   Needs newline

2. +++ b/assets/date_recur_rrule.js
   @@ -3,71 +3,283 @@
   +  //
   +  RRule.FREQUENCY_CODES = [
   

   This comment should either be removed or something should be written here.

3. +++ b/assets/date_recur_rrule.js
   @@ -3,71 +3,283 @@
   +  // add helpful constants to RRule
   +  RRule.FREQUENCY_NAMES = [
   

   Needs to start with a Capital and end with a period.

4. +++ b/assets/date_recur_rrule.js
   @@ -3,71 +3,283 @@
   +}(jQuery, Drupal, RRule, moment));
   \ No newline at end of file
   

   Needs newline

5. +++ b/composer.json
   @@ -11,6 +11,7 @@
   +    "moment/moment": "2.*"
   

   This is a javascript library, right? I don't think we can get those via composer. At least not with a default setup. It's possible only when extending composer to also do front-end packages.

   This should be removed from composer.json and committed (if the license allows it) or fixed in another way.

6. +++ b/src/DateRecurDefaultRRule.php
   @@ -45,7 +45,7 @@ class DateRecurDefaultRRule extends RRule {
        $daynames = DateHelper::weekDays(TRUE);
   -    array_push($daynames, $daynames[0]);
   +    array_push($daynames, $daynames[0]); // Move Sunday to the end of the week as day 7.
   

   This needs to respect 80 cols rule and the comment should be before the actual action. So:

   Move sunday to the end of the week as day 7
   array_push($daynames, $daynames[0]);

7. +++ b/src/DateRecurRRule.php
   @@ -121,6 +121,18 @@ class DateRecurRRule implements \Iterator {
   +   * return the current rule.
   

   This comment should be expanded, to have an @return statement and it should also start with uppercase.

8. +++ b/src/DateRecurRRule.php
   @@ -320,6 +332,12 @@ class DateRecurRRule implements \Iterator {
   +      $date = date_create_from_format('Y-m-d\TH:i:s',$date);
   

   Space after comma (H:i:s', $date)

9. +++ b/src/Element/RRule.php
   @@ -0,0 +1,501 @@
   +    // If no rule is set then use a default of rule of weekly but turn repeating off.
   

   Needs to respect 80 cols rule.

10. +++ b/src/Element/RRule.php
    @@ -0,0 +1,501 @@
    +      '#value' => isset($parts['UNTIL']) ? [ 'date' => $date->format(DateFormat::load('html_date')->getPattern()), 'time' => $date->format(DateFormat::load('html_time')->getPattern()), 'object' => $date ]  : NULL,
    

    This line is really long and hard to understand, can we extract this variable and make it simpler by not using an inline if here?

11. +++ b/src/Element/RRule.php
    @@ -0,0 +1,501 @@
    +   *
    +      ],
    

    This doesn't look right.

12. +++ b/src/Element/RRule.php
    @@ -0,0 +1,501 @@
    +   * @todo There seems to be an issue where the object is not being saved to the datetime field. I am not sure if this is an issue with datetime_tweaks module or some other weird thing.
    

    80 cols

13. +++ b/src/Element/RRule.php
    @@ -0,0 +1,501 @@
    +  public static function getRRule($element, $value, FormStateInterface $form_state) {
    

    Needs a docblock.

14. +++ b/src/Element/RRule.php
    @@ -0,0 +1,501 @@
    +  protected static function buildParts($element, $input, FormStateInterface $form_state) {
    

    Needs a docblock

15. +++ b/src/Element/RRule.php
    @@ -0,0 +1,501 @@
    +    $parts = array_change_key_case(array_intersect_key($input, array_flip(['freq', 'interval', 'byday', 'byhour', 'byminute', 'bysecond'])), CASE_UPPER);
    

    I think I had to read this line 4 times before I understood what's going on in here :D. Looks real nice though, good work!

16. +++ b/src/Element/RRule.php
    @@ -0,0 +1,501 @@
    +      if ($pos = array_filter($input['pos'])) {
    +        $new_pos = [];
    

    This is very, very personal, but I dislike setting a variable in an if statement, can this be refactored to be something like this.

    $pos = array_filter($input['pos']);
    if (!empty($pos)) { ...
    

    This improves readability, is stricter and less prone to others (espcially me ;)) fucking up (by thinking that it should be 2*=, changing that and everything breaking)

17. +++ b/templates/rrule-byday-pos-wrapper.html.twig
    @@ -0,0 +1,20 @@
    + * Theme override for a 'checkboxes' #type form element.
    

    /s/checkboxes/rrule-byday-pos-wrapper/

18. +++ b/templates/rrule-end.html.twig
    @@ -0,0 +1,25 @@
    + * Default theme implementation for a 'radios' #type form element.
    

    /s/radios/rrule-end/ ?

19. +++ b/templates/rrule-end.html.twig
    @@ -0,0 +1,25 @@
    \ No newline at end of file
    

    Needs newline

Back to needs work per #28

I'm uploading a new patch, re-rolled against the current branch and addressing many of the issues listed. This patch also fixes

• Drupal Coding standards violations
• PHP Notices generated in some use cases
• Defensive code to avoid uncaught exception errors

OMG, same names used in the files, that was stupid.

My apologies. Second patch and interdiff are correct ones, in spite of the duplicate names. I found an error in the JavaScript for a data attribute and corrected it.

It's too late now to rename the patch files, now they're uploaded with an incorrect comment number.

Patch from #31 mostly works. Thanks all!

There are two issues I've noticed so far when selecting "Monthly". It will ignore the Month selection but save the Day selection.
i.e.
If I choose Day: Sunday and Month: Aug, Oct
Summary: every August and October on Sunday
RRule: FREQ=MONTHLY;BYDAY=SU;BYMONTH=8,10

The result of saving is repeat on every Sunday, every month. The month is ignored.

Editing the node again shows the month(s) are not selected and the Summary does not show a month selection.

Summary: On the Sunday each month at 22:59
RRule: FREQ=MONTHLY;BYDAY=SU

The second thing as a comment about the summary. Should the summary read [day] in [months] instead?
Summary: every Sunday in August and October

Also noticed Summary time is way off: Set 00:00:00, but it shows 23:48. Just in the summary. The data is saved correctly.

Can someone explain the need for the moment library dependency? I see it's called twice, once for the moment.ISO_8601 standard and the other for the format.

I know there's not a consistent mechanism for including libraries in Drupal with a contrib module but having it as an external library would be ideal so it doesn't conflict with other projects (hopefully) using the same library.

Good work; this is a big improvement all round.

As with the D7 Date module and Option to hide end date, keeping time, imposing a hard requirement for an end date is undesirable. I note that alpha 4 version does not require a time, although the input field remains visible, but this seems to have been overlooked with this patch.

Like D7 Date, I do think the option to hide the time field would make for the best UX and compatibility with a good third party date picker would round it off nicely.

I'm very interested in testing the patch but it does not apply to the current dev version of date_recur.

A bunch of things have changed in the newest dev version so it'll take time to refactor.

As noted above many things have changed since the last round of work on patches here. Before going further I'd encourage working on the widget in a new project, similar to the other widget issue: #2933796-21: [Ideas] Add new Drupalized widget. Notably dependencies like rrule.js are no longer in the main project so theyd need to be re-introduced.

Theres good work here, keep it going!

There seems to be little momentum or interest in this issue, so closing.