Closed Bug 467069 Opened 11 years ago Closed 11 years ago

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

Categories

(Calendar :: Calendar Views, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED

People

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

Details

Attachments

(2 files)

The two bindings "calendar-decorated-month-view" and "calendar-decorated-multiday-view" share a lot of code that can be consolidated.
I added a parent binding calendar-decorated-month-view-parent that contains all common methods, handlers, properties and that both bindings extend to.
Attachment #350444 - Flags: review?(philipp)
Attachment #350444 - Flags: review?(philipp) → review+
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
Worked in the comments of philipp and pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/fac88ec64ee0
->fixed
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Keywords: qawanted
Comment on attachment 351164 [details] [diff] [review]
[checked in] patch v. #2

Andreas tested this patch and found it was Ok.
Attachment #351164 - Flags: review?(philipp)
Attachment #350444 - Attachment description: patch v. #1 → [checked in] patch v. #1
Comment on attachment 351164 [details] [diff] [review]
[checked in] patch v. #2

Looks good, r=philipp
Attachment #351164 - Flags: review?(philipp) → review+
Please fix the missing space before the {
pushed patch v. #2 to comm-central:

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

-> fixed
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Attachment #351164 - Attachment description: patch v. #2 → [checked in] patch v. #2
Checked in lightning and sunbird build 20081207 -> VERIFIED.
Status: RESOLVED → VERIFIED
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.