1621135 - Improve the UI of the Calendar sidebar

Status: RESOLVED FIXED

(Thunderbird :: Theme, enhancement, P1)

Description

[Alessandro Castellani [:aleca]]

Attachment: calendar-sidebar.png

After a couple of meetings with Paul regarding the integration of the Calendar into core, we ended up in need of reworking the UI of the Calendar sidebar in order to improve the discoverability of some options, and allowing us to better handle different scenarios.

Attached you can find a mock-up highlighting the major improvements we'd like to add to this section.

• Add a + button to allow easy access tot he Calendar creation dialog.
• Remove the checkboxes to hide/show the calendar and use the eye icon to handle those states, visible only on rollover.
• Show a more icon on rollover/focus to hint at the context menu.
• Show a permanent hidden icon when a Calendar is hidden.
• Show a quick enable link when a Calendar is disabled.

Comment 1

[Magnus Melin [:mkmelin]]

Comment on attachment 9132081 [details]
calendar-sidebar.png

I like the plus, and that the checkbox next to the calendar name is gone - that's really a confusing UI. Though what is hidden is maybe not clear enough in the new UI. 

Calendar names are often pretty long (like 30ch), so you may not be able to easily implement it nicely the way it's in the proposal.

At the same time we could also get rid of how this is a collapsible list (is it a tree? I didn't look). I don't see a reason you'd ever collapse it.
And why does it have a header "Calendar", when it's  list of CalendarS?

Comment 2

[Paul Morris [:pmorris]]

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

At the same time we could also get rid of how this is a collapsible list (is
it a tree? I didn't look). I don't see a reason you'd ever collapse it.

It used to be a tree but Geoff has converted it to a richlist. Good call on removing the collapsible aspect, and on renaming to "Calendars".

Comment 3

[Alessandro Castellani [:aleca]]

I'll update the mock-up to improve the hidden status, and show how we can handle longer names.

Comment 4

[Alessandro Castellani [:aleca]]

Attachment: calendar-sidebar.png

Here's an updated proposal based on Magnus' feedback.

Long calendar names
We could allow the label to crop at the end in order to let that section adapt based on how many icons we're showing.
If a calendar already has some icons showing some specific states, the "more" and "visible" icon push those to the left (screen 3).

If a calendar is hidden or disabled, no other icons are shown except for those representing those statuses.

A hidden calendar will show the label with an italic style, and the hidden icon always visible.

Another nice addition could be allowing users to trigger the color picker by clicking on the colored circle, reducing the number of clicks to access that option.

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9132418 [details]
calendar-sidebar.png

Looks pretty good. 
Which calendars are not shown may have to be more prominent somehow. I wonder if they could just be shown in a separate group, under  the enabled ones.
For the use case that you opt to show just one or two calendars it's not very clear that the others are not currently shown, later on. This is a rather major problem in the current implementation and disabled calendars: it's rather subtle that they are disabled and one can easily miss events if one calendar got disabled.

Comment 6

[Alessandro Castellani [:aleca]]

Attachment: calendar-sidebar-2.png

Which calendars are not shown may have to be more prominent somehow. I wonder if they could just be shown in a separate group, under the enabled ones.

I wouldn't do that as creating too much separation between calendar states (enabled, hidden, disabled) doesn't bring any benefit and it forces us to over-engineer the UX too much.
We should let the user decide the order of those calendars, keeping that order consistent and familiar.

For the use case that you opt to show just one or two calendars it's not very clear that the others are not currently shown, later on.

I attached another mock-up example.
I think with the suggested implementation, it will be pretty clear at first glance which calendar is hidden or disabled.

it's rather subtle that they are disabled and one can easily miss events if one calendar got disabled.

This can be solved by showing a message when disabling a calendar to let the user know, and we could also add a tooltip when hovering over a disabled calendar, something like "Enable this Calendar to receive event notifications".

I don't think we should make this sidebar too cluttered with groups and sections, or move things around too often. We should keep it simple and intuitive, leveraging iconography to communicate important statuses, and allowing the user to reorder the calendar as they want.

Comment 7

[Alessandro Castellani [:aleca]]

Attachment: 1621135-calendar-sidebar.diff

All right, this is ready for a review.

I had to pull the addons.properties file in order to get the "Enable" string as we don't have it anywhere else and I don't want to add a new string so we can land this on 78.

Richard, I added some colors based on my mock-up but I didn't have time to properly test them with dark mode and other platforms. Feel free to suggest the usage of some predefined variables.

I'm pinging also Paul on this review since he worked on this section and might know a better approach on how to tackle this.

Comment 8

[Magnus Melin [:mkmelin]]

Comment on attachment 9173910 [details] [diff] [review]
1621135-calendar-sidebar.diff

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

For a slightly longer calendar name this doesn't work at all, the "enable" button is pushed out of sight. Anyway, the icons are also now not lined up in anyway so it's far from pretty.
Works ok if there is only one disabled home calendar though...

I think it could make sense to add an Enable button to the notification. And maybe add some accesspoint for setting up a new calendar. I guess a Create button could be added to the notifcation as well.

Comment 9

[Richard Marti (:Paenglab)]

Comment on attachment 9173910 [details] [diff] [review]
1621135-calendar-sidebar.diff

Yes, for the dark themes it needs some tweaks. I'll attach a patch that fixes the most theme issues.
When I go to the plus icon, the tooltip shows "Remove Calendar...", a little bit irritating. ;-)

Comment 10

[Richard Marti (:Paenglab)]

Attachment: 1621135-calendar-sidebar.diff

Theme fixes. Plus I fixed the tooltip. There was also a typo in calendar-list-header. It was calednar-list-header.
For the dark theme I had to add the calendar-list-create button to messenger.css. We should think about using a class instead of adding every special button to this list.

Comment 11

[Richard Marti (:Paenglab)]

Attachment: actualPatchOnDark.png

Screenshot how it looks with my patch. The top "ENABLE" button is hovered. What do you think, can we use this colours on dark themes?

Comment 12

[Richard Marti (:Paenglab)]

Attachment: 1621135-calendar-sidebar.diff

My patch updated after Bug 1624676 landing.

Comment 13

[Paul Morris [:pmorris]]

Comment on attachment 9173910 [details] [diff] [review]
1621135-calendar-sidebar.diff

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

Thanks Alex!  The code changes look fine to me (except for the typo, which Richard caught), and this will improve the UX for making it clear how to enable the Home calendar, and also how to create a new calendar.

Magnus makes a good point about long names pushing the enable button out of sight.  I see this already happens on 78 with the "displayed" eye icons/checkboxes.  If there's not enough time to fix that before we want to get this into 78, then I think it would be alright to do it in a follow-up.

Just a thought, but aesthetically, I wonder if it would be better to make the enable button a little more subtle?  Like for the case where a user keeps several disabled calendars around for awhile, would the buttons get distracting?

I like the idea of guiding the user to enable or create calendars from the notification, but that may be difficult to do well on 78 without being able to introduce new strings, especially for the case where a user has several disabled calendars, and would need some way to choose which one(s) to enable.

::: calendar/base/content/calendar-management.js
@@ +179,5 @@
> +    checkbox.hidden = calendar.getProperty("disabled");
> +    let stringName = cal.view.getCompositeCalendar(window).getCalendarById(calendar.id)
> +      ? "hideCalendar"
> +      : "showCalendar";
> +    checkbox.setAttribute("tooltiptext", cal.l10n.getCalString(stringName, [calendar.name]));

While we're here should we rename "checkbox" to "displayedCheckbox" or something similar, to make it clearer which part of the UI it is?  Now that it looks like an eye icon, I have to think twice when I read "checkbox".

::: calendar/base/themes/common/widgets/calendar-widgets.css
@@ +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/. */
>  
> +.calednar-list-header {

typo

::: calendar/lightning/content/calendar-tab-panels.inc.xhtml
@@ +176,5 @@
>          <calendar-modevbox id="calendar-list-pane"
>                             flex="1"
>                             mode="calendar,task"
>                             refcontrol="calendar_toggle_calendarlist_command">
> +          <hbox align="center" class="calednar-list-header">

typo: calednar

Comment 14

[Alessandro Castellani [:aleca]]

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

For a slightly longer calendar name this doesn't work at all, the "enable" button is pushed out of sight.

My bad, I missed the crop="end" attribute on the label, that fixes it.

Anyway, the icons are also now not lined up in anyway so it's far from pretty.

That seems odd, could you share a screenshot?

I think it could make sense to add an Enable button to the notification. And maybe add some accesspoint for setting up a new calendar. I guess a Create button could be added to the notifcation as well.

Indeed, we should improve the notification with a couple of call to action, but I'd like to do it in a different bug and here focus only on the sidebar.
Already with the + icon and a clear enable button alongside the default disabled Calendar, it's way more intuitive and easy to figure out for first time users.

Comment 15

[Alessandro Castellani [:aleca]]

Attachment: 1621135-calendar-sidebar.diff

Patch updated with the improvements from Richard and the fix for long labels.

Comment 16

[Alessandro Castellani [:aleca]]

Attachment: Screenshot from 2020-09-04 10-18-42.png

Here's how it looks with a couple of disabled calendars.

Comment 17

[Richard Marti (:Paenglab)]

Comment on attachment 9173995 [details] [diff] [review]
1621135-calendar-sidebar.diff

I had to create a longer calendar name to see the cropping.
The sidebar changes are looking good.

Comment 18

[Magnus Melin [:mkmelin]]

Attachment: calenable.png

It's a bit better now with the cropping.
Still not sure about the over all concept. For many localizations the "enable" word is going to be much longer.

Comment 19

[Magnus Melin [:mkmelin]]

Comment on attachment 9173995 [details] [diff] [review]
1621135-calendar-sidebar.diff

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

(I'm not a calendar/ reviewer)

::: calendar/base/content/calendar-management.js
@@ +15,5 @@
>  
>  var { cal } = ChromeUtils.import("resource:///modules/calendar/calUtils.jsm");
>  var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
>  
> +const bundle = Services.strings.createBundle("chrome://messenger/locale/addons.properties");

top level const in a .js file is generally recipe for trouble...

Would move this down and create it only if you're going to show the disable button

Comment 20

[Alessandro Castellani [:aleca]]

Attachment: 1621135-calendar-sidebar.diff

Sweet, thanks for the feedback.
I'm asking Paul for a full review as he's a Calendar peer/reviewer.

Comment 21

[Alessandro Castellani [:aleca]]

Attachment: 1621135-calendar-sidebar.diff

commit message updated

Comment 22

[Paul Morris [:pmorris]]

Comment on attachment 9174499 [details] [diff] [review]
1621135-calendar-sidebar.diff

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

Looks good to me.  One suggestion on lazy loading the string bundle.

::: calendar/base/content/calendar-management.js
@@ +124,5 @@
>    } else {
>      initHomeCalendar();
>    }
>  
> +  bundle = Services.strings.createBundle("chrome://messenger/locale/addons.properties");

See below on this.

@@ +170,5 @@
>      image.setAttribute("tooltip", "calendar-list-tooltip");
>      item.appendChild(image);
>  
> +    let enable = document.createXULElement("button");
> +    enable.label = bundle.GetStringFromName("webextPerms.sideloadEnable.label");

As Magnus suggested, could we only add a label if the calendar is disabled?  Then we could set "bundle" here, only if it hasn't been set already, so the bundle is only loaded if there's a disabled calendar.  Something like:

if disabled {
    if not bundle {
        bundle =  ... 
    }
    enable.label = bundle...
}

Comment 23

[Alessandro Castellani [:aleca]]

Attachment: 1621135-calendar-sidebar.diff

Thanks for the review, patch updated and try-run launched: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=7f6618b4e387ffde1bafc6f632502a76dc072e89

Comment 24

[Alessandro Castellani [:aleca]]

Attachment: 1621135-calendar-sidebar.diff

Good thing for the try run as I did break a couple of tests with this change.
Now everything looks good.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/34c56fe1451c
Improve the UI of the Calendar sidebar. r=pmorris, ui-r=Paenglab

Comment 26

[Alessandro Castellani [:aleca]]

Wayne, should we uplift this bug to beta to allow users to test it and see if it solves almost all the issues reported in bug 1660644?

Comment 27

[Wayne Mery (:wsmwk)]

Comment on attachment 9174802 [details] [diff] [review]
1621135-calendar-sidebar.diff

[Triage Comment]
Approved for beta.

Wayne, should we uplift this bug to beta to allow users to test it and see if it solves almost all the issues reported in bug 1660644?

Yes. But if it causes problems let's back out because we are still driving important patches through beta and on to 78.

Comment 28

[Rob Lemley [:rjl]]

Thunderbird 81.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/76d4ee2aa580

Comment 29

[WaltS48 [:walts48]]

Much nicer in 81.0b4 on Windows10.

The "Enable" button to enable a calendar and the "+" to add a new calendar make it much easier.

Comment 31

[Alessandro Castellani [:aleca]]

Comment on attachment 9174802 [details] [diff] [review]
1621135-calendar-sidebar.diff

[Approval Request Comment]
Regression caused by (bug #): -
User impact if declined: Users will have issues in finding how to enable the default calendar on startup, and how to create a new calendar.
Testing completed (on c-c, etc.): on c-c and beta
Risk to taking this patch (and alternatives if risky): moderate as it changes the UI quite a bit, but the tests have been updated and Walt confirmed the improvements on beta.

Comment 32

[Wayne Mery (:wsmwk)]

Comment on attachment 9174802 [details] [diff] [review]
1621135-calendar-sidebar.diff

[Triage Comment]
Approved for esr78

We can be sure to test this pre-release

Comment 33

[Rob Lemley [:rjl]]

Thunderbird 78.3.0:
https://hg.mozilla.org/releases/comm-esr78/rev/28b8f3d8e951

Comment 34

[WaltS48 [:walts48]]

Looks great in my test of 78.3.0 on Windows10!