Closed
Bug 1582717
Opened 5 years ago
Closed 5 years ago
remove grid usage from comm/calendar/base/content/calendar-month-base-view.js
Categories
(Calendar :: Calendar Frontend, task)
Calendar
Calendar Frontend
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 71.0
People
(Reporter: khushil324, Assigned: khushil324)
References
Details
Attachments
(1 file, 1 obsolete file)
9.45 KB,
patch
|
pmorris
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → khushil324
Depends on: tb-burn-xul-grids
Assignee | ||
Updated•5 years ago
|
Blocks: tb-burn-xul-grids
No longer depends on: tb-burn-xul-grids
Assignee | ||
Comment 1•5 years ago
|
||
Attachment #9099570 -
Flags: review?(paul)
Assignee | ||
Updated•5 years ago
|
Status: NEW → ASSIGNED
Comment 2•5 years ago
|
||
Comment on attachment 9099570 [details] [diff] [review] Bug-1582717_remove-frid-calendar-month-base-view.patch Review of attachment 9099570 [details] [diff] [review]: ----------------------------------------------------------------- Code changes look good and the grid views look like they are working fine. Just a few comments on the code to address. ::: calendar/base/content/calendar-month-base-view.js @@ +433,4 @@ > // week labels, taking into account whether days-off are displayed or not. > let weekLabelColumnPos = -1; > > + const rows = this.monthgrid.childNodes; Better to go ahead and use `.children` instead of `.childNodes` here and elsewhere in this patch. (It's friendlier for use with HTML, see bug 1570954). @@ +451,1 @@ > } Lets simplify this while we're here: row.toggleAttribute("hidden", finished); if (finished) { continue; } @@ +600,5 @@ > + for (let j = 0; j < rows[0].childNodes.length; j++) { > + if (rows[j]) { > + rows[j].childNodes[i].toggleAttribute("hidden", dayOff && !this.mDisplayDaysOff); > + } > + } Here's a way to simplify and make this easier to follow: const lastColNum = rows[0].children.length - 1; for (let colNum = 0; colNum <= lastColNum; colNum++) { ... for (let row of rows) { row.children[colNum].toggleAttribute(...); } }
Attachment #9099570 -
Flags: review?(paul)
Assignee | ||
Comment 3•5 years ago
|
||
Attachment #9099570 -
Attachment is obsolete: true
Attachment #9099968 -
Flags: review?(paul)
Comment 4•5 years ago
|
||
Comment on attachment 9099968 [details] [diff] [review] Bug-1582717_remove-frid-calendar-month-base-view-1.patch Review of attachment 9099968 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks. I checked again and the month view grid is still intact.
Attachment #9099968 -
Flags: review?(paul) → review+
Assignee | ||
Updated•5 years ago
|
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/311c42a94e23
remove grid usage from calendar-month-base-view.js. r=pmorris
Updated•5 years ago
|
Target Milestone: --- → 71
You need to log in
before you can comment on or make changes to this bug.
Description
•