1680044 - remove <deck> from calendar-tab-panels.inc.xhtml

Status: RESOLVED FIXED

(Calendar :: General, task)

Description

[Khushil Mistry [:khushil324]]

Remove <deck> from https://searchfox.org/comm-central/rev/8783b24ad5915927163977ea19a9968659b90d4c/calendar/lightning/content/calendar-tab-panels.inc.xhtml#205

It has two children. These we can show/hide as needed.

Comment 1

[Khushil Mistry [:khushil324]]

Attachment: Bug-1680044_de-deck-calendar-tab-panels-inc-0.patch

I have removed two functions, I think there is no use for them.

Comment 2

[Alessandro Castellani [:aleca]]

Comment on attachment 9191458 [details] [diff] [review]
Bug-1680044_de-deck-calendar-tab-panels-inc-0.patch

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

Bouncing this review to Geoff since it touches some core components of the calendar code, especially the removal of those 2 observers.

Comment 3

[Geoff Lankow (:darktrojan)]

Comment on attachment 9191458 [details] [diff] [review]
Bug-1680044_de-deck-calendar-tab-panels-inc-0.patch

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

I hate how the calendar and task tabs work. One day we will fix this. Today is not that day. :-)

The two functions you're removing appear to be ancient history. They've gone almost untouched since the day this code was imported to HG, and I don't care enough to look back any further. As long as everything works without them, I have no problem removing them.

::: calendar/base/content/calendar-task-view.js
@@ -426,5 @@
> -  }
> -
> -  // In case we find that the task view has been made visible, we refresh the view.
> -  if (deck.selectedPanel && deck.selectedPanel.id == "calendar-task-box") {
> -    taskViewUpdate();

I *think* you're correct in not replacing this. I can't see how calling taskViewUpdate whenever the tasks tab is selected would achieve anything.

::: calendar/base/content/calendar-views-utils.js
@@ +261,5 @@
>    ids.forEach(x => setupViewNode(x, "tooltiptext"));
>  
>    try {
> +    selectedDay = currentView().selectedDay;
> +    currentSelection = currentView().getSelectedItems();

Do currentView() once and use the result twice.

@@ +315,4 @@
>   * @return      The selected calendar view
>   */
>  function currentView() {
> +  for (let element of document.getElementById("view-box").children) {

We can use getViewBox() here.

@@ +466,5 @@
>    let newValue = cmd.getAttribute("checked") == "true" ? "false" : "true";
>    cmd.setAttribute("checked", newValue);
>  
> +  let viewBox = getViewBox();
> +  for (let view of viewBox.children) {

Let's combine these two lines (and in the other functions that look like this one).

::: calendar/lightning/content/calendar-tab-panels.inc.xhtml
@@ +432,4 @@
>        </vbox>
>        <!-- Tasks View -->
>        <vbox id="calendar-task-box" flex="1"
> +            onselect="taskDetailsView.onSelect(event);"

It seems like this event should be somewhere else, like on calendar-task-tree. I don't know why it's up here. Please investigate.

Comment 4

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679828_de-deck-calendar-today-pane-inc-xhtml-1.patch

Comment 5

[Khushil Mistry [:khushil324]]

Attachment: Bug-1680044_de-deck-calendar-tab-panels-inc-2.patch

Comment 6

[Geoff Lankow (:darktrojan)]

Comment on attachment 9192520 [details] [diff] [review]
Bug-1680044_de-deck-calendar-tab-panels-inc-2.patch

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

This is good.

BTW, I've noticed your diffs only have 3 lines of context instead of the usual 8. Could you fix that please? It's so much easier to work out what's going on with more context.

Comment 7

[Khushil Mistry [:khushil324]]

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d32b7dfa99ad
remove <deck> XUL element from calendar-tab-panels.inc.xhtml. r=aleca,darktrojan