Merge lp:~charlesk/indicator-datetime/lp-1320880-shorter-notifications-for-calendar-events into lp:indicator-datetime/rtm-14.09

[Charles Kerr]

Change notifications for calendar events s.t. the sound is nonrepeating and the notification is temporary, not requiring user interaction to disappear.

Same MP as proposed for Vivid @ https://code.launchpad.net/~charlesk/indicator-datetime/lp-1320880-shorter-notifications-for-calendar-events/+merge/243939

Description of the Change
=========================

This branch culls the bugfix for bug #1320880 from my development branch so that it's easier to review and to backport in isolation.

It's a fairly simple patch: it changes notifications for calendar events s.t. the sound is nonrepeating and the notification is temporary, not requiring user interaction to disappear.

Checklist
=========

> Are there any related MPs required for this MP to build/function as expected? Please list.

This MP stacks on top of lp:~charlesk/indicator-datetime/lp-1387231-honor-x-canonical-disabled-tag

> Is your branch in sync with latest trunk? (e.g. bzr pull lp:trunk -> no changes)

Yes

> Did the code build without warnings?

Yes

> Did the tests run successfully?

Yes

> Did you perform an exploratory manual test run of your code change and any related functionality?

Yes

> If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

N/A

> What device (or emulator) has your component test plan been executed successfully on?

Krillin running ubuntu-touch/ubuntu-rtm/14.09-proposed image 173

> What manual tests are relevant for this MP?

indicator-datetime/calendar-event-notification

> Did you include a link to the MR Review Checklist Template to make your reviewer's life easier?

https://wiki.ubuntu.com/Process/Merges/Checklists/indicator-datetime

[Ted Gould (ted)]

review: Approve

[Michał Sawicz (saviq)]

I believe these changes need a design review, see bug #1401802.

review: Needs Fixing (ux)

[lp:~charlesk/indicator-datetime/lp-1320880-shorter-notifications-for-calendar-events]

391. By Charles Kerr on 2014-12-12

Make calendar event notifications snap decisions that require manual dismissal, but without looping sound and without haptic feedback. This splits the difference between bug #1320880's complaint "calendar notifications are too intrusive" and bug #1401802's complaint "the new calendar notifications aren't intrusive enough."

[Ted Gould (ted)]

Works for me. We need to get comprehensive specs on this. Hopefully with notifications work.

review: Approve

[Ted Gould (ted)]

Also, we need an MR to merge r391 into trunk.

Unmerged revisions

391. By Charles Kerr on 2014-12-12

Make calendar event notifications snap decisions that require manual dismissal, but without looping sound and without haptic feedback. This splits the difference between bug #1320880's complaint "calendar notifications are too intrusive" and bug #1401802's complaint "the new calendar notifications aren't intrusive enough."

=== modified file 'src/snap.cpp'
--- src/snap.cpp	2014-12-12 18:03:01 +0000
+++ src/snap.cpp	2014-12-12 18:03:02 +0000
@@ -69,23 +69,20 @@
         // create the sound...
         const auto uri = get_alarm_uri(appointment, m_settings);
         const auto volume = m_settings->alarm_volume.get();
-        const bool loop = m_engine->supports_actions();
+        const bool loop = appointment.is_ubuntu_alarm() && m_engine->supports_actions();
         auto sound = std::make_shared<uin::Sound>(uri, volume, loop);
 
         // create the haptic feedback...
-        const auto haptic_mode = m_settings->alarm_haptic.get();
+        const auto haptic_mode = appointment.is_ubuntu_alarm() ? m_settings->alarm_haptic.get() : "none";
         std::shared_ptr<uin::Haptic> haptic;
         if (haptic_mode == "pulse")
             haptic = std::make_shared<uin::Haptic>(uin::Haptic::MODE_PULSE);
 
         // show a notification...
         const auto minutes = std::chrono::minutes(m_settings->alarm_duration.get());
-        const bool interactive = m_engine->supports_actions();
         uin::Builder b;
         b.set_body (appointment.summary);
         b.set_icon_name ("alarm-clock");
-        b.add_hint (uin::Builder::HINT_SNAP);
-        b.add_hint (uin::Builder::HINT_AFFIRMATIVE_HINT);
         b.add_hint (uin::Builder::HINT_NONSHAPED_ICON);
 
         const char * timefmt;
@@ -103,7 +100,9 @@
         b.set_title (title);
         g_free (title);
         b.set_timeout (std::chrono::duration_cast<std::chrono::seconds>(minutes));
-        if (interactive) {
+        if (m_engine->supports_actions()) {
+            b.add_hint (uin::Builder::HINT_SNAP);
+            b.add_hint (uin::Builder::HINT_AFFIRMATIVE_HINT);
             b.add_action ("ok", _("OK"));
             b.add_action ("snooze", _("Snooze"));
         }

=== modified file 'tests/manual'
--- tests/manual	2014-12-12 18:03:01 +0000
+++ tests/manual	2014-12-12 18:03:02 +0000
@@ -48,6 +48,15 @@
 		<dd>When the alarm is enabled, the alarm icon should reappear.</dd>
 </dl>
 
+Test-case indicator-datetime/calendar-event-notification
+<dl>
+	<dt>In the calendar app, create and save a new upcoming calendar event that will occur in the next few minutes.</dt>
+		<dd>The datetime indicator's event list should update itself to show this new event.</dd>
+		<dd>Calendar events do not get the alarm icon, so no alarm icon should be shown in the header unless there is also an upcoming alarm set.</dd>
+	<dt>Wait for the event's time to be reached</dt>
+		<dd>A snap decision notification should appear; but unlike alarms, the sound should be nonrepeating and there should be no haptic feedback.</dd>
+</dl>
+
 Test-case indicator-datetime/alarm-timezone
 <dl>
 	<dt>In ubuntu-system-settings, change your timezone to a zone you're not in</dt>