Closed Bug 296893 Opened 19 years ago Closed 19 years ago

Replace old event dialog with new one

Categories

(Calendar :: Sunbird Only, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: pavlov, Assigned: jminta)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 3 obsolete files)

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.
(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.
OS: Windows XP → All
Hardware: PC → All
(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.
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
(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.
gekacheka: I suggest looking at trunk sunbird for your comparison. My comments are based on it and not 0.2.
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?
(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.
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.
Attached patch move sunbird to the new dialog v1 (obsolete) (deleted) — Splinter Review
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.
Assignee: mostafah → jminta
Status: NEW → ASSIGNED
Attached patch move sunbird to the new dialog v1 -w (obsolete) (deleted) — Splinter Review
Same as above, but -w for easier review.
Attachment #208484 - Flags: second-review?
Attachment #208484 - Flags: first-review?(pavlov)
Attached image screenshots of the new dialog (obsolete) (deleted) —
Attachment #208484 - Flags: second-review? → second-review?(beltzner)
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?
(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 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.
Attachment #208484 - Flags: second-review?(beltzner) → second-review-
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.
Full patch updated to beltzner's comments.
Attachment #208483 - Attachment is obsolete: true
Updated patch -w for easier reviewing
Attachment #208484 - Attachment is obsolete: true
Attachment #208858 - Flags: second-review?(beltzner)
Attachment #208858 - Flags: first-review?(pavlov)
Attachment #208484 - Flags: first-review?(pavlov)
Attached image screenshots of the new dialog v2 (deleted) —
New screenshots. No real surprises here. It looks like beltzner's ascii art.
Attachment #208485 - Attachment is obsolete: true
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
Attachment #208858 - Flags: first-review?(pavlov) → first-review+
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?
Attachment #208858 - Flags: second-review?(beltzner) → second-review+
Patch checked in. Please file any regressions from this bug as separate bugs, blocking this one.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Depends on: 324286
Depends on: 324312
Depends on: 327832
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: