467069 - consolidate decorated-month-view-binding and decorated-multiday-view-binding

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Berend Cornelius]

The two bindings "calendar-decorated-month-view" and "calendar-decorated-multiday-view" share a lot of code that can be consolidated.

Comment 1

[Berend Cornelius]

Attachment: [checked in] patch v. #1

I added a parent binding calendar-decorated-month-view-parent that contains all common methods, handlers, properties and that both bindings extend to.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 350444 [details] [diff] [review]
[checked in] patch v. #1

>+            <!-- this property may be overridden by the
>+                descendent classes if neeeded  -->
>+            <property name="weeksInView">
>+                <getter><![CDATA[
>+                ]]></getter>
>+                <setter><![CDATA[
>+                ]]></setter>
>+            </property>
>+
I'd make this property do at least /something/. weeksInView could return 0 in the getter case and |val| in the setter case.


>+                        if (getPrefSafe(weekPrefix+prefNames[i])) {
spaces around operators, please check for other style errors while you are at it.

r=philipp

Comment 3

[Berend Cornelius]

Worked in the comments of philipp and pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/fac88ec64ee0
->fixed

Comment 4

[Andreas Treumann]

I checked this issue and found a minor bug:

STR:
====

- switch to multiweek or month view
- move mouse over a day with many events (scroll bar in day box should be visible)
- scroll the mouse wheel

Result:
=======

- the calendar view scrolls to the next week

Expected result:
================

- the scrolling should happens in the day box

Comment 5

[Berend Cornelius]

Attachment: [checked in] patch v. #2

This patch should fix it. As I investigated I could not see that my patch v. #1 was the culprit of this flaw. I did not take the time which was the actual reason for this but I find that the patch attached should be a clean solution for the problem.

Comment 6

[Berend Cornelius]

Comment on attachment 351164 [details] [diff] [review]
[checked in] patch v. #2

Andreas tested this patch and found it was Ok.

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 351164 [details] [diff] [review]
[checked in] patch v. #2

Looks good, r=philipp

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Please fix the missing space before the {

Comment 9

[Berend Cornelius]

pushed patch v. #2 to comm-central:

http://hg.mozilla.org/comm-central/rev/f1771043d32b

-> fixed

Comment 10

[Andreas Treumann]

Checked in lightning and sunbird build 20081207 -> VERIFIED.