Closed
Bug 1569513
Opened 5 years ago
Closed 5 years ago
Tasks lists should only be filled when first visible
Categories
(Calendar :: Calendar Frontend, enhancement)
Calendar
Calendar Frontend
Tracking
(Not tracked)
RESOLVED
FIXED
68
People
(Reporter: darktrojan, Assigned: darktrojan)
References
(Blocks 1 open bug)
Details
(Keywords: perf)
Attachments
(2 files, 1 obsolete file)
|
(deleted),
patch
|
darktrojan
:
review+
|
Details | Diff | Splinter Review |
|
(deleted),
patch
|
darktrojan
:
review+
darktrojan
:
approval-calendar-esr+
|
Details | Diff | Splinter Review |
Like bug 1568723, the lists of tasks in the Today Pane and in the Tasks tab are filled at start-up, even if they are not visible. This should be delayed until the UI appears.
|
Assignee | |
Comment 1•5 years ago
|
||
new CalendarTaskTreeViewis now called only once per tree, instead of on every call torefresh.updateFiltergets called when each tree appears, and this callsrefresh. This was already happening for the Tasks tab (so the initial load is completely unnecessary!), and is now done at the appropriate time in the Today Pane by callingprepareCalendarToDoUnifinder.
Attachment #9081178 -
Flags: review?(paul)
|
||
Comment 2•5 years ago
|
||
Comment on attachment 9081178 [details] [diff] [review]
1569513-task-tree-lazy-1.diff
Review of attachment 9081178 [details] [diff] [review]:
-----------------------------------------------------------------
Nice! Thanks for the improvements to the task tree code. Changes look good and the task lists in the tab and the today pane were populated as expected when I tested this. r+ with a couple comments to address.
::: calendar/base/content/calendar-unifinder-todo.js
@@ +20,5 @@
> * Updates the applied filter and show completed view of the unifinder todo.
> *
> * @param aFilter The filter name to set.
> */
> async function updateCalendarToDoUnifinder(aFilter) {
"async" no longer needed here.
::: calendar/base/content/today-pane.js
@@ +116,3 @@
> }
> + if (todoIsVisible) {
> + prepareCalendarToDoUnifinder();
This "setTodayHeader" function now seems to be doing more than its name indicates. Might be worth renaming it or something similar to make things easier to follow.
Attachment #9081178 -
Flags: review?(paul) → review+
|
|
Assignee | |
Comment 3•5 years ago
|
||
Attachment #9081178 -
Attachment is obsolete: true
Attachment #9081475 -
Flags: review+
|
|
Assignee | |
Updated•5 years ago
|
Blocks: tb-startupperf
Keywords: perf
|
||
Updated•5 years ago
|
Target Milestone: --- → 70
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/ce4fab069c39
Load tasks lists only when first visible. r=pmorris DONTBUILD
|
|
||
Comment 6•5 years ago
|
||
I'm porting this patch to esr68 to fix bug 1581411. This also entails porting the patch from bug 1568723 (since this patch depends on it).
Attachment #9095682 -
Flags: review?(geoff)
Attachment #9095682 -
Flags: approval-calendar-esr?(geoff)
|
|
||
Comment 7•5 years ago
|
||
|
|
Assignee | |
Updated•5 years ago
|
Attachment #9095682 -
Flags: review?(geoff)
Attachment #9095682 -
Flags: review+
Attachment #9095682 -
Flags: approval-calendar-esr?(geoff)
Attachment #9095682 -
Flags: approval-calendar-esr+
|
|
||
Comment 8•5 years ago
|
||
TB 68.1.2 or TB 68.2 / Cal 7.0:
https://hg.mozilla.org/releases/comm-esr68/rev/b54334fbfbeaaa04595387264e8af98458379e62
Target Milestone: 70 → 7.0
|
|
||
Comment 9•5 years ago
|
||
That patch was wrong, see bug 1560547 comment #37. At least here:
https://hg.mozilla.org/releases/comm-esr68/rev/b54334fbfbeaaa04595387264e8af98458379e62#l4.58
Flags: needinfo?(paul)
You need to log in
before you can comment on or make changes to this bug.
Description
•