1449464 - Improvements to the calendar list tree

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Philipp Kewisch [:Fallen][📱📆]]

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.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v1

Comment 2

[Markus Adrario [:Taraman]]

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

Comment 3

[Philipp Kewisch [:Fallen][📱📆]]

(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.

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v2

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9c3be58fe2a9
Improvements to the calendar list tree. r=Taraman

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

https://hg.mozilla.org/releases/comm-beta/rev/2d0822c5a149