1703078 - Adapt the TB stylings to the Proton infobars

Status: RESOLVED FIXED

(Thunderbird :: Theme, task, P3)

Description

[Richard Marti (:Paenglab)]

Behind the pref browser.proton.infobars.enabled are new infobar stylings we need to adapt to look good. In bug 1702330 they plan to move the pref into browser.proton.enabled and then it should be ready.

With this pref enabled we will have issues with the menu-buttons in remote-blocked and attachment-reminder infobars. But this bug isn't for this issues. I'll file a bug when this one is landed.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1703078-proton-infobar-fixes.patch

You need to create and enable browser.proton.infobars.enabled to see the new infobars that are now inside the message headers. But please check also with disabled pref to be sure it doesn't change the actual bars.

Comment 2

[Alessandro Castellani [:aleca]]

Comment on attachment 9213622 [details] [diff] [review]
1703078-proton-infobar-fixes.patch

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

This is a great porting, thanks for taking care of this.

A couple of issues.
1. We also have a type="critical" for some error messages or OpenPGP/SMIME related notifications. That type should be styled like the error (red).
2. Multiple notifications don't stack on top of each other on the z-axis anymore, but are listed one after another, is this something coming from Proton?

Multiple notifications from the same container should properly stack otherwise we risk to have spacing issues (reported in previous bugs) where multiple notifications fill up the entire screen.

I didn't find any issue with these CSS changes and our regular notifications.

::: mail/themes/shared/mail/message-bar.css
@@ +40,5 @@
> +
> +:host([type=warning]) {
> +  --message-bar-background-color: #fff160;
> +  --message-bar-text-color: #000;
> +

Remove blank line

@@ +47,5 @@
> +
> +:host([type=success]) {
> +  --message-bar-background-color: var(--green-50);
> +  --message-bar-text-color: var(--green-90);
> +

Also here

@@ +100,5 @@
> +  padding: 4px 8px;
> +  position: relative;
> +
> +  border-radius: 4px;
> +

Are these blank lines for readability? Can they be removed?

@@ +171,5 @@
> +    box-shadow: 0 1px 2px rgba(58, 57, 68, 0.3);
> +  }
> +  :host([type="info"]) .container.infobar {
> +    box-shadow: 0 0 1px #3a3944;
> +  }

I think it's okay for a regular infobar to have the same drop shadow instead of a simulated 1px border.
The regular info has a very low priority and it's okay if it doesn't catch the attention immediately, it's kind of did it on purpose, and we can simply rely on the icon, text, and button as visible items with good contrast.

::: mail/themes/windows/mail/messageHeader.css
@@ +8,5 @@
>  
>  @import url("chrome://messenger/skin/shared/messageHeader.css");
>  
>  /* ::::: for the entire area ::::: */
> +#singlemessage,

Since we're here, this should be renamed #singleMessage (camelCase) and of course also the id in the XHTML file.

Comment 3

[Richard Marti (:Paenglab)]

Attachment: 1703078-proton-infobar-fixes.patch

(In reply to Alessandro Castellani [:aleca] from comment #2)

Comment on attachment 9213622 [details] [diff] [review]
1703078-proton-infobar-fixes.patch

Review of attachment 9213622 [details] [diff] [review]:

This is a great porting, thanks for taking care of this.

A couple of issues.

1. We also have a type="critical" for some error messages or OpenPGP/SMIME
   related notifications. That type should be styled like the error (red).

Added the type="critical".

2. Multiple notifications don't stack on top of each other on the z-axis
   anymore, but are listed one after another, is this something coming from
   Proton?

Multiple notifications from the same container should properly stack
otherwise we risk to have spacing issues (reported in previous bugs) where
multiple notifications fill up the entire screen.

I think this is for the JS issues follow-up bug.

I didn't find any issue with these CSS changes and our regular notifications.
@@ +100,5 @@

• padding: 4px 8px;
• position: relative;
• border-radius: 4px;

Are these blank lines for readability? Can they be removed?

These came from the original M-C file. I removed all.

@@ +171,5 @@

• box-shadow: 0 1px 2px rgba(58, 57, 68, 0.3);
• }
• :host([type="info"]) .container.infobar {
• box-shadow: 0 0 1px #3a3944;
• }

I think it's okay for a regular infobar to have the same drop shadow instead
of a simulated 1px border.
The regular info has a very low priority and it's okay if it doesn't catch
the attention immediately, it's kind of did it on purpose, and we can simply
rely on the icon, text, and button as visible items with good contrast.

Done.

::: mail/themes/windows/mail/messageHeader.css
@@ +8,5 @@

@import url("chrome://messenger/skin/shared/messageHeader.css");

/* ::::: for the entire area ::::: */
+#singlemessage,

Since we're here, this should be renamed #singleMessage (camelCase) and of
course also the id in the XHTML file.

Fixed also in JS files

Comment 4

[Alessandro Castellani [:aleca]]

Comment on attachment 9213809 [details] [diff] [review]
1703078-proton-infobar-fixes.patch

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

Looks good, great work.

::: mail/themes/shared/mail/message-bar.css
@@ +236,5 @@
> +}
> +
> +strong {
> +  font-weight: 600;
> +}

This is never used, do we need it?
Written like this will also affect any <strong> tag in any page this CSS is included.

Comment 5

[Richard Marti (:Paenglab)]

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

Comment on attachment 9213809 [details] [diff] [review]
1703078-proton-infobar-fixes.patch

Review of attachment 9213809 [details] [diff] [review]:

Looks good, great work.

::: mail/themes/shared/mail/message-bar.css
@@ +236,5 @@

+}
+
+strong {

• font-weight: 600;
  +}

This is never used, do we need it?
Written like this will also affect any <strong> tag in any page this CSS is
included.

I'll let it in. Then we can use it to highlight some text as bold (like the trigger word in the attachment reminder). It shouldn't bleed out as this style is directly loaded into the shadow-DOM of the message-bar.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d9aeee2e011a
Adapt the TB stylings to the Proton infobars. r=aleca