Skip to content
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

Send invite over email if possible #2112

Merged
merged 17 commits into from
Nov 10, 2020
Merged

Send invite over email if possible #2112

merged 17 commits into from
Nov 10, 2020

Conversation

Twixes
Copy link
Member

@Twixes Twixes commented Oct 29, 2020

Changes

This makes invites emailed to target email on creation. Resolves #1611.

Invite Email

A few considerations before merging:

  1. What if SITE_URL is not defined?
    • Decided that emailing just won't be available, since we can't send full URLs then, but we should definitely explain in the docs why is that so and what the user should configure for emailed invites to work.
  2. The send attempt should be communicated in the frontend.
    • Done.
  3. The design of the invite email could be refined. Feedback or designs welcome.
    • Refined.
  4. Wording could be changed from "Create Invite" to "Send Invite" if email sending is available.
    • Should be OK with point 2. being solved.

Checklist

  • All querysets/queries filter by Organization, Team, and User (if this PR affects ANY querysets/queries).
  • Django backend tests (if this PR affects the backend).

@Twixes Twixes requested a review from paolodamico October 29, 2020 15:58
@Twixes Twixes marked this pull request as draft October 29, 2020 15:58
@timgl timgl temporarily deployed to posthog-invite-improvem-ocvuxk October 29, 2020 15:58 Inactive
@Twixes Twixes temporarily deployed to posthog-invite-improvem-ocvuxk October 29, 2020 16:08 Inactive
@paolodamico
Copy link
Contributor

Took a stab at the design (found here), @yakkomajuri may want to tweak the copy.

@yakkomajuri
Copy link
Contributor

That's fine by me @paolodamico

@Twixes Twixes temporarily deployed to posthog-invite-improvem-ocvuxk October 30, 2020 11:04 Inactive
@Twixes
Copy link
Member Author

Twixes commented Oct 30, 2020

Also added invite expiry after 3 days.
Here's the refined email:

Screen Shot 2020-10-30 at 15 10 36

Note: changed the default weight of email text to 400 (regular), since 300 (light) was too thin for copy.
@paolodamico

@Twixes Twixes marked this pull request as ready for review October 30, 2020 14:07
@Twixes Twixes temporarily deployed to posthog-invite-improvem-ocvuxk October 30, 2020 14:07 Inactive
@Twixes Twixes temporarily deployed to posthog-invite-improvem-ocvuxk October 30, 2020 14:12 Inactive
@paolodamico
Copy link
Contributor

Great @Twixes, lmk if you want me to review now

@Twixes
Copy link
Member Author

Twixes commented Oct 30, 2020

Yep, go ahead.

@Twixes Twixes temporarily deployed to posthog-invite-improvem-ocvuxk October 30, 2020 14:16 Inactive
Copy link
Contributor

@paolodamico paolodamico left a comment

Choose a reason for hiding this comment

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

Looking good!

Some general UX suggestions,

  1. When creating an invite and email is not available, I think it's a better experience to just show the invite link (and allow copying to clipboard) immediately in that window. Think it's better to remove the invite link from the list table. If they lost the link, they can just create another one (seems pretty common behavior).
    • Related to the above, I would add an "expires at" column.
  2. The social buttons on the /signup/{invite} page say "Sign in with GitHub", when it should be "Sign up ...". The new design just says "Continue with [provider]" to simplify things.
  3. Maybe out-of-scope for this PR (feel free to open a separate issue if that's the case). When you open a link to an invalid invite (e.g. I created an invite link and then revoked it), you're redirected to the login. I believe a better user experience would be to get an error message saying the invite link you have is invalid or expired.

<i>Invites emailed by PostHog coming soon.</i>
{user?.are_invite_emails_available
? 'PostHog will then email it to the address.'
: "Since this PostHog instance isn't configured for emailing invites, remember to share the link!"}
</p>
<Input
Copy link
Contributor

Choose a reason for hiding this comment

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

Per the new design guidelines, use a label instead of the add-on and I also suggest adding a placeholder with an example email (maybe even using the domain from the current user? GQ)

Copy link
Contributor

Choose a reason for hiding this comment

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

As an improvement, submitting the form with the enter key would be great.

"""
return bool(settings.EMAIL_HOST)
return bool(settings.EMAIL_HOST) and (not with_absolute_urls or settings.SITE_URL is not None)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all emails require absolute URLs to be available (at least all emails that call this method).

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what action you suggest here: always require SITE_URL?

<strong>{{ invite.created_by.first_name }}</strong> ({{ invite.created_by.email }}) invited you to join organization {{ invite.organization.name }} on PostHog.<br/>
PostHog is an open-source product analytics tool that will help you better understand your users.
</p>
<p><a class="button" href="{{ invite }}">Join {{ invite.organization.name }} Now</a></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Think the button centered would look better.

<div class="container">
<div class="section">
<h1>Join {{ invite.organization.name }} on PostHog</h1>
<p>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think adding the image from the design would look pretty cool.

@timgl
Copy link
Collaborator

timgl commented Nov 6, 2020

Merge conflicts

Copy link
Contributor

@paolodamico paolodamico left a comment

Choose a reason for hiding this comment

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

Looks good! Made a few minor improvements,

  • Adjusted copy for instructions and submit button on the invite modal
  • Auto-focus the input when opening the modal and automatically copying link to clipboard if email is not sent.
  • Allow submitting the email address with the Enter key & wrap up in a form for auto-capture to properly register this.
  • Adjusted email styles and added hero image

@paolodamico paolodamico merged commit f0ea1bd into master Nov 10, 2020
@paolodamico paolodamico deleted the invite-improvements branch November 10, 2020 09:13
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.

Invite team members via email
4 participants