335106 - [RFE] Ability to reach Categories manager while creating/editing an event

Status: VERIFIED FIXED

(Calendar :: Internal Components, enhancement)

Description

[Cael]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a1) Gecko/20060418 Mozilla Sunbird/0.3a1+

Often the user doesn't realize they need to create a new Category until they're in the middle of creating an event and none of their currently-existing Categories are appropriate (this is especially true for users running Sunbird on multiple machines).

Because the File Menu isn't available while creating an event (under Windows, at least) the user must close the Event window, open Tools | Options..., create a Category, close the Options window, then edit their event to reflect the appropriate Category.

A button named 'Edit Categories' next to the Category dropdown would be optimal.

Reproducible: Always

Comment 1

[Joey Minta]

Attachment: add "New Category..." option

This feels like a significant usability win to me.  I agree completely that users can easily enter the dialog prior to having created the category that they want.  This patch adds a 'New Category...' option at the end of the dialog.  This opens the New Category dialog from the preferences panel, so we have consistency there.  It then adds the category to the list and selects it, when the user is done.

Comment 2

[Michael Büttner (no reviews TFN)]

I would suggest that we first define the requirements and carefully prepare the appropriate specifications and then start implementing the agreed features instead of spinning off feature implementations at will. this would mean that we should first have the up-to-date specification available at the wiki and then start the implementation of the missing features [see my comment on bug #338215 comment #6].

Again, I don't have a problem with this specific feature but feelings about a usability win shouldn't be the only criteria for implementing features. In this case it means that I'm not sure that the event dialog is the right place for editing the categories. I would suggest that we first agree on a final specification available at the wiki and then start implementing the missing features.

Comment 3

[Joey Minta]

(In reply to comment #2)
> I would suggest that we first define the requirements and carefully prepare the
> appropriate specifications and then start implementing the agreed features
> instead of spinning off feature implementations at will. this would mean that
> we should first have the up-to-date specification available at the wiki and
> then start the implementation of the missing features [see my comment on bug
> #338215 comment #6].
In my mind, there's 2 different workflows at play in your comment.  For larger feature landings, I definitely think planning/specs are appropriate prior to implementation.  For smaller changes, I think specs are overkill and will block too much.  This definitely feels in the smaller change realm.

> 
> Again, I don't have a problem with this specific feature but feelings about a
> usability win shouldn't be the only criteria for implementing features. In this
> case it means that I'm not sure that the event dialog is the right place for
> editing the categories. 
I do not intend for this patch to make the event dialog the primary place for editing categories.  That belongs in categories, as it is in Sunbird.  This patch merely addresses what seems to be a reasonably common difficulty within the current design.

Comment 4

[Michael Büttner (no reviews TFN)]

> In my mind, there's 2 different workflows at play in your comment.  For larger
> feature landings, I definitely think planning/specs are appropriate prior to
> implementation.  For smaller changes, I think specs are overkill and will
> block too much.  This definitely feels in the smaller change realm.
if we agree to first specify features and implement them according to the specification there's no such thing as smaller or larger features. everything would need to follow this process. otherwise we would first need to discuss whether a feature is to be considered large or small and therefore would need to follow the process or not. this seems to be totally confusing. i can't see a problem what's wrong with following previously agreed specifications. i find it extremely obvious that constantly adding ad-hoc features won't lead to a consistent user interface, therefore i strongly suggest that we agree on a specification first.
my understanding was that we put a full-blown event-dialog proposal on the wiki in order to drive towards what has been presented there. of course we're still discussing the different bits and pieces of it, but as far as i remember there wasn't an option to edit the categories mentioned there. i have no problem with adding this to the dialog if people think that it should be necessary, but i think we *all* should stick to specifications. i can't see a reason why we use different measures here.

Comment 5

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

(In reply to comment #3)
> In my mind, there's 2 different workflows at play in your comment.  For larger
> feature landings, I definitely think planning/specs are appropriate prior to
> implementation.  For smaller changes, I think specs are overkill and will block
> too much.  This definitely feels in the smaller change realm.

I totally disagree. You should have specs before working on UI. Especially if the UI you are working on is part of UI that is currently in discussion. And in this case, for two reasons: the entire dialog and the idea of using tags instead of categories (tags should be dynamicly editable).

So instead of small changes like this, let's finish the UI discussions and spec-ing!

Comment 6

[Mike Shaver (:shaver emeritus)]

Comment on attachment 224637 [details] [diff] [review]
add "New Category..." option

>--- base/content/calendar-event-dialog.js	15 May 2006 22:02:53 -0000	1.47
>+++ base/content/calendar-event-dialog.js	7 Jun 2006 00:36:35 -0000
>@@ -221,17 +221,23 @@ function loadDialog(item)
>         }
>+        var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                            .getService(Components.interfaces.nsIStringBundleService);
>+        var props = sbs.createBundle("chrome://calendar/locale/calendar.properties");
>+        var newCategory = props.GetStringFromName("newCategory");

We don't have a helper function for getting a localized string?  We do now!  (Please add one and use it in both places in this patch.)

>+var gOldCatIndex = 0;

>+function categorySelect(aValue) {
>+    if (aValue != "##NEW") {
>+        gOldCatIndex = document.getElementById("item-categories").selectedIndex;
>+        return;
>+    }
>+
>+    // Make sure we don't leave 'New..' selected if they hit cancel
>+    document.getElementById("item-categories").selectedIndex = gOldCatIndex;

Is it possible for gOldCatIndex to be out of range here?  _Seems_ not, but I wonder what our failure mode is if it is...

>+    var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                        .getService(Components.interfaces.nsIStringBundleService);
>+    var props = sbs.createBundle("chrome://calendar/locale/calendar.properties");
>+    window.openDialog("chrome://calendar/content/pref/editCategory.xul",
>+                      "addCategory", "modal,centerscreen,chrome,resizable=no",
>+                      "", null, props.GetStringFromName("addCategory"));

Yay! More calGetLocalizedString() use already!

>+}
>+
>+function saveCategory(aName, aColor) {
>+    //Check to make sure another category doesn't have the same name
>+    var promptService = 
>+             Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
>+                       .getService(Components.interfaces.nsIPromptService);
>+    var pb2 = Components.classes["@mozilla.org/preferences-service;1"]
>+                        .getService(Components.interfaces.nsIPrefBranch2);
>+    var categoriesString = pb2.getComplexValue("calendar.categories.names", 

Do we like pref helpers too?  I bet we do!

Components.interfaces.nsISupportsString).data;
>+    var categoriesList = categoriesString.split( "," );
>+    for each (cat in categoriesList) {
>+        if (aName.toLowerCase() == cat.toLowerCase()) {
>+            var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                                .getService(Components.interfaces.nsIStringBundleService);
>+            var props = sbs.createBundle("chrome://calendar/locale/calendar.properties");
>+            var overwriteTitle = props.GetStringFromName("categoryOverwriteTitle");
>+            var overwrite = props.GetStringFromName("categoryOverwrite");

And again!  It's a localization-helper party and _everyone_'s invited!

>+            if (promptService.confirm(null, overwriteTitle, overwrite)) {
>+                var categoryNameFix = aName.toLowerCase().replace(' ','_');
>+                pb2.setCharPref("calendar.category.color."+categoryNameFix, aColor);

You defend above against things that differ only in case, but I suspect that "foo_bar"
and "foo bar" would stomp on each other's prefs, but not be shown as duplicates.  Do we want calMakePrefSafe(aName) for here and elsewhere?

What's our story like on non-ASCII in category names?

>+    var str = Components.classes["@mozilla.org/supports-string;1"]
>+                        .createInstance(Components.interfaces.nsISupportsString);
>+    str.data = categoriesList.join(',');
>+    pb2.setComplexValue("calendar.categories.names",
>+                        Components.interfaces.nsISupportsString, str);

(Wow, that's gross -- we really have to do that to store an array?  How do you feel about some JSON-serialization here?)

>+    if (aColor) {
>+        var categoryNameFix = aName.toLowerCase().replace(' ','_');
>+        pb2.setCharPref("calendar.category.color."+categoryNameFix, aColor);
>+    }
>+    var catList = document.getElementById("item-categories");
>+    var index = categoriesList.indexOf(aName);
>+    catList.insertItemAt(index, aName, aName);
>+    catList.selectedIndex = index;
>+}

>Index: base/content/calendar-event-dialog.xul
>===================================================================
>RCS file: /cvsroot/mozilla/calendar/base/content/calendar-event-dialog.xul,v
>retrieving revision 1.23
>diff -p -U8 -r1.23 calendar-event-dialog.xul
>+# Categories
>+categoryOverwrite=A category already exists with that name. \n Do you want to overwrite it?

"replace" is better than "overwrite" here, I think, in both key and value.

>+categoryOverwriteTitle=Warning: Duplicate name

r=shaver with that stuff fixed, or you can put up another patch and fight about it.

Comment 7

[Joey Minta]

Patch checked in.  Follow-up bug filed for investigating JSON serialization goodness.  There's already a bug on better dupe detection/common category name functions.

Comment 8

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060929 Sunbird/0.3