Remove red font color when entering new recipients not yet in AB (no autocomplete matches found - so what?)
Categories
(Thunderbird :: Message Compose Window, enhancement)
Tracking
(thunderbird_esr78 wontfix)
Tracking | Status | |
---|---|---|
thunderbird_esr78 | --- | wontfix |
People
(Reporter: thomas8, Assigned: aleca)
References
Details
Attachments
(2 files, 7 obsolete files)
74.11 KB,
image/png
|
Details | |
12.57 KB,
patch
|
aleca
:
review+
aleca
:
ui-review+
|
Details | Diff | Splinter Review |
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.
- 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?
- So if i have
John Doe <john@example.com>
in my AB, and typeJohn Miller <john.miller@foo.bar>
, 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? - 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...
- Pasting any fully valid email address:
John Miller <john.miller@foo.bar>
- marked red until confirmed with Enter! WHY? It's useless! - Worse: Even when pasting a full address
John Doe <john@example.com>
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...
Reporter | ||
Comment 1•4 years ago
•
|
||
.
Reporter | ||
Comment 2•4 years ago
•
|
||
.
Reporter | ||
Comment 3•4 years ago
|
||
(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...
Assignee | ||
Comment 4•4 years ago
|
||
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•4 years ago
|
||
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.
Assignee | ||
Comment 6•4 years ago
|
||
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
Assignee | ||
Comment 7•4 years ago
|
||
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.
Reporter | ||
Comment 8•4 years ago
|
||
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•4 years ago
|
||
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.
Assignee | ||
Comment 10•4 years ago
•
|
||
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.
Assignee | ||
Updated•4 years ago
|
Comment 11•4 years ago
|
||
Assignee | ||
Comment 12•4 years ago
|
||
Patch updated with Richard's changes.
Reporter | ||
Comment 14•4 years ago
•
|
||
Comment 15•4 years ago
|
||
Assignee | ||
Comment 16•4 years ago
|
||
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•4 years ago
|
||
(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"
Assignee | ||
Comment 18•4 years ago
•
|
||
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.
Assignee | ||
Comment 19•4 years ago
|
||
Comment 20•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 21•4 years ago
|
||
Reporter | ||
Comment 22•4 years ago
•
|
||
Reporter | ||
Comment 23•4 years ago
•
|
||
(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•4 years ago
|
||
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.
Assignee | ||
Comment 25•4 years ago
|
||
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?
Assignee | ||
Comment 26•4 years ago
|
||
Assignee | ||
Comment 27•4 years ago
|
||
Screenshot of the implementation.
Comment 28•4 years ago
|
||
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.
Assignee | ||
Comment 29•4 years ago
|
||
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.
Assignee | ||
Comment 30•4 years ago
|
||
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.
Assignee | ||
Comment 31•4 years ago
|
||
Comment 32•4 years ago
|
||
Comment 33•4 years ago
|
||
Assignee | ||
Comment 34•4 years ago
|
||
Assignee | ||
Updated•4 years ago
|
Comment 35•4 years ago
|
||
Pushed by mkmelin@iki.fi:
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•4 years ago
|
||
(In reply to Alessandro Castellani (:aleca)[PTO: Dec 21 - Jan 3] from comment #30)
Created attachment 9193838 [details] [diff] [review]
1660691-recipient-pills-match.diffI 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 thetooltiptext
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
Description
•