Improvements to the calendar list tree

RESOLVED FIXED in 6.2

Status

defect
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Fallen, Assigned: Fallen)

Tracking

(Blocks 1 bug)

Lightning 6.2
Dependency tree / graph

Details

Attachments

(1 attachment, 1 obsolete attachment)

Here are some improvements and fixes to the calendar list tree. I noticed that when clearing calendars the row count is not set correctly, so there is a fix for that as well.

These changes are needed for the autodiscovery patch.
Posted patch Fix - v1 (obsolete) β€” β€” Splinter Review
Attachment #8963014 - Flags: review?(Mozilla)
Comment on attachment 8963014 [details] [diff] [review]
Fix - v1

Review of attachment 8963014 [details] [diff] [review]:
-----------------------------------------------------------------

#ThereIsAlwaysOneStyleNit:
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/comm/calendar/base/content/widgets/calendar-list-tree.xml:620:29 | Multiple spaces found before 'colorMatch'. (no-multi-spaces)
from [1]

>+      <field name="cycleDebounce">200</field>
--------------
>+            if (this.cycleDebounce) {
>+                if (this.mCycleTimer) {
>+                    clearTimeout(this.mCycleTimer);
>+                }
>+                this.mCycleTimer = setTimeout(this.cycleCellCommit.bind(this), 200);
>+            } else {
>+                this.cycleCellCommit();
>             }

Here I'm not sure what the "if (this.cycleDebounce)" check is for, since the value is set fixed to 200 and never changed.
Should you not just drop it and then use:
  this.mCycleTimer = setTimeout(this.cycleCellCommit.bind(this), this.cycleDebounce);
?

r+ with these two points addressed and if there is no unexpected fallout from the rest of the tryrun (OSX and Windows still running)

[1]: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8abe12d496edddf8951151a3029a362d8cab9d16
Attachment #8963014 - Flags: review?(Mozilla) → review+
(In reply to Markus Adrario [:Taraman] from comment #2)
> #ThereIsAlwaysOneStyleNit:
> TEST-UNEXPECTED-ERROR |
> /builds/worker/checkouts/gecko/comm/calendar/base/content/widgets/calendar-
> list-tree.xml:620:29 | Multiple spaces found before 'colorMatch'.
> (no-multi-spaces)
> from [1]
Thanks for doing the try run, fixed this!


> 
> >+      <field name="cycleDebounce">200</field>
> --------------
> >+            if (this.cycleDebounce) {
> >+                if (this.mCycleTimer) {
> >+                    clearTimeout(this.mCycleTimer);
> >+                }
> >+                this.mCycleTimer = setTimeout(this.cycleCellCommit.bind(this), 200);
> >+            } else {
> >+                this.cycleCellCommit();
> >             }
> 
> Here I'm not sure what the "if (this.cycleDebounce)" check is for, since the
> value is set fixed to 200 and never changed.
> Should you not just drop it and then use:
>   this.mCycleTimer = setTimeout(this.cycleCellCommit.bind(this),
> this.cycleDebounce);
> ?
This is a new feature. In the autodiscovery patch I set the value to 0 for that instance of the binding so it happens immediately.
Posted patch Fix - v2 β€” β€” Splinter Review
Attachment #8963014 - Attachment is obsolete: true
Attachment #8967874 - Flags: review+
Attachment #8967874 - Flags: approval-calendar-beta+

Comment 5

a year ago
Pushed by mozilla@kewis.ch:
https://hg.mozilla.org/comm-central/rev/9c3be58fe2a9
Improvements to the calendar list tree. r=Taraman
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.