Date Repeat user interface improvements

After just a quick glance, my only question/concern so far is the 'Never' option under 'Range of Repeat.'
Date does not handle an infinite number of repeats well... (rather it does not handle them at all IIRC).

Also, 'Range of Repeat' seems a bit un-intuitive. But I am not sure how to better describe that part. Perhaps 'Repeat Until' ...

The Date module does not support 'never' because we have no way to handle an infinite number of dates. That's why there is no such option now. Unless Acquia also wants to tackle the issue of how in the heck we can create a date field with unlimited values we need to hide that option.

The patch won't apply to dev at all. When I applied it to alpha5 I got a syntax error, Parse error: syntax error, unexpected T_FUNCTION in ../date/date_repeat/date_repeat_form.inc on line 1004. The code there looks like:

function filter_checkbox_values($values) {
  return array_filter($values, function ($i) {
    return $i !== 0;
  });
}

Which doesn't work at all, nor can I figure out what it should be doing.

Could you re-roll against dev?

Kill it. Forcing a number into # of occurrences is probably sufficient

I don't know what 'kill it' means. Do you mean hide the option or leave it in. I'm very confused about that comment.

The form seems to work fine so long as you choose an Until date, but if you choose a count instead the results are wrong. This is because there was logic in date_repeat.inc that assumed the only valid option was UNTIL. I am working on that. I got it working but my fix broke the date repeat tests, so I have to dig into it.

I think I got the logic changes in date_repeat.inc and date_repeat_calc.inc figured out. I'm going to add an option to send and empty end day to date_repeat_calc.inc that is only valid if there is a count. Without one or the other we would potentially return an infinite array of values, which won't work.

I don't like the 'range of repeat' terminology either. I prefer Google calendar, just using 'Ends'. I think that is far more clear. Google calendar also floats the labels next to the elements (screenshot below), which takes less screen real estate. Is that a possibility?

I'm going to commit my changes that support the count option and do some more testing of the more complex possibilities. Can I ask you to write some tests for the new form and add them to the patch? I have tests for the calculation logic but especially when making changes like this to the form we could use some tests for the form itself.

The following commit should make it possible for the COUNT option to work correctly: http://drupalcode.org/project/date.git/commit/a4d8dea.

Sorry, but kill it, I meant exclude it from showing, i.e. leave it out or assume it was never in the design.

I also got a little tripped up on Range of Repeat - especially with the Never option. In fact I looked into that when this 'never' question came up. I didn't suggest a change though for two reasons..

1. If you use Google's 'ends' its confusing - seemingly conflicting - with the 'end' date that shows prior to selecting date.

2. If you remove Never I have less issue with the label, I.e.:
Range of repeat:
After [ ] occurrences'
I find this less confusing than two fields labeled 'End' (or End and Ends) that have two slightly different meanings.

What makes Google so successful here is that they use this format...
[ ] to [ ]
[x] Repeat
Ends
After [ ] occurrences.

I believe it's not an option to use the [ ] to [ ] format. As such, I think we need a new label for Range of Repeat that conflicts less with the first instances of End. 'Repeat ends' helps me quite a bit. You?

I'm OK with either 'Repeat ends' or 'Stop repeating'.

I like 'stop repeating'

you mean [ ] to [ ] ?

I thought this was off the table? Can you maybe reiterate the questions?

OK, this looks good and everything I tried works and you added tests and they work. The only thing not here is the idea of saving some screen real estate by floating the labels to the left, which is not critical. So I'm committing this much. If you interested and willing to do a follow-up patch to float the labels, that would be great. If not we can stop here.

I need to get this much committed because I have other issues about repeating values that I can't finish until this code is finalized.

Thanks!

The only question I see is about floating labels to the left, so I'll answer that.

I think it's ok to do so. It's usable either way. However all floats are not equal: https://skitch.com/jeff.noyes/gqau1/adobe-fireworks-cs5, and floating is not the Drupal standard.

#1 is probably easiest to build, but the UI is a little noisier because of the number of left edges on the form element. Additionally forms controls are on the right of the label except on [ ] repeat, [ ] exclude dates, [ ] include dates - which is a bit strange. I don't think this strangeness would hinder a usability test though.

#2 is probably harder to build, but the UI is less noisy because the form elements left edges are aligned. however, in this UI, the distance between label and form element is it's weakness. This UI also has the oddity of forms controls being on the right of the label except on [ ] repeat, [ ] exclude dates, [ ] include dates - which is a bit strange. Again, neither of these would probably show in a usability test

#3 is how Google does it. They do this to keep left edges aligned to reduce noise, and to make sure the distance between label / form is not an issue. However, I don't know of a case in Drupal where this treatment is used. Also, this UI also has the oddity of forms controls being on the right of the label except on [ ] repeat, [ ] exclude dates, [ ] include dates. Again, neither of these would probably show in a usability test

Net net, I'm a bit agnostic since all will test similarly. As it stands (stacked), is usable and consistent with Drupal - and stacked doesn't have the oddity of forms controls being on the right of the label except on [ ] repeat, [ ] exclude dates, [ ] include dates. It may have the weakness of more vertical real-estate, but I don't think it's a big issue here. If you choose to float, I tend to think #2 is the best since the distance between label / form is not to great, and because this pattern does exist in Drupal.

I guess I'm convinced we might as well leave it the way it is, stacked. Adding floating labels always introduces the possibility of browser bugs too.

So I guess the only thing left is whatever cleanup and additional tests temaruk was thinking about. I'll mark it 'needs work' to indicate we need that follow up patch.

Committed. Thanks! And thanks for adding tests for the additions and exceptions.

OK, I committed the last bit. Thanks!