296893 - Replace old event dialog with new one

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Stuart Parmenter]

I'd like to see the old event dialog replaced in Sunbird with the new one that
lightning is using.  It is as-functional as the old one and will be getting the
most work done to it in the near future.  This would allow resolution of a lot
of old bugs with old dialog.

Comment 1

[gekacheka]

(In reply to comment #0)
> I'd like to see the old event dialog replaced in Sunbird with the new one
> that lightning is using.  

Do you mean replace
http://lxr.mozilla.org/mozilla/source/calendar/resources/content/eventDialog.xul
with
http://lxr.mozilla.org/mozilla/source/calendar/base/content/calendar-event-dialog.xul

I ask, because the following statement is clearly premature for this code:

> It is as-functional as the old one 

The version at the base/content link is clearly much less functional: it doesn't
handle todos.  Also, it doesn't check for illegal dates, it doesn't handle
exceptions, no email alarms, no categories, no status, no private, etc.

> and will be getting the
> most work done to it in the near future.  This would allow resolution of a lot
> of old bugs with old dialog.

To which bugs are you referring?

The new code looks simpler at the moment, but this is partly because it has less
functionality and polish so it is not a fair comparison.  The new code
introduces problems, itself (e.g., can't remove some values, event description
too small, confusing attendees in two rows...).  

I agree the current sunbird event dialog design needs some improvements... one
issue is that the start/end dates are not visible while editing the recurrences.
 The most significant difference in the new code is that it separates the
recurrence and alarm into separate dialogs rather than tabs.  So the start/end
dates might be visible, depending on where the recurrence dialog pops up.  (On
the other hand if all the options are in dialogs, it takes more clicks to fill
them out that it would if they are in tabs, but that may be an acceptable tradeoff.)

(I have a design where the title, start date, end date, allday, and calendar
file are above the tabs for everything else.  It also addresses the issue of
making the dates visible while editing the recurrences, alarms, etc., but fewer
options are visible on each tab.)

(One ongoing issue with the event dialog is that there are too many options to
fit comfortably in one dialog.  Everyone wants the options they use frequently
to appear on the main event dialog/tab so they can see them all in one glance,
but not everyone uses the same options, so it is impossible to satisfy everyone
with one design.  For example, someone who frequently uses alarms and attendees
but not categories and descriptions will want alarms and attendees to appear in
the main event dialog in place of categories and descriptions, but someone else
who uses categories and descriptions would rather see those items in the main
event dialog.

A thought: Is there some way to make the event dialog design customizable in the
way Firefox toolbars are?  Most people would use the default to start, but those
who edit particular features frequently could customize the dialog so the
features they use are in the main event dialog, and the features they don't use
don't clutter it.)

Anyway, simply replacing the sunbird event dialog with the lightning one looks
like a bad idea for sunbird at this time because it removes too much
functionality.  Maybe it would be better to discuss what parts of the designs to
merge.

Comment 2

[Stuart Parmenter]

(In reply to comment #1)
> The version at the base/content link is clearly much less functional: it
doesn't handle todos.

Todos will be a seperate dialog and sunbird should continue to use the existing
dialog for todos.


> Also, it doesn't check for illegal dates, it doesn't handle
> exceptions, no email alarms, no categories, no status, no private, etc.

The old dialog doesn't handle exceptions at all.
No one has yet told me what email alarms is supposed to do.  The current plan is
to support this via prefs.  I don't see this as a very important feature really.
There is currently no support anywhere for categories aside from setting them.
We don't do anything with status or private either.

These things are all features that would be nice to support, but supporting them
involves more than just including them in the event dialog.  We need to have an
understanding of how they work

> 
> > and will be getting the
> > most work done to it in the near future.  This would allow resolution of a lot
> > of old bugs with old dialog.
> 
> To which bugs are you referring?

Random bugs that I keep getting asked to review or look at or that are sitting
idle in bugzilla.

> 
> The new code looks simpler at the moment, but this is partly because it has less
> functionality and polish so it is not a fair comparison.  The new code
> introduces problems, itself (e.g., can't remove some values, event description
> too small, confusing attendees in two rows...).  

"Can't remove some values" -- huh?
"Event description is too small" -- how big should it be?
"confusing attendees in two rows" -- what?  attendees is currently just a
textfield of comma seperated email addresses....


> 
> I agree the current sunbird event dialog design needs some improvements... one
> issue is that the start/end dates are not visible while editing the recurrences.
>  The most significant difference in the new code is that it separates the
> recurrence and alarm into separate dialogs rather than tabs.  So the start/end

Alarms are in the main dialog...



> dates might be visible, depending on where the recurrence dialog pops up.  (On
> the other hand if all the options are in dialogs, it takes more clicks to fill
> them out that it would if they are in tabs, but that may be an acceptable
tradeoff.)

The checkbox will be going away shortly, which leaves you with the same number
of clicks...  Click set to edit is the same as clicking the recurrence tab. 
Clicking Accept is the same as clicking back to the main tab...


> 
> (I have a design where the title, start date, end date, allday, and calendar
> file are above the tabs for everything else.  It also addresses the issue of
> making the dates visible while editing the recurrences, alarms, etc., but fewer
> options are visible on each tab.)
> 
> (One ongoing issue with the event dialog is that there are too many options to
> fit comfortably in one dialog.  Everyone wants the options they use frequently
> to appear on the main event dialog/tab so they can see them all in one glance,
> but not everyone uses the same options, so it is impossible to satisfy everyone
> with one design.  For example, someone who frequently uses alarms and attendees
> but not categories and descriptions will want alarms and attendees to appear in
> the main event dialog in place of categories and descriptions, but someone else
> who uses categories and descriptions would rather see those items in the main
> event dialog.
> 
> A thought: Is there some way to make the event dialog design customizable in the
> way Firefox toolbars are?  Most people would use the default to start, but those
> who edit particular features frequently could customize the dialog so the
> features they use are in the main event dialog, and the features they don't use
> don't clutter it.)
> 

Making the dialog customizable in this way is a horrible user experience.  The
idea here is to keep a small, uncluttered dialog and seperate things out that
make sense and/or simply can't fit in the main dialog.  


> Anyway, simply replacing the sunbird event dialog with the lightning one looks
> like a bad idea for sunbird at this time because it removes too much
> functionality.  Maybe it would be better to discuss what parts of the designs to
> merge.

As I think I've pointed out above, you aren't actually losing any real
functionality.  The invalid date thing is a bug and should be resolved, but the
rest of your issues with the dialog seem somewhat invalid to me.  Feature
requests for using categories, status, private, etc in sunbird and lightning
seem like great bugs to file, but until they are actually used by the
applications in some way, they don't make any sense being in the dialog.

Comment 3

[gekacheka]

Shared event/task dialog:

> Todos will be a seperate dialog and sunbird should continue to use the
> existing dialog for todos.

Tasks and events share many fields, and there are distinct advantages
to being able to switch from one to the other in the same dialog:
* When the user drags or pastes text into calendar, it starts up an
  event dialog.  If the user wanted a todo, they can just change the
  pull down.
* A user may select one type, say task, when they create it, and
  later decide it should be the other type, say, event. 
  With separate dialogs, the user has to reenter all the data
  (bug 277731).


Exceptions:

>
> The old dialog doesn't handle exceptions at all.
>

The Sunbird 0.2 dialog does handle exceptions (skipped recurrences).
(It does not handle rescheduling/postponements, maybe that's what you
mean?  An initial fix, assuming libical supports RDATE as well as
EXDATE, is to just add another date list for the RDATEs next to the
exception dates.)


Email alarms:

> No one has yet told me what email alarms is supposed to do.  The
> current plan is to support this via prefs.

Sends an email message instead of triggering a sound.  The email
message is sent to the address given, and contains at least the title
(subject), the start/end dates, the location, and the description.
Sunbird0.2 code is in calendarMail.js.

Some people use it to send reminders as text messages/pages, so it is
not necessarily to the user's main email address.  I think some
managers use it to send reminders to their supervisees, so it is not
the same for every event/task.  (Now that there is an attendees list,
it might be a good idea to add a checkbox option to send to all
attendees also, but that's low priority since the user can workaround
by copy and paste.)


Categories:
>
> There is currently no support anywhere for categories aside from setting them.
>
They may be used for sorting.  There also has been work to associate
event border colors with categories.  (bug 171657, bug 273394,
bug 268084)

>
> We don't do anything with status or private either.
>

Private:

Currently used by the printing code, which has the option to omit
private events.  Also to be used by publish (bug 215975).

Status:

Used for styling and sorting.
Cancelled/Completed todos are styled with line-through.
(Tentative appointments could be styled in grey and/or italic/oblique.
Needs-Action could be styled in bold or underline.)


Customizable dialog: 

> Making the dialog customizable in this way is a horrible user
> experience.

(I don't see that it is any worse a user experience than customizable
toolbars.  Casual users need never use it.  For frequent users, like
a busy receptionist who manages appointments for a (say, law or medical)
office, being able to adjust the interface to the work is much better
than forcing the work into a one-design-fits-all interface.)


Conclusion:

> As I think I've pointed out above, you aren't actually losing any real
> functionality. ... until they are actually used by the
> applications in some way, they don't make any sense being in the dialog.

As I think I've pointed out, the functionality is used by sunbird, not as
fallow as you suggested.  If there are other reasons to omit certain features in
progress from a particular release, please discuss.  Are there plans for an
immanent release requiring a feature freeze/backout?  

(Another thought:  is there anyway to log what features people are actually
using?  I guess at a calendar site like (Axentra?) you might be able run a
script that summarized what attributes were set without revealing what they were
set to (for privacy).  That would reveal what task/event features are used, but
not what preference features are used.)


p.s. nits

> "Can't remove some values" -- huh?
> 
sorry, that was too vague.  The problem is is easy to fix, it is
just code like
  if (value)
     event.setProperty(propertyName, value);
which means if you clear value to "", it won't clear the event property.

> "Event description is too small" -- how big should it be?
> 

The event description for a meeting or seminar may be an agenda or
abstract.  It should be large enough to edit multi-line text, which
means it should be tall enough to support scrollbars.  (When I last
looked, it was only 2 lines tall, which is too short; 4 lines is
probably the minimum, esp. since one line is lost if a horizontal
scrollbar appears.)

Another way of using the description is to past info from a web page
or email into the description and then copy parts relevant parts to
the title, location, date, etc. into the appropriate fields.

> "confusing attendees in two rows" -- what?  attendees is currently just a
> textfield of comma seperated email addresses....

Currently base/content/calendar-event-dialog.xul has 
 87          <row align="center">
 88             <label value="Attendees" class="label"/>
 89             <textbox id="event-attendees"/>
 90           </row>
 91 
...
119           <row align="center" hidden="true">
120             <label value="Attendees" class="label"/>
121             <hbox align="center">
122               <label value="none"/>
123               <spacer flex="1"/>
124               <button disabled="true" label="set..."/>
125             </hbox>
126           </row>
127 

Comment 4

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

(In reply to comment #3)
> * A user may select one type, say task, when they create it, and
>   later decide it should be the other type, say, event. 
>   With separate dialogs, the user has to reenter all the data
>   (bug 277731).

There might be other ways to switch from task to event, like a context menu
option and/or some drag-and-drop action. Everything to make the event dialog
simpler!

> Email alarms:

Sure, there can be situations where every event has a different email address to
send mail to, but do we really want to support that? How many users use it?
Again, let's keep the dialog as simple as possible.

Comment 5

[Stuart Parmenter]

gekacheka: I suggest looking at trunk sunbird for your comparison.  My comments
are based on it and not 0.2.

Comment 6

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

Point is, trunk has a number of regressions, for example exceptions not working.
The question is: do we want to fix them, or do we want to go for the new dialog
and fix the problems there?
I sure want to go back to one dialog one day. Keeping this forked makes no
sense. Is now the right time to unfork?

Comment 7

[Simon Paquet [:sipaq]]

(In reply to comment #6)
> I sure want to go back to one dialog one day. Keeping this forked makes no
> sense. Is now the right time to unfork?

From a user and QA perspective it would be good to unfork. This means much more
testing (especially when Lightning does his first release) and more eyes on the
code.

Comment 8

[gekacheka]

Bug #298102 explores a merged item-dialog, merging code from both the lightning
dialog and the sunbird dialog, and provides patches for the merged code and
integration into sunbird to try it out...details there.

Comment 9

[Joey Minta]

Attachment: move sunbird to the new dialog v1

This is the full patch to move Sunbird to the new dialog.  It moves the options the old dialog offered into the new one in order to keep Sunbird users happy as well as moving over some of the warnings that the old dialog gave regarding improper input.  It is largely based on the recommendations beltzner made in Bug 298102 comment #32.  The main change from the ascii art there is the removal of attendees from the main screen.  This was done after and IRC discussion with both dmose and beltzner.

Comment 10

[Joey Minta]

Attachment: move sunbird to the new dialog v1 -w

Same as above, but -w for easier review.

Comment 11

[Joey Minta]

Attachment: screenshots of the new dialog

Comment 12

[Calum Mackay]

Looks great Joey. I do wonder about the "set..." label for the Repeat controls. It's not really obvious that it's part of the Repeat function, and "set..." isn't very clear.

Perhaps either visually tie the button more obviously to the Repeat checkbox, or just change the button label to something more obviously connected to Repeat?

Comment 13

[Joey Minta]

(In reply to comment #12)
> Looks great Joey. I do wonder about the "set..." label for the Repeat controls.
> It's not really obvious that it's part of the Repeat function, and "set..."
> isn't very clear.
> 
> Perhaps either visually tie the button more obviously to the Repeat checkbox,
> or just change the button label to something more obviously connected to
> Repeat?
> 
I'm going to wait for some feedback from beltzner, he may want to comment on that topic.  In general, I left as much of the current Lightning dialog code as possible, which includes the 'Set...' button.  Depending on what my reviewers think, this may be better fodder for a follow-up bug.

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 208484 [details] [diff] [review]
move sunbird to the new dialog v1 -w

Great work so far; a couple of suggested tweaks & nits:

1. Clicking on More >> / Less << shouldn't change the width of the dialog. Some layout changes (which also clean up the look of the dialog a little, IMO) can make it possible to get away with a narrower dialog in the "More >>" section ..:

.                                                         .
:                                                         :
| Description: [                                        ] |
|              [                                        ] |
|                                                         |
| Attendees:   [                   ]  Alarm    [none  |V] |
|              [                   ]  Priority [none  |V] |
|              [                   ]  Privacy  [Public|V] |
|                     [Add] [Remove]  Status   [none  |V] |
|                                                         |
| URL: [                                                ] |
|                                                         |
| --------------------------------------------------------|
|                                       [ OK ] [ Cancel ] |
'---------------------------------------------------------'

This will still need to be wider than the basic (ie: non-extended) dialog, which should actually give you room to align the "Repeat" controls with the "All Day" button, associating them more closely with the controls for setting start/end times on the event.

2. The from/to fields seem out of alignment with the title/location/calendar fields

3. "Item" doesn't add anything to the label "item type", imo; "type" would work fine, or even nothing at all, just the drop-down.

4. Perhaps use [Add] [Remove] [Edit] buttons for attendees instead of [Edit Attendees] (as shown in ASCII art above)?

5. Someone should check me on this, but I believe that the [Less <<] button should bump down to the bottom of the panel in the large dialog.

Comment 15

[Stefan Sitter [:ssitter]]

I also want to add a comment ;-)

1) Yeah, I think it would be nice if the more/less button would be on the lower left corner like:

|---------------------------------------------------------|
| [ << Less ]                           [ OK ] [ Cancel ] |
'---------------------------------------------------------'

This should take up less space than having a own row for this button, and I think it is easier to find the button again after the dialog resized.

2) I think the checkboxes should be on the left of the text, like: [ ] All Day instead of All Day [ ]. (I know this is a leftover from the previous code)

3) The [Set...] button could be labeled e.g. with [Recurrence Settings...]. This makes the purpose more understandable, but does not match together with the [ ] Repeat checkbox.

Comment 16

[Joey Minta]

Attachment: move sunbird to the new dialog v2

Full patch updated to beltzner's comments.

Comment 17

[Joey Minta]

Attachment: move sunbird to the new dialog v2 -w

Updated patch -w for easier reviewing

Comment 18

[Joey Minta]

Attachment: screenshots of the new dialog v2

New screenshots.  No real surprises here.  It looks like beltzner's ascii art.

Comment 19

[Stuart Parmenter]

Comment on attachment 208858 [details] [diff] [review]
move sunbird to the new dialog v2 -w

Good work.  A few nit picks though:

Can you make Calendar and Category line up with To and Privacy in the more area.

I would also suggest putting more spacing between the attendees stuff and the privacy, priority etc parts.  Maybe even a vertical line.

Aside from that, I think this looks pretty good.
r=pavlov with the UI nits  fixed

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 208858 [details] [diff] [review]
move sunbird to the new dialog v2 -w

ui-r=beltzner given pavlov's nits. Some breathing room for those labels along the right side of the "attendees" field will be needed for l10n purposes. Maybe another 20px or so in width?

Comment 21

[Joey Minta]

Patch checked in.  Please file any regressions from this bug as separate bugs, blocking this one.