Add documentation to UI code and some UI code fixes

VERIFIED FIXED in 0.9

Status

Calendar
Calendar Views
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: Fallen, Assigned: Fallen)

Tracking

unspecified

Details

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
Created attachment 329129 [details] [diff] [review]
UI code fixes - v1

This patch moves around some UI code to a more appropriate place and adds some documentation. This was originally part of the alarms patch.
Attachment #329129 - Flags: review?(Berend.Cornelius)
(Assignee)

Updated

10 years ago
Blocks: 353492

Comment 1

10 years ago
Comment on attachment 329129 [details] [diff] [review]
UI code fixes - v1

>-    var calendarToUse = aCalendarToUse || aItem.calendar;
>-    var calendars = getCalendarManager().getCalendars({});
>-    var indexToSelect = 0;
>-    var index = -1;
>+var calendarToUse = aCalendarToUse || aItem.calendar;
>+var calendars = getCalendarManager().getCalendars({});
>+var indexToSelect = 0;
>+var index = -1;

Why did you remove the indentation here? I think it's not conforming to our style guide and IMO should also not be taken over there as we do declaration and definition in one step.

"CreateAlarm" is never used in the project as I noticed. I don't mind providing it anyway but is there an important member that we should deliver (like the "icalString" for your "createDuration()" function)?
Patch is a good cleanup. r=berend
Attachment #329129 - Flags: review?(Berend.Cornelius) → review+
(Assignee)

Comment 2

10 years ago
(In reply to comment #1)
> Why did you remove the indentation here? I think it's not conforming to our
> style guide and IMO should also not be taken over there as we do declaration
> and definition in one step.
Good catch, wasn't done on purpose.

> "CreateAlarm" is never used in the project as I noticed. I don't mind providing
> it anyway but is there an important member that we should deliver (like the
> "icalString" for your "createDuration()" function)?
> Patch is a good cleanup. r=berend
No parameters needed for now.


Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.9

Comment 3

9 years ago
Checked via mxr.mozilla.com -> VERIFIED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.