1673654 - opening an event in the today pane should open the event summary, not the edit event dialog

Status: RESOLVED FIXED

(Calendar :: Dialogs, enhancement)

Description

[Magnus Melin [:mkmelin]]

+++ This bug was initially created as a clone of Bug #1575195 +++

For the Today pane, bug 1575195 is still not implemented - we should do the same there: open the summary pane instead of editing dialog. I think for the today pane it's even more likely you'd want to check the event details so that you know where to go, which meeting link to open and such.

Comment 1

[Lasana Murray]

Attachment: bug1673654.patch

Changes modifyEventWithDialog to add openEventDialogForViewing in various places. Adds test to ensure unfinder and today pane open events in summary dialog.

Comment 2

[Lasana Murray]

Attachment: bug1673654.patch

Fixed a typo in the test.

Comment 3

[Lasana Murray]

Try run, had some issues with the todayPane test but finally got it to work on linux at least.

https://treeherder.mozilla.org/jobs?repo=try-comm-central&selectedTaskRun=DKKKmJXzREmqVoDWm02j8Q.0

Comment 4

[Lasana Murray]

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

Comment 5

[Geoff Lankow (:darktrojan)]

Comment on attachment 9185243 [details] [diff] [review]
bug1673654.patch

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

This looks pretty good to me. A few things to tidy up.

::: calendar/test/browser/browser_calendarUnifinder.js
@@ +1,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/* globals agendaListbox */

Not used here.

@@ +22,5 @@
> +  let manager = cal.getCalendarManager();
> +  let calendar = manager.createCalendar("memory", uri);
> +  let calendarProxy = cal.async.promisifyCalendar(calendar);
> +
> +  calendar.name = "Today Pane";

Hmm!

@@ +26,5 @@
> +  calendar.name = "Today Pane";
> +  manager.registerCalendar(calendar);
> +  registerCleanupFunction(() => manager.removeCalendar(calendar));
> +
> +  let now = new Date().toISOString().replace(/[:.-]/g, "");

Why not cal.dtz.now()?

@@ +40,5 @@
> +  repeatEvent.recurrenceInfo.appendRecurrenceItem(
> +    cal.createRecurrenceRule("RRULE:FREQ=DAILY;COUNT=30")
> +  );
> +
> +  await CalendarTestUtils.openCalendarTab(window);

You should check the unifinder is open here. We haven't closed it in tests, but that could change.

@@ +59,5 @@
> +    );
> +
> +    let closePromise = BrowserTestUtils.domWindowClosed(dialogWindow);
> +    dialogWindow.close();
> +    await closePromise;

BrowserTestUtils.closeWindow will do this for you. At this point you could delete the event like you do in the Today Pane test, instead of keeping track with idx.

Comment 6

[Lasana Murray]

Attachment: bug1673654v2.patch

Feedback incorporated.

Comment 7

[Lasana Murray]

Try run looks good:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=204a01db1d443c276944170e42ea15755e6b6b18

Comment 8

[Geoff Lankow (:darktrojan)]

Comment on attachment 9188918 [details] [diff] [review]
bug1673654v2.patch

That's better. Thanks.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f2ca0b224080
Open events in calendar summary dialog for today pane and unifinder. r=darktrojan