272732 - New event/edit event dialog updates

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Stuart Parmenter]

Updating the event dialog to use the new calendar APIs as well as a revamp'd
dialog by Matthew Willis.  A few things are still needed like recurance and some
alarm work, but this gets things being added to a calendar.

Comment 1

[Stuart Parmenter]

Attachment: inital pass

Comment 2

[Mike Shaver (:shaver emeritus)]

Comment on attachment 167625 [details] [diff] [review]
inital pass

>     getProperty: function (aName) {
>-        return this.mProperties.getProperty(aName);
>+        try {
>+            return this.mProperties.getProperty(aName);
>+        } catch (e) {
>+            return undefined;
>+        }
>     },

I think we'd rather return null here, if indeed we want to suppress this error
in that fashion.

>     setProperty: function (aName, aValue) {
>         if (this.mImmutable)
>@@ -153,7 +157,10 @@
>     deleteProperty: function (aName) {
>         if (this.mImmutable)
>             throw Components.results.NS_ERROR_FAILURE;
>-        this.mProperties.deleteProperty(aName);
>+        try {
>+            this.mProperties.deleteProperty(aName);
>+        } catch (e) {
>+        }
>     }
> };

Is the intent to silently fail to delete an item that isn't present?  If so,
please confer with Vlad to make sure that that's the desired behavior for this
API, and stick a comment in the IDL somewhere.

>+      case "popup":
>+        showElement("alarm-length-field");
>+        showElement("alarm-length-units");
>+        hideElement("alarm-box-email");
>+        break;
>+      case "popupAndSound":
>+        showElement("alarm-length-field");
>+        showElement("alarm-length-units");
>+        hideElement("alarm-box-email");
>+        break;

Can merge these into a single block with two case labels.

Seems like we could generally merge the popup and popupAndSound cases
throughout
the code, but that's not what this patch is about.

Nothing here looks like a show-stopper to me.  Time to wreck some stuff!

Comment 3

[Stuart Parmenter]

checked in.  i'll leave this open for additional work on this.

Comment 4

[Matthew (lilmatt) Willis]

Attachment: rev1 - trunk - further updates to eventDialog - more works

Comment 5

[Stuart Parmenter]

Comment on attachment 169851 [details] [diff] [review]
rev1 - trunk - further updates to eventDialog - more works

with the changes we talked about on irc (style stuff mostly) r=

Comment 6

[Matthew (lilmatt) Willis]

(In reply to comment #5)
> (From update of attachment 169851 [details] [diff] [review] [edit])
> with the changes we talked about on irc (style stuff mostly) r=
> 

checked in

Comment 7

[Matthew (lilmatt) Willis]

Attachment: rev2 - trunk - fixes window title

Comment 8

[Matthew (lilmatt) Willis]

(In reply to comment #7)
> Created an attachment (id=169853) [edit]
> rev2 - trunk - fixes window title
> 

checked in

Comment 9

[Matthew (lilmatt) Willis]

Attachment: rev3 - trunk - more fixes, more refactoring, less redundant code

Comment 10

[Matthew (lilmatt) Willis]

Attachment: rev4 - trunk - more fixes, more refactoring, less redundant code, more work on event vs. todo

Comment 11

[Matthew (lilmatt) Willis]

(In reply to comment #10)
> Created an attachment (id=170157) [edit]
> rev4 - trunk - more fixes, more refactoring, less redundant code, more work on
> event vs. todo

checked in

Comment 12

[Matthew (lilmatt) Willis]

Attachment: rev5 - trunk - incorporates changes we talked about on IRC

Comment 13

[Stuart Parmenter]

Comment on attachment 170179 [details] [diff] [review]
rev5 - trunk - incorporates changes we talked about on IRC

r= with the title fixes we talked about on irc

Comment 14

[Matthew (lilmatt) Willis]

(In reply to comment #13)
> (From update of attachment 170179 [details] [diff] [review] [edit])
> r= with the title fixes we talked about on irc
> 

Checked in with the fixes:
16:33 <stuart> your changeTitleBar thing doesn't have a var title anywhere
16:33 <stuart> add var title; at the top of that function and remove it fro 
                mprocessComponentType

Comment 15

[Matthew (lilmatt) Willis]

Attachment: rev6 - trunk - more todo and UI work

Comment 16

[Matthew (lilmatt) Willis]

Attachment: rev7 - trunk - more UI work, adds support for event priority and todo statuses

Comment 17

[Matthew (lilmatt) Willis]

(In reply to comment #16)
> Created an attachment (id=170242) [edit]
> rev7 - trunk - more UI work, adds support for event priority and todo statuses

Checked in.

Comment 18

[Matthew (lilmatt) Willis]

Attachment: rev8 - trunk - fixes to alarms for event vs. todo, etc.

Comment 19

[Matthew (lilmatt) Willis]

(In reply to comment #18)
> Created an attachment (id=170276) [edit]
> rev8 - trunk - fixes to alarms for event vs. todo, etc.

Checked in.

Comment 20

[Matthew (lilmatt) Willis]

Attachment: rev9 - trunk - "OK" stuff now works for both events and todos

Comment 21

[Matthew (lilmatt) Willis]

Comment on attachment 170309 [details] [diff] [review]
rev9 - trunk - "OK" stuff now works for both events and todos

r=dmose via irc

Comment 22

[Matthew (lilmatt) Willis]

(In reply to comment #20)
> Created an attachment (id=170309) [edit]
> rev9 - trunk - "OK" stuff now works for both events and todos

Checked in.

Comment 23

[Matthew (lilmatt) Willis]

Attachment: rev10 - minor fixes to xul/js for recurrence

Comment 24

[Matthew (lilmatt) Willis]

(In reply to comment #23)
> Created an attachment (id=170368) [edit]
> rev10 - minor fixes to xul/js for recurrence

Checked in.

Comment 25

[Matthew (lilmatt) Willis]

Attachment: rev11 - trunk - fixes day checkboxes in recurrence

Comment 26

[Matthew (lilmatt) Willis]

(In reply to comment #25)
> Created an attachment (id=170398) [edit]
> rev11 - trunk - fixes day checkboxes in recurrence
 
Checked in.

Comment 27

[Matthew (lilmatt) Willis]

Attachment: rev12 - fixes UI issues. Also fixes issues storing recurrence to storage

Comment 28

[Matthew (lilmatt) Willis]

Attachment: rev13 - trunk - more UI polish - listbox "remove" buttons now disable/enable properly

Comment 29

[Mike Shaver (:shaver emeritus)]

Comment on attachment 170632 [details] [diff] [review]
rev13 - trunk - more UI polish - listbox "remove" buttons now disable/enable properly

Local style omits braces on single-line thens and elses.  With that changed,
r=shaver.

Comment 30

[Matthew (lilmatt) Willis]

(In reply to comment #29)
> (From update of attachment 170632 [details] [diff] [review] [edit])
> Local style omits braces on single-line thens and elses.  With that changed,
> r=shaver.

rev13 checked in with those changes

Comment 31

[Matthew (lilmatt) Willis]

Attachment: rev14 - trunk - patch standardizes on use of get/setFieldValue, and removes many global vars (ie: gEvent)

Comment 32

[Mike Shaver (:shaver emeritus)]

Comment on attachment 170680 [details] [diff] [review]
rev14 - trunk - patch standardizes on use of get/setFieldValue, and removes many global vars (ie: gEvent)

r=shaver with IRC comments

Comment 33

[Matthew (lilmatt) Willis]

(In reply to comment #32)
> (From update of attachment 170680 [details] [diff] [review] [edit])
> r=shaver with IRC comments

Checked in (w/ comments and conflict fixes)

This patch breaks the ability to switch from Event to Todo in the dialog.
More back end work needs to happen to support that.

Comment 34

[Dan Mosedale (:dmosedale, :dmose)]

As far as I can tell, this bug itself is done.  We can use individual bugs for remaining things that need to be tweaked.