1582717 - remove grid usage from comm/calendar/base/content/calendar-month-base-view.js

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, task)

Comment 1

[Khushil Mistry [:khushil324]]

Attachment: Bug-1582717_remove-frid-calendar-month-base-view.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c5c960b8f6c8ad0181fc9dc61c28541c221b8a65

Comment 2

[Paul Morris [:pmorris]]

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(...);
  }
}

Comment 3

[Khushil Mistry [:khushil324]]

Attachment: Bug-1582717_remove-frid-calendar-month-base-view-1.patch

Comment 4

[Paul Morris [:pmorris]]

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.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/311c42a94e23
remove grid usage from calendar-month-base-view.js. r=pmorris