490169 - Should be an easily-accessible option to fire an alarm "on time"

Status: RESOLVED FIXED

(Calendar :: Dialogs, enhancement)

Description

[Brennan Vincent]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US) AppleWebKit/525.19 (KHTML, like Gecko) Chrome/1.0.154.59 Safari/525.19
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.18pre) Gecko/20080917 Sunbird/0.9

The user has a few predefined choices to fire an alarm before an event (five minutes before, ten minutes before, etc.) without having to define a custom alarm. These should include the option to fire an alarm at the moment the event starts.

Reproducible: Always

Steps to Reproduce:
1. Create a new event.
2. Attempt to set an alarm to occur at the moment the event starts.
Actual Results:  
1. It is necessary to create a custom alarm.

Expected Results:  
1. That should be a predefined option.

Comment 1

[Brennan Vincent]

Attachment: Patch fixing the problem

Something like this...

Comment 2

[Martin Schröder [:mschroeder]]

Comment on attachment 374627 [details] [diff] [review]
Patch fixing the problem

Brennan, please set the review flag next time you attach a patch you want somebody to have a look at. :) Also see https://wiki.mozilla.org/Calendar:Module_Ownership.

Comment 3

[Stefan Sitter [:ssitter]]

Some minor comments: This change is contrary to Bug 391673 that requests to shorten the list. Is the label "On time" informative? Or should it be "0 minutes" to match the other entries? In particular, if using a non en-US localization? Currently a 0 minutes events is displayed as "The moment the event starts".

Comment 4

[Brennan Vincent]

Attachment: Changing "On time" to "0 minutes"

Comment 5

[Brennan Vincent]

Stefan,

I have thought about your comments.
I disagree that this is contrary to the bug you linked. The list could still be shortened if "0 minutes" were added as an option. In particular, I think things like "45 minutes before" and "15 hours before" are quite arbitrary, unlikely to be used, and less useful than 0 minutes. Personally, if I ran the Sunbird project, I would do away with several menu items, as Bug 391673 suggests, but add 0 minutes.

You are probably right in saying that "0 minutes" is a more informative label. Not sure why I thought of "On time" first. (IMO using "The moment the event starts" as does the rest of the application would be too long, making the menu ugly".)

I've submitted a new patch changing "On time" to "0 minutes".

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 374652 [details] [diff] [review]
Changing "On time" to "0 minutes"

I'd like Bryan to take a look at the list and tell us which entries we should best add/remove.

Bryan, maybe you could also tell us what best to do for bug 391673.

Brennan, maybe you could fix that bug too while you are at it. Also, you need to add the change to calendary-event-dialog.xul also in calendar-summary-dialog.xul. I prefer to align the locale entity to event.reminder.0minutes.before.label

r=philipp anyway since this is only a nit. I'd appreciate if you could upload a new patch though.

Bonus points for including a checkin comment (i.e with hg qref -u "name <email>" -m "Fix bug NNN - DESC. r=philipp,ui-r=clarkbw")

Comment 7

[Brennan Vincent]

Philipp,

I'm not sure who "Bryan" is. In any case, once it's decided how to resolve bug 391673 I'd be glad to fix that one "while I'm at it".

Sorry, but I'm not clear on what a checkin comment is or how to go about adding one.

~Brennan

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Brennan: Bryan is Bryan Clark, the shared Thunderbird+Calendar UI Expert. He does ui-reviews.

Don't worry about the checkin comment, its not that important. I'll send you some info per email about why its nice for me :-)

Comment 9

[Brennan Vincent]

Attachment: Patch incorporating Philipp's comments

Comment 10

[Bryan Clark (DevTools PM) [:clarkbw]]

I'm not sure about the 0 minutes display but I don't have anything better in mind.  The "0 minutes before" is just a bit too programatic and cold, it makes me think people would have to read it and realize it means "at event time". This feeling could just come from the fact that the word "before" is used and 0 minutes isn't before the event.  Any other suggestions?  "On time" doesn't seem quite right either.

As for shrinking the list I'll comment in that bug with the options I think should be default.  15 hours before seems ripe for removal to me.

Comment 11

[Brennan Vincent]

Why not go with what you just said -- "At event time"?

Comment 12

[Michiel van Leeuwen (email: mvl+moz@)]

(I don't know exactly how the dialog looks, but) what about "when the event starts"?

Comment 13

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 374981 [details] [diff] [review]
Patch incorporating Philipp's comments

with a change of the 0 minute label to be "At event time" this looks good to me.

Comment 14

[Stefan Sitter [:ssitter]]

Does "At event time" works for tasks too? If not it probably need to be changed at runtime if the dialog is in task or event layout.

Comment 15

[Bryan Clark (DevTools PM) [:clarkbw]]

No, "At event time" probably doesn't work for tasks.  We probably need something like "At completion time" or something related to the task deadline.  Any suggestions?

Comment 17

[Martin Schröder [:mschroeder]]

(In reply to comment #15)
> No, "At event time" probably doesn't work for tasks.  We probably need
> something like "At completion time" or something related to the task deadline. 
> Any suggestions?

Bryan, how about "On Time" for both? Philipp, what's your opinion on comment#13 ff.?

Comment 18

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Hmm...I guess we could just let the labels differ between events and tasks. In general I agree we need this, I'm fine with any wording Bryan agrees on.

Comment 19

[Martin Schröder [:mschroeder]]

(In reply to comment #17)
> (In reply to comment #15)
> > No, "At event time" probably doesn't work for tasks.  We probably need
> > something like "At completion time" or something related to the task deadline. 
> > Any suggestions?
> 
> Bryan, how about "On Time" for both? Philipp, what's your opinion on comment#13
> ff.?

Forget my suggestion because it has already been discussed.

Comment 21

[jsherk]

In Lightning 0.9 for TB2, you simply could set the Reminder to 0 minutes/days/hours before/after the event starts/ends.

You could simply allow 0 as a choice to duplicate this functionality in the custom menu, or you could put it in the drop down list of Reminder in between No Reminder and 5 Minutes Before.

Comment 22

[Michael de Raadt]

This bug continues for Lightning 1.0b1 but is now worse as you cannot create an alarm with 0 minutes. The custom alarm does not allow you to set 0 as the number of minutes/hours/days. I agree with the comments above that there should be the option to create a reminder alarm without having to create a custom alarm, but you should also be able to create a custom alarm with 0 units.

Comment 23

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: [short,2patches] Re-allow 0 minutes

Confirmed. I'm going to fix the 0 minute issue on this bug since its trivial. To bad we didn't notice this before. An extension that fixes this issue for 1.0b1 could easily be created though.

Putting this on my review queue so I don't forget to check in this bug.

Comment 24

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

We should add a mozmill testcase for this.

Comment 26

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/0acc8bcd23f3> and <http://hg.mozilla.org/comm-central/rev/18af48d5f0c8>

-> FIXED