Closed Bug 467546 Opened 11 years ago Closed 11 years ago

Consolidate calendar-multiday-view.xml and calendar-month-view.xml

Categories

(Calendar :: Calendar Views, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: berend.cornelius09, Assigned: berend.cornelius09)

Details

Attachments

(1 file)

Both mentioned views currently contain a lot of duplicate code that should be removed bit by bit.
Note: File calendar-multiday-view.xml is responsible for the day and week view and the file calendar-month-view.xml is responsible for the multiweekview and the month-view
Attached patch patch v. #1Splinter Review
This patch moves together the most obvious methods, properties and fieldnames of both bindings.
Assignee: nobody → Berend.Cornelius
Status: NEW → ASSIGNED
Attachment #350975 - Flags: review?(philipp)
Don't we already have a "parent" with calendar-view-core.xml?
"calendar-view-core.xml" only contains bindings for the event-boxes. I found it appropriate to create a new file and separate the view bindings from the content - namely the eventboxes - as they currently contain so much code.
I agree that the naming "calendar-view-core" is in this respect a little confusing and should maybe be changed.
Comment on attachment 350975 [details] [diff] [review]
patch v. #1

>-            const kWorkdaysCommand = "calendar_toggle_workdays_only_command";
>-            const kTasksInViewCommand = "calendar_toggle_tasks_in_view_command";
>-            const kShowCompleted = "calendar_toggle_show_completed_in_view_command";
>-            const kOrientation = "calendar_toggle_orientation_command";
This was done specifically to reduce the line lengths, I'd rather keep it.

>+calendar-base-view {
>+  -moz-binding: url(chrome://calendar/content/calendar-base-view.xml#calendar-base-view);
>   -moz-user-focus: normal;
> }
If you are not using this element directly (i.e <calendar-base-view .../> in the xml somewhere) then you don't need to create a css rule for it.

r=philipp
Attachment #350975 - Flags: review?(philipp) → review+
It would be nice to rename/refactor calendar-view-core.xml to something more fitting while you are at it.
Worked in the comments of Philipp.

>It would be nice to rename/refactor calendar-view-core.xml to something more
>fitting while you are at it.

I am aware that there is also lots of room for consolidation, but I would prefer to give this my dedicated attention in another issue.

>If you are not using this element directly (i.e <calendar-base-view .../> in
>the xml somewhere) then you don't need to create a css rule for it.

I was not using this element directly but as you can see I was also applying an attribute in this rule that I otherwise would have to apply twice on the derived children.


pushed to comm-central:

http://hg.mozilla.org/comm-central/rev/66a806479c6c

->fixed.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Checked 8via litmus test cases) in lightning and Sunbird build 20081216 -> VERIFIED
Status: RESOLVED → VERIFIED
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.