Closed Bug 1621130 Opened 4 years ago Closed 4 years ago

In calendar and tasks tabs show a message when all calendars are disabled

Categories

(Calendar :: General, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 76.0

People

(Reporter: pmorris, Assigned: pmorris)

References

Details

Attachments

(6 files, 5 obsolete files)

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

Following up on bug 1617786, address this special case:

SPECIAL CASES

  • Calendar, Tasks, Event, and Task tabs
    Basically unchanged, but if calendar is unused, show a message to explain the current status, and provide a smooth path for users to start using calendar features.

After discussing the details of this today with Alessandro, we will show a brief message in the calendar and tasks tabs when users have all calendars disabled, something simple like "All calendars are currently disabled." It will appear just below the minimonth and above the "Calendars" heading. The background color could be something like light blue (like the one used in the iMIP bar) to attract some attention but not too much.

We also discussed further enhancements to the calendar list to both improve it in general and make how to enable a disabled calendar more obvious. We'll open a separate bug for that work. Edit: see bug 1621135.

Attached patch show-calendars-disabled-message-0.patch (obsolete) β€” β€” Splinter Review

This does the job and uses Fluent. I went with a brief message (similar to what aleca and I had discussed) to try to keep it to one line. The background color is from the calendar theme, namely var(--viewTodayLabelSelectedBackground). Suggestions welcome on styling. I'll post a screenshot.

Attachment #9132703 - Flags: ui-review?(alessandro)
Attachment #9132703 - Flags: review?(geoff)
Attachment #9132703 - Flags: feedback?(alessandro)
Status: NEW → ASSIGNED
Comment on attachment 9132703 [details] [diff] [review]
show-calendars-disabled-message-0.patch

I think this is fine, but I wonder if it's a bit too short and we should tell users a little bit more about this current status.
Something that they might not be aware of is that with all calendars disabled they won't be able to create events or receive any related  notifications.

I like the clean one line output, but I doubt it'll be like that in all the other languages, so we shouldn't be too strict.
Maybe we could explore the possibility of having an info icon which opens a popup panel or tooltip with an explanation of the current status.
"Without an enabled Calendar you won't be able to create Events or process Invitations", or something like that.

I'm not really sure, tho, it's just an idea.
Attachment #9132703 - Flags: ui-review?(alessandro)
Attachment #9132703 - Flags: ui-review+
Attachment #9132703 - Flags: feedback?(alessandro)

I would have assumed the message, or at least in addition, a message would be shown in the main area. Either the same way as for "nothing found" for the quicksearch, or as a notification bar somewhere. There there is also plenty of space.

(In reply to Alessandro Castellani (:aleca) from comment #4)

I like the idea of an info icon that opens a popup panel or tooltip with more details. It's the kind of message people won't likely need to read more than once, which seems well suited for a popup like that.

(In reply to Magnus Melin [:mkmelin] from comment #5)

One use case aleca and I discussed was a user not enabling a calendar but just using the calendar for things like checking the day of the week for a given date, or similar. So putting the message in the sidebar leaves the main area uncluttered for that kind of use.

Also it's good to have the message next to the list of calendars, which helps the user find where to enable them, and if the user disables the last enabled one, the message appears there where their focus was already. Those are some reasons I like the sidebar approach.

(In reply to Paul Morris [:pmorris] from comment #6)

I like the idea of an info icon that opens a popup panel or tooltip with more details. It's the kind of message people won't likely need to read more than once, which seems well suited for a popup like that.

Let's flesh that idea out some more before we go ahead with this piece.

(In reply to Geoff Lankow (:darktrojan) from comment #7)

Let's flesh that idea out some more before we go ahead with this piece.

Mock-ups incoming in 3...2...1...

Attached image calendar-notification.jpg β€”

What about something like this?
I wrote a longer text to simulate what might happen if the content doesn't fit in one line.
Hovering over the info icon might reveal a tooltip with more info.

We might even consider using a popover if we want to include direct links to something if necessary.

I like the look of this. When combined with the improvements in bug 1621135 it will be clear how to enable a disabled calendar, so I'd think a tooltip would be sufficient.

Jorgk pointed out that the sidebar can be collapsed, and if it gets collapsed with all calendars disabled then that could lead to confusion since the message about disabled calendars is hidden, along with how to enable them. "Why can't I create an event?" One solution is to put the message somewhere in the main area rather than in the sidebar.

On the other hand, currently the way the sidebar is collapsed is potentially confusing in any case, since it's not easy to see that it can be expanded again, not obvious that something has been hidden or how to un-hide it. So there may be room for improvement there as well.

Indeed, if there's a chance that the calendar sidebar might be collapsed, the use could miss this important message.
At this point we might use a standard MozElements.NotificationBox above the main calendar area.

Attached patch show-calendars-disabled-message-1.patch (obsolete) β€” β€” Splinter Review

Alright, now using a lazily loaded MozElements.NotificationBox.

Attachment #9132703 - Attachment is obsolete: true
Attachment #9132703 - Flags: review?(geoff)
Attachment #9135252 - Flags: review?(geoff)
Attachment #9135252 - Flags: review?(alessandro)
Attachment #9135252 - Flags: ui-review?(alessandro)

This is with an "info" priority, which seemed too subtle to me.

This is with a "warning" priority level, which is much more noticeable.

Clicking the close button makes the notification go away. I assume that's desired behavior in this case.

Comment on attachment 9135252 [details] [diff] [review]
show-calendars-disabled-message-1.patch

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

This looks good, and yes, the warning level is appropriate since the current state of the calendars prevents access to a lot of features, so it shouldn't be just a simple info text.

I'm OK with dismissing the notification, but I wonder if we should consider showing it again at the next startup, so after TB gets closed and reopened.
We need to consider a user that dismisses that message immediately, and after a while decides to use the calendar and doesn't remember why everything is disabled.

We could potentially also offer the usual "More" button > "Don't show me this message anymore" option if it gets annoying for the user.
Attachment #9135252 - Flags: ui-review?(alessandro)
Attachment #9135252 - Flags: ui-review+
Attachment #9135252 - Flags: review?(alessandro)
Comment on attachment 9135252 [details] [diff] [review]
show-calendars-disabled-message-1.patch

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

I think it's fine showing every time without a "don't show again" option. I could be wrong but let's do it like this for now and see how many complaints there are.

::: calendar/lightning/content/calendar-tab-panels.inc.xhtml
@@ +196,5 @@
>                persist="state"
>                class="calendar-sidebar-splitter"/>
> +    <vbox flex="1">
> +      <hbox id="calendar-deactivated-notification-location"
> +            flex="1">

This should not have flex="1". It causes an enormous notification with rotated views for reasons I'm not entirely certain of.
Attachment #9135252 - Flags: review?(geoff) → review+

Maybe we should have a separate message for event and tasks? To end users it's a bit odd that calendars affect tasks...

(In reply to Magnus Melin [:mkmelin] from comment #18)

Maybe we should have a separate message for event and tasks? To end users it's a bit odd that calendars affect tasks...

Hm, what would the separate messages be? Seems like the would be almost the same:

"All calendars are currently disabled. Without an enabled calendar you won't be able to create events."
"All calendars are currently disabled. Without an enabled calendar you won't be able to create tasks."

And only one of those messages shields users from the inescapable oddness that working with tasks depends on having a calendar enabled. I'm not sure it's worth it. I'd say it's better to err on the side of fully informing users about what enabling a calendar lets them do.

That's the strings I would have used yes.

Alessandro, what say you? While we are work-shopping messages, it probably makes sense to mention the possibility of adding a new calendar, since this is what users will see on first run. Something like:

"All calendars are currently disabled. Enable an existing calendar or add a new one to create and edit ${events | tasks | events and tasks}."

Flags: needinfo?(alessandro)

(In reply to Paul Morris [:pmorris] from comment #21)

"All calendars are currently disabled. Enable an existing calendar or add a new one to create and edit ${events | tasks | events and tasks}."

Yes, this sounds good.

Flags: needinfo?(alessandro)
Attached patch show-calendars-disabled-message-2.patch (obsolete) β€” β€” Splinter Review

This implements what I discussed with Magnus in a meeting today, which is to have separate notificationbox elements, one for the calendar tab and one for the tasks tab, with separate messages for each, as discussed above. Wording is:

"All calendars are currently disabled. Enable an existing calendar or add a new one to create and edit events."

And then swap "events" with "tasks" for the tasks tab. If you click the close button "X" the bar goes away, but if you restart Thunderbird it comes back again, which is the desired behavior Alessandro described above.

Try run just started: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f89222647be912edbb06409cae9e0918c41470d1

Flagging Geoff for review in case he would like another pass at it after the changes.

Attachment #9135252 - Attachment is obsolete: true
Attachment #9135573 - Flags: review?(geoff)
Attached patch show-calendars-disabled-message-3.patch (obsolete) β€” β€” Splinter Review

Rebased so it applies to current trunk, and with fixed lookup paths for the two failing tests from the previous try run: browser_basicFunctionality and browser_taskView. New try run underway:

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

Eslint found no issues locally, but there are some in the try run. The same set of errors are showing up in some of the previous try runs (and some of those not initiated by me). Not sure what's going on with that.

Attachment #9135573 - Attachment is obsolete: true
Attachment #9135573 - Flags: review?(geoff)
Attachment #9135705 - Flags: review?(geoff)
Comment on attachment 9135705 [details] [diff] [review]
show-calendars-disabled-message-3.patch

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

::: calendar/base/modules/calCalendarDeactivator.jsm
@@ +122,5 @@
> +    ];
> +
> +    for (let [notificationbox, messageName] of notificationboxes) {
> +      if (calendarIsActivated) {
> +        notificationbox.removeAllNotifications();

I think we should only remove the notification we added. Notification boxes tend to get reused for things other than their original intention, and we don't want to cause problems if that happens.

::: calendar/lightning/content/messenger-overlay-sidebar.js
@@ +18,5 @@
>  var { cal } = ChromeUtils.import("resource:///modules/calendar/calUtils.jsm");
>  
>  var gLastShownCalendarView = null;
>  
> +var gCalendarDeactivatedEventsNotification = {};

Can we add these to calendarTabType instead and avoid adding more global variables? We already have way too many.

::: mail/base/content/messenger.xhtml
@@ +139,5 @@
>  <linkset>
>    <html:link rel="localization" href="toolkit/main-window/findbar.ftl"/>
>    <html:link rel="localization" href="toolkit/global/textActions.ftl"/>
>    <html:link rel="localization" href="messenger/menubar.ftl"/>
> +  <html:link rel="localization" href="calendar/calendar-widgets.ftl"/>

Is this necessary or a leftover from an earlier version of this patch?
Attachment #9135705 - Flags: review?(geoff)
Attached patch show-calendars-disabled-message-4.patch (obsolete) β€” β€” Splinter Review

Thanks for the review. This addresses the comments, which were all good points.

New try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=30a692d82f6c9dea07aebdc94427758eea9f52bf

(In reply to Geoff Lankow (:darktrojan) from comment #25)

Is this necessary or a leftover from an earlier version of this patch?

Huh, turns out this isn't needed when you're using Fluent via JS as the patch does now.

Attachment #9135705 - Attachment is obsolete: true
Attachment #9135878 - Flags: review?(geoff)

Based on chat discussion with Geoff I've put the lazily loaded notificationbox instances on calendarTabType.modes.calendar / .tasks. And did a little renaming to make searching for related things easier by putting "events" and "tasks" at the end of the names.

New try run, just started:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ab152b25b40414f9ef9c0635e27b0847091a821b

Attachment #9135878 - Attachment is obsolete: true
Attachment #9135878 - Flags: review?(geoff)
Attachment #9135881 - Flags: review?(geoff)
Attachment #9135881 - Flags: review?(geoff) → review+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/41e7738adee2
Show a message in calendar tabs when calendars are disabled. r=darktrojan ui-r=aleca

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 76

(In reply to Paul Morris [:pmorris] from comment #19)

(In reply to Magnus Melin [:mkmelin] from comment #18)

Maybe we should have a separate message for event and tasks? To end users it's a bit odd that calendars affect tasks...

Hm, what would the separate messages be? Seems like the would be almost the same:

"All calendars are currently disabled. Without an enabled calendar you won't be able to create events."
"All calendars are currently disabled. Without an enabled calendar you won't be able to create tasks."

And only one of those messages shields users from the inescapable oddness that working with tasks depends on having a calendar enabled. I'm not sure it's worth it. I'd say it's better to err on the side of fully informing users about what enabling a calendar lets them do.

"Tasks and events are both parts of , and require a working calendar. Without an enabled calendar, you will not be able to create either of these."

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: