Skip to content

Email verification feedback #4534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 12, 2018
Merged

Conversation

yeraydiazdiaz
Copy link
Contributor

Fixes #3590 , here are some captures:

email_verification_verified_warning
email_verification_unverified_none
email_verification_unverified_soft_bounce
email_verification_unverified_hard_bounce
email_verification_unverified_spam_complaint

Copy suggestions are welcomed and encouraged.

I couldn't find any tests for the actual rendering of the views, have I missed something or is this by design?

Copy link
Contributor

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM visually. :)

@nlhkabu
Copy link
Contributor

nlhkabu commented Aug 11, 2018

Hi @yeraydiazdiaz thanks for this PR 😁

From an accessibility perspective, we should give another indicator other than color to indicate status. This is for color blind users.

To do this, I think we should add a small icon inside each verified / unverified badge.

I'm thinking:

Badge class Badge text Badge icon tooltip text / message
badge--success Verified fa-check -
badge--warning Verified fa-exclamation-circle Intermittent delivery problems may lead to verification loss
badge--danger Unverified fa-spinner Verification in progress
badge--danger Unverified fa-exclamation-circle Too many delivery problems
badge--danger Unverified fa-exclamation-circle Hard failure during delivery
badge--danger Unverified fa-ban Email from PyPI being treated as spam

Maybe we could also add a FAQ under https://pypi.org/help/#verified-email that describes each of these messages in detail?

We could link to the FAQ either from the tooltip, or by adding a link above the table, something like:

For more information on verification status', visit the [FAQ]

What do you think?

@yeraydiazdiaz
Copy link
Contributor Author

Adding a badge makes total sense.

I don't think you can add a link in the tooltip though as the text is added in the arial-label attribute and putting and a tag would break the HTML.

@nlhkabu
Copy link
Contributor

nlhkabu commented Aug 11, 2018

Ok, I thought that might be the case. In this case, we can add a separate link. Maybe, though, we can do this in a separate PR to keep this granular?

@yeraydiazdiaz
Copy link
Contributor Author

Updated with icons and copy tweaks
screen shot 2018-08-11 at 19 46 36
screen shot 2018-08-11 at 19 46 07
screen shot 2018-08-11 at 19 44 49
screen shot 2018-08-11 at 19 43 58
screen shot 2018-08-11 at 19 43 32
screen shot 2018-08-11 at 19 43 04

@nlhkabu
Copy link
Contributor

nlhkabu commented Aug 11, 2018

Looking good @yeraydiazdiaz! One thing - could we move the icons to the left of the text?
I'm also thinking that using exclamation-triangle might be better / clearer?

@dstufft are you happy with this text?

@dstufft
Copy link
Member

dstufft commented Aug 11, 2018

The text looks fine to me.

@yeraydiazdiaz
Copy link
Contributor Author

Updated location and exclamation-triangle:

screen shot 2018-08-12 at 09 45 16

screen shot 2018-08-12 at 09 47 22

screen shot 2018-08-12 at 09 47 51

screen shot 2018-08-12 at 09 49 11

screen shot 2018-08-12 at 09 49 41

screen shot 2018-08-12 at 09 50 07

@pradyunsg
Copy link
Contributor

A super minor point -- we can probably drop the tick in the verified case, since it's redundant.

@nlhkabu
Copy link
Contributor

nlhkabu commented Aug 12, 2018

I disagree @pradyunsg - I we should be explicit over implicit :) Also, visually, it is more consistent.

@dstufft dstufft merged commit bb7e4a8 into pypi:master Aug 12, 2018
@dstufft
Copy link
Member

dstufft commented Aug 12, 2018

Thanks! ✨

@yeraydiazdiaz yeraydiazdiaz deleted the email-status-feedback branch August 12, 2018 15:50
@pradyunsg
Copy link
Contributor

I feel that having text only badge would be consistent with the badge beside it. ;)

As I said, it's a minor point anyway; I don't have a problem going either way and slightly prefer the one I'm suggesting. I'm not going to push for it. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants