1660691 - Remove red font color when entering new recipients not yet in AB (no autocomplete matches found - so what?)

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, enhancement)

Description

[Thomas D. (:thomas8)]

Think about it a bit more, and the red font color upon input of recipients which do not have any autocomplete matches is not just irritating when entering valid addresses, but it's actually useless, apart from not working consistently.

I propose to remove this red font color thing from address inputs entirely.

1. Type fast the beginning of a known recipient: shows red first, then changes to black as autocomplete finds matches. Type slow again, no red. Inconsistent, useless - why warn in the first place for known recipient?
2. So if i have John Doe <[e-mail]> in my AB, and type John Miller <[e-mail]>, the new address will remain red even when completely entered - why warn for a completely correct address? That's extremely irritating. Then red will go away when I press Enter and valid pill is created, so what was it really warning me about all along? We may enter new email addresses all day, and TB shouldn't suggest that there's someting wrong with that. Yes, any new address won't have an autocomplete match - so what?
3. Given 2 - valid new addresses - what's the purpose of this red color? To warn me that it's not matching anything from my AB?

• Right, so if I want a particular match from my AB, and type the right thing - it will autocomplete (very visible), and I press Enter or Tab to autocomplete when satisfied, which will ensure correct address.
• If I start typing the wrong thing which accidentally does not match anyone else, but matches no one (which is the only case where we can show red), I will see immediately that it's NOT autocompleting - no results dropdown, no inline autocompletion (very visible). So red color doesn't add anything. Note that we will not warn if your input autocompletes wrongly against your intentions (we can't guess that), but only if it happens to autocomplete nothing - so you'll have to double-check autocompletion anyway...

4. Pasting any fully valid email address: John Miller <[e-mail]> - marked red until confirmed with Enter! WHY? It's useless!
5. Worse: Even when pasting a full address John Doe <[e-mail]> which is in your AB, it will be marked red until confirmed with Enter or blurred. Inconsistent, and very wrong, kinda leading the "feature" ad absurdum...

Comment 1

[Thomas D. (:thomas8)]

.

Comment 2

[Thomas D. (:thomas8)]

.

Comment 3

[Thomas D. (:thomas8)]

(In reply to Thomas D. (:thomas8) from comment #0)
Wrt comment 1, comment 2: Sorry for the noise, for some strange reason I keep mixing those tiny comment buttons for Edit Comment and Reply - maybe if they'd have a tooltip, that might help...

Comment 4

[Alessandro Castellani [:aleca]]

I'm okay with removing the red flag when the [nomatch] attribute is added, since that's not really indicative of an error.
The generated pill gets highlighted red in case an invalid address was typed, but maybe we could improve the isValidAddress method to validate the newly typed address (with a delay to not flag the input immediately as error) before the user generates the pill.
Or maybe we can prevent the creation of a pill altogether in case the address is not valid.

Let's define this behaviour properly before coding anything.
My question is:

• Do we want to alert the user of an invalid address typed before the creation of the pill?

Comment 5

[Magnus Melin [:mkmelin]]

If the address is invalid, I think we should keep it red - also keep the pill red for that case, and/or with some small warning triangle. That seems like enough alert that the address is invalid.

Comment 6

[Alessandro Castellani [:aleca]]

Yeah, I think that's good.
So, red only if it's invalid, not if it doesn't match any contact in the address book. And the red should be removed once the address is correct

Comment 7

[Alessandro Castellani [:aleca]]

Attachment: compose-invalid-new.png

Circling back to this bug to continue the improvements to the usability and discoverability of pill features.
Here's the proposal mock-up to improve the detection of wrong addresses and new recipients not present in the address book.

Autocomplete
Remove the nomatch red highlight in the autocomplete input.
This will prevent false alerts and unnecessary warnings just because an address is new.

Invalid address
Create a red pill + warning icon.
The red pill should have the same style of our high level warning notification to maintain the consistent look.
An icon also is important for color blind or visually impaired users who might miss the color variation.
We should also add an aria-label and tooltip to the icon or the whole pill that reads "john@invalid is an invalid email address".

New valid recipients
We can add an icon button allowing users to add the new address in the address book directly during compose. This icon will also help users distinguish if an address is already in the address book or it's new.

For known address we shouldn't add anything to keep the interface clean and uncluttered, and we could add a "Edit Contact" menu item in the context menu.

Comment 8

[Thomas D. (:thomas8)]

Comment on attachment 9192258 [details]
compose-invalid-new.png

Thank you for circling back to this, Alex! :-)
This looks pretty awesome, well-reflected, and well-designed.

I might need a bit of time to get used to the "Add to AB" icon (too fuzzy bc of double-icon?), but it's probably good and needed. Let's hope users won't mistake that for a button to add another address in the row... But it's quite clearly positioned inside the pill. Note: In message reader, we have star for "Add to AB" (arguably less fuzzy), should probably be consistent. How do dozens of these icons look in message reader?
Should work same as in message reader, display names should be taken over into new contact. I recall there was something odd (bug on record) about populating first/last from the display name, but we can use the current routine for now and look into the details later.

Yes, "Edit Contact" context menu for known contacts would be great. Will suffer from the same problems as in msg reader because of our poor AB design (think duplicate contacts - e.g. necessitated by our mailing list misdesign - which one to edit?), but the benefits outweigh the risks.

Btw, if there's (only) a secondary email address on some contact matching our recipient, is it known? (And in the future we'll allow more...) What about message reader?

Comment 9

[Richard Marti (:Paenglab)]

Comment on attachment 9192258 [details]
compose-invalid-new.png

Looks good. I don't think that the icon for the "Add to AB" needs to look like the one in main window's message header. We're not in the same window and it is only shown when the address isn't in a AB. So a slightly different functionality.

Comment 10

[Alessandro Castellani [:aleca]]

Attachment: 1660691-recipient-pills-match.diff

This patch takes care of implementing the mocked feature.

Here's a quick recap of what I did.

Removed

• I removed the red highlight when a typed address is not in the Address Book.
• I removed the related pref as it's not used anymore (is it necessary for something else?).

Implemented

• The pill now has a toolbarbutton that can be extended in the future to add more interaction points, if we'll ever need them. For now it only handles the "Add to Address Book" action.
• The button appears/disappear accordingly and it's refreshed after every edit.
• When clicked, the address will be saved and the pill updated. A little loading animation and a check to prevent multiple clicks is in place, in case the operation takes more than a split second.

Keyboard navigation

• The button is keyboard accessible only if the current pill is selected and the "ArrowRight" is hit without any modifier. Adding this exclusive access point helps us not to mess with the focus ring and prevent unnecessary extra steps. What do you think?

I didn't implement the "Edit Contact" context menu item to open the popup panel (same as the star icon in the message header) as it creates a bit of complications if the user edits the display name, or deletes the contact.
I will take care of that in a follow up bug as we need to listen for those edits once the popup closes.

EDIT:
If this is a good implementation, I'll add a couple of tests to cover these new scenarios.

Comment 11

[Richard Marti (:Paenglab)]

Comment on attachment 9192559 [details] [diff] [review]
1660691-recipient-pills-match.diff

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

Good improvement, thanks!

::: mail/themes/shared/mail/messengercompose.css
@@ +718,5 @@
> +.pill-button-add-contact {
> +  appearance: none;
> +  border-radius: 2px;
> +  padding-block: 1px;
> +  padding-inline: 3px;

A `padding: 1px 3px;` would be enough here.

@@ +774,5 @@
> +  color: #fff;
> +  background-color: #d70022;
> +  background-image: url("chrome://global/skin/icons/warning.svg");
> +  background-repeat: no-repeat;
> +  background-position: calc(100% - 3px);

For LTR locales this position is okay. But for RTL locales the text is over the icon.
a
.address-pill.error:not(.editing):-moz-locale-dir(rtl),
#MsgHeadersToolbar[brighttext] .address-pill.error:not(.editing):-moz-locale-dir(rtl) {
  background-position: 3px;
}
should do the trick.

Comment 12

[Alessandro Castellani [:aleca]]

Attachment: 1660691-recipient-pills-match.diff

Patch updated with Richard's changes.

Comment 14

[Thomas D. (:thomas8)]

Comment on attachment 9192774 [details] [diff] [review]
1660691-recipient-pills-match.diff

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

Thank you Alex for this awesome energy of implementing and trying new things fast!
I like the general design idea and it's looking pretty good.
I love that Ctrl/Shift+Arrow key combo continues to treat each pill as a single entity regardless of the new "Add to AB" button, as well as Arrow-Left, and only Arrow-Right reaches the extra focus stop for adding an unknown pill. That's pretty ingenious, feels quite natural, easy to discover, and intelligently adapting to different intentions as required. For lots of unknown pills, this will add lots of extra stops for Arrow-Right, but one could hold down the Arrow key a bit to move faster, or, for controlled and fast movements with single stops, use Ctrl+Arrow-Right and press Space on the target pill, apart from other shortcuts like END.

After testing this in action, there's quite a lot of devil in the detail.
Let's make this work better!

I have identified the following UX issues:

1) ux-consistency: Mouse click vs. Enter on pill's "Add to AB" button -> different focus
  - mouse: focus (automatically) moves to and remains on pill which was added to AB (that feels right)
  - Enter: focus jumps to address input, context lost, no ux-feedback to check for success. What if I want to add several nearby addresses in a row via keyboard? The focus jump will make that very cumbersome (curse Thunderbird!)
     (Btw, the focus jump is also wrong for editing a pill, bug on record iirc).
2) Space bar on focused "Add to AB" button selects pill instead
3) Deleting the only contact with a given email address from contacts sidebar (CSB) does not update recipient pill(s) to show "Add to AB" button if applicable (need to check for every deletion from AB and update all applicable pills in all open composition windows)
4) Creating a new contact in AB which has one of the recipient email addresses previously not yet known in AB does not update status of affected pills to remove the Add-to-AB buttons
5) For duplicate recipient addresses in addressing area, adding one to AB does not mark the duplicate pills as known. Worse, as the duplicate pill still has the add button, it can actually be added as a duplicate card into AB. Recipe for chaos and confusion!
6) Have search word "John" in contacts sidebar (All ABs), click on Add-to-AB button of a pill "Peter": new contact "Peter" shows up in CSB although it doesn't match the searchword, "John"
---
7) Maybe clicking the button could have some sort of visual feedback effect, on the button, and/or afterwards? Maybe turn button into green checkmark for 1 second before hiding it?
8) Would be nice to have a context menu entry: Add to AB - oh, is that part of the other bug?
9) Imo, the hover effect on Add to AB button is too subtle, both for selected and unselected pills.
10) I would sort of wish for this to work for multiple selected pills (selection should stay - preventDefault?), but we'd have to avoid ambiguities whether only the clicked pill or all selected pills get added. Maybe if for multiple selection, all the Add-to-AB buttons of all selected pills could be highlighted on hover of a single selected pill, that would work to indicate the action scope.

::: mail/base/content/mailWidgets.js
@@ +1984,5 @@
> +      addressBook.addCard(card);
> +
> +      this.pillButton.classList.remove("updating");
> +      this.updatePillStatus();
> +      this.rowInput.focus();

Not helpful.

::: mail/base/content/msgHdrView.js
@@ +1547,1 @@
>            aDocumentNode.cardDetails &&

Could the first two conditions be rewritten as follows using [Optional chaining operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining)?
`aDocumentNode?.cardDetails &&`

::: mail/components/compose/content/MsgComposeCommands.js
@@ -7294,5 @@
>      [identityElement.selectedItem.value]
>    );
>  }
>  
> -function setupAutocompleteInput(input, highlightNonMatches) {

I'm not sure if we should remove highlightNonMatches parameter and pref. Security-sensitive users might still want to be alerted as early as possible about addresses not found in their AB. What if we just flip the pref to false by default?

::: mail/components/compose/content/addressingWidgetOverlay.js
@@ +910,5 @@
>    document.getElementById("recipientsContainer").removeSelectedPills(pill);
>  }
>  
>  /**
> + * Handle the disabling of context menu items according to the types and amount

count instead of amount?

::: mail/locales/en-US/messenger/messengercompose/messengercompose.ftl
@@ +25,5 @@
>  
> +#   $email (String) - the email address
> +pill-tooltip-invalid = { $email } is not a valid e-mail address
> +
> +pill-tooltip-add-address-book = Add to Address Book

If we know to which AB we're adding, should we say that in the tooltip? (not sure)

Add to Personal Address Book?

::: mail/themes/shared/mail/messengercompose.css
@@ +773,5 @@
> +  color: #fff;
> +  background-color: #d70022;
> +  background-image: url("chrome://global/skin/icons/warning.svg");
> +  background-repeat: no-repeat;
> +  background-position: calc(100% - 3px);

Brilliant!

@@ +787,5 @@
>  }
>  
>  .address-pill.error:hover:not(.editing),
> +.address-pill.error:focus:not(.editing),
> +#MsgHeadersToolbar[brighttext] .address-pill.error:hover:not(.editing)

Should this keep the trailing comma?

::: mailnews/mailnews.js
@@ -665,5 @@
>  pref("mailnews.traits.lastIndex", 1000);
>  
> -// Address entry will highlight input that doesn't match anything in the
> -// address books.
> -pref("mail.autoComplete.highlightNonMatches", true);

I would probably keep this pref but default to false.

Comment 15

[Magnus Melin [:mkmelin]]

Comment on attachment 9192774 [details] [diff] [review]
1660691-recipient-pills-match.diff

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

I think the warning for invalid is nice.

I don't think we should pursue the addition using an icon like that: it creates an unnecessary chore for users who maybe only are doing a Reply-All with lots of unknown people. Not adding them then and there manually, will still add them *automatically* to the collected addresses address book (with slightly different trust characteristics as well, which would be lost in the current patch.)

Beside the extra chore, it's also a bit weird that existing contacts would not have any person icon, but those that do not exist would have one.

::: mail/base/content/mailWidgets.js
@@ +2114,5 @@
>          listNames.length > 0 &&
>          MailServices.ab.mailListNameExists(listNames[0].name);
>        let isNewsgroup = this.emailInput.classList.contains("news-input");
>  
> +      if (!isValid && !isMailingList && !isNewsgroup) {

Not this exact spot, but mailing lists get the "add to address book" with the current patch.

::: mailnews/mailnews.js
@@ -665,5 @@
>  pref("mailnews.traits.lastIndex", 1000);
>  
> -// Address entry will highlight input that doesn't match anything in the
> -// address books.
> -pref("mail.autoComplete.highlightNonMatches", true);

I don't think there's much reason to keep it.

Comment 16

[Alessandro Castellani [:aleca]]

I don't think we should pursue the addition using an icon like that: it creates an unnecessary chore for users who maybe only are doing a Reply-All with lots of unknown people. Not adding them then and there manually, will still add them automatically to the collected addresses address book (with slightly different trust characteristics as well, which would be lost in the current patch.)

The problem we should solve here is find a good way to highlight that a contact is new and not known in the address book.
I think it's important to let the user know it since we're removing the red color which was the only indicator used in 68.
Also an "Add contact to Address Book" action is quite handy during compose.
Maybe we could use a more subtle icon?

Beside the extra chore, it's also a bit weird that existing contacts would not have any person icon, but those that do not exist would have one.

That's a common design and usability approach when dealing with repeated elements, where icons and actions are only visible when there's something different from a "normal" state.
It's the same principle behind the fact that we show a notification only when something goes wrong in a message, rather than always showing a notification saying "Everything is good".

Comment 17

[Magnus Melin [:mkmelin]]

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

The problem we should solve here is find a good way to highlight that a contact is new and not known in the address book.
I think it's important to let the user know it since we're removing the red color which was the only indicator used in 68.

If you ask me, it can just be removed. If the address was in the address book it will be autocorrected to the correct one almost 100% of the time. If you're adding a new one, whether it's right or not is anybody's guess.

Also an "Add contact to Address Book" action is quite handy during compose.

But why is it handy? It gets added anyway as soon as you click send. I think software should things it can do automatically, and not force humans to interact when not needed.

That's a common design and usability approach when dealing with repeated elements, where icons and actions are only visible when there's something different from a "normal" state.

Sure. Maybe a subtle glowing star in the corner or something would be appropriate to indicate "this address is not previously used, and will now be collected"

Comment 18

[Alessandro Castellani [:aleca]]

All right, I think I agree with Magnus' approach.
Mostly due to the complexity we would have to deal in implementing a proper "add to AB" workflow in the compose window, with the related focus ring and multi-selection paradigm we have in place.
Also, we can stumble upon some edge cases where users write an address, add it to the AB, then realize it is wrong and they change it in the compose. Now they have an extra address slightly different they'll need to dig in tot he AB and edit/delete.
Better leave the burden to the software.

I'll implement a small circle indicator in case an address is new, with a tooltip description, and keep it simple.

Comment 19

[Alessandro Castellani [:aleca]]

Attachment: 1660691-recipient-pills-match.diff

Comment 20

[Magnus Melin [:mkmelin]]

Comment on attachment 9193378 [details] [diff] [review]
1660691-recipient-pills-match.diff

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

The orange ball looks a bit like and online indicator. Maybe it shouldn't be so in the middle but more in a corner, and use yellow like we do elsewhere for the "new" stars?

::: mail/base/content/mailWidgets.js
@@ +1809,5 @@
> +      this.pillIndicator.classList.add("pill-indicator");
> +      this.pillIndicator.setAttribute("tabindex", "-1");
> +      this.pillIndicator.setAttribute(
> +        "tooltiptext",
> +        l10nCompose.formatValueSync("pill-tooltip-not-in-address-book")

You shouldn't sync format this. You just need to set the localization on the element. (See bug 1629360)

@@ +2071,5 @@
>          MailServices.ab.mailListNameExists(listNames[0].name);
>        let isNewsgroup = this.emailInput.classList.contains("news-input");
>  
> +      if (!isValid && !isMailingList && !isNewsgroup) {
> +        this.classList.add("error");

for findability could be better to use something more elaborate, like invalid-address

@@ +2074,5 @@
> +      if (!isValid && !isMailingList && !isNewsgroup) {
> +        this.classList.add("error");
> +        this.setAttribute(
> +          "tooltiptext",
> +          l10nCompose.formatValueSync("pill-tooltip-invalid", {

same thing here re l10n.
Like https://searchfox.org/comm-central/rev/b97629284ff9f582b11ab6b7a983356730ddb850/mail/extensions/openpgp/content/ui/confirmPubkeyImport.js#25

::: mail/base/content/msgHdrView.js
@@ +1542,5 @@
>          }
>        } else if (aItem instanceof Ci.nsIAbCard) {
>          // If we don't have a card, does this new one match?
>          if (
> +          aDocumentNode?.cardDetails &&

This is protecting against a null aDocumentNode, which I don't think was intended.

@@ +1547,1 @@
>            !aDocumentNode.cardDetails.card &&

Probably instead that line can be dropped and this be
!aDocumentNode.cardDetails?.card

Comment 21

[Magnus Melin [:mkmelin]]

Comment on attachment 9193378 [details] [diff] [review]
1660691-recipient-pills-match.diff

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

::: mail/locales/en-US/messenger/messengercompose/messengercompose.ftl
@@ +26,5 @@
> +#   $email (String) - the email address
> +pill-tooltip-invalid = { $email } is not a valid e-mail address
> +
> +pill-tooltip-not-in-address-book = This address hasn't been previously used and will be collected upon sending
> +

Forgot to mention: normally it will be collected, but that's behind a pref, so I don't think we should mention the collection here. It may not be true. Perhaps

"This address is not in your address book." or better yet:
{ $email } is not in your address book

Comment 22

[Thomas D. (:thomas8)]

Comment on attachment 9193378 [details] [diff] [review]
1660691-recipient-pills-match.diff

[Midair collision with comment 21]

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

Haven't looked at this in action yet, but I have some things here.

What's your plan for addressing point 3 and 4 of my comment 14, which are still applicable by analogy?

> 3) Deleting the only contact with a given email address from contacts
> sidebar (CSB) does not update recipient pill(s) to show [Not-in-AB*] indicator
> if applicable (need to check for every deletion from AB and update all
> applicable pills in all open composition windows)
>
> 4) Creating a new contact in AB which has one of the recipient email
> addresses previously not yet known in AB does not update status of affected
> pills to remove the [Not-in-AB*] indicator

::: mail/components/compose/content/MsgComposeCommands.js
@@ +7295,5 @@
>    );
>  }
>  
> +/**
> + * Set up the autocomplete search parameters to the composer input fields.

Perhaps this?
 * Set up autocomplete search parameters for address inputs of inbuilt headers.

@@ +7297,5 @@
>  
> +/**
> + * Set up the autocomplete search parameters to the composer input fields.
> + *
> + * @param {Element} input - The composer input field.

* @param {Element} input - The address input of an inbuilt header field.

::: mail/locales/en-US/messenger/messengercompose/messengercompose.ftl
@@ +25,5 @@
>  
> +#   $email (String) - the email address
> +pill-tooltip-invalid = { $email } is not a valid e-mail address
> +
> +pill-tooltip-not-in-address-book = This address hasn't been previously used and will be collected upon sending

That's not correct. We don't know if the address has been used before or not, and it will not necessarily be collected, because it's possible to de-activate collecting addresses upon sending, as well as to collect into another AB. To make correct statements here, imo we need to read the prefs and dynamically adjust the tooltip, something like this (using fluent flexible syntax as required):

Pseudo code indicating the dynamic parts:

This address is not yet in your address books, {but it will be added to {"Collected Addresses"} when sending.}
This address is not yet in your address books, {and because of your settings, it will not be added to any address book when sending}

Or (slightly less dynamic and informative, and I'm not sure how good "collecting an address" scales into other languages):

This address is not yet in your address books, {but it will be collected when sending.}
This address is not yet in your address books, {and because of your settings, it will not be collected when sending}

Comment 23

[Thomas D. (:thomas8)]

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

Forgot to mention: normally it will be collected, but that's behind a pref,
so I don't think we should mention the collection here. It may not be true.

I suggest to read the pref and adjust the tooltip accordingly.
If you don't mention collection, you're back to the scenario which you wanted to avoid - we'll be stressing the user with information that will be hard to act on at this time, because we don't offer the "Add to AB" button now. Users can mistake this as an indirect appeal to save the address now.

Perhaps
"This address is not in your address book." or better yet:
{ $email } is not in your address book

I'm not sure if { $email } is useful here, as the address is fully visible right under the tooltip. Imho this would make the tooltip even harder to read and understand.

Comment 24

[Magnus Melin [:mkmelin]]

Let's not overcomplicate things with dynamic tooltips.
Re the email: well, it's visible, but if you also include display name in the addressing area, it might not be easily noticeable that a one char is off, so - it can be good to put focus on the actual address.

Comment 25

[Alessandro Castellani [:aleca]]

Maybe it shouldn't be so in the middle but more in a corner, and use yellow like we do elsewhere for the "new" stars?

I put it in the top right corner to follow the paradigm of new email indicators in folder pane and message pane.
I'm using alight orange on a default theme and yellow on a dark theme.
The yellow is not really visible with the light grey background color of a pill.

What's your plan for addressing point 3 and 4 of my comment 14, which are still applicable by analogy?

I think we should handle those edge cases in a dedicated bug.

I suggest to read the pref and adjust the tooltip accordingly.

I kind of agree with this.
Having the indicator and tooltip might prompt the user to take an action that we don't offer.
So having an indication of what's going to happen based on pref might be good.
Maybe also this in a follow up bug to improve the UX of this area?

Comment 26

[Alessandro Castellani [:aleca]]

Attachment: 1660691-recipient-pills-match.diff

Comment 27

[Alessandro Castellani [:aleca]]

Attachment: Screenshot from 2020-12-16 15-23-45.png

Screenshot of the implementation.

Comment 28

[Richard Marti (:Paenglab)]

Comment on attachment 9193637 [details] [diff] [review]
1660691-recipient-pills-match.diff

Not a super fan of this "not in AB" indicator but it's not intrusive - so okay for me.

Unfortunately the "not in AB" tooltip shows only when I hover the pill-indicator and not on the rest of the pill.

Comment 29

[Alessandro Castellani [:aleca]]

Unfortunately the "not in AB" tooltip shows only when I hover the pill-indicator and not on the rest of the pill.

Mh, maybe the tooltip could be on the entire pill.

Comment 30

[Alessandro Castellani [:aleca]]

Attachment: 1660691-recipient-pills-match.diff

I updated the patch to have a more subtle indicator with a cut out effect (screenshot coming).

I tried using the data-l10n-attrs attribute to assign the tooltiptext to the pill but that never worked, and the whole label was getting stripped away when adding a fluent string.
I'm assigning the string directly to the attribute. Let me know if you know of a method that works in order to attach a fluent string only to an attribute without overriding the existing label.

The tooltip is applied to the whole pill to make it easier to trigger.

Comment 31

[Alessandro Castellani [:aleca]]

Attachment: Screenshot from 2020-12-17 16-01-47.png

Comment 32

[Richard Marti (:Paenglab)]

Comment on attachment 9193838 [details] [diff] [review]
1660691-recipient-pills-match.diff

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

I like this "not in AB" indication.

::: mail/themes/shared/mail/messengercompose.css
@@ +724,5 @@
> +  margin-top: -18px;
> +  transition: fill .2s ease, stroke .2s ease;
> +}
> +
> +#MsgHeadersToolbar[brighttext] .pill-indicator {

Please use `:root[lwt-tree]` instead of `#MsgHeadersToolbar[brighttext]`. Then the stroke is in sync with #msgSubject and .address-container and works also with third patry themes correctly.

Comment 33

[Magnus Melin [:mkmelin]]

Comment on attachment 9193838 [details] [diff] [review]
1660691-recipient-pills-match.diff

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

Alright! r=mkmelin

Comment 34

[Alessandro Castellani [:aleca]]

Attachment: 1660691-recipient-pills-match.diff

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7f80b142cdce
Remove the nomatch red highlight in compose autocomplete and improve the invalid and new email pills UI. r=mkmelin,ui-r=Paenglab

Comment 36

[Magnus Melin [:mkmelin]]

(In reply to Alessandro Castellani (:aleca)[PTO: Dec 21 - Jan 3] from comment #30)

Created attachment 9193838 [details] [diff] [review]
1660691-recipient-pills-match.diff

I updated the patch to have a more subtle indicator with a cut out effect (screenshot coming).

I tried using the data-l10n-attrs attribute to assign the tooltiptext to the pill but that never worked, and the whole label was getting stripped away when adding a fluent string.
I'm assigning the string directly to the attribute. Let me know if you know of a method that works in order to attach a fluent string only to an attribute without overriding the existing label.

Actually, I remembered now. I think you should set the attributes to localized, using something like data-l10n-attrs="tooltiptext" and data-l10n-args then dynamically