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

People: Invites: Redirect and display notice on invite dismiss #1184

Merged
merged 4 commits into from
Dec 3, 2015

Conversation

lezama
Copy link
Contributor

@lezama lezama commented Dec 2, 2015

Fixes #1146

This PR displays a notice when declining and invite and redirects the user to the reader

image

To test

  • Checkout update/invites-redirect-and-notice-on-dismiss

Decline Invite

  • Create an invite at $site/wp-admin/users.php?page=wpcom-invite-users
  • In the invite email, make note of the invitation key
  • While logged in, go to /accept-invite/$site/$invite_key
  • Dismiss the notice.
  • Do you get redirected to the reader?
  • Do you get the dismiss notice?

cc @ebinnion

@lezama lezama added this to the People Management: m6 milestone Dec 2, 2015
@lezama lezama added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. People Management labels Dec 2, 2015
@lezama lezama self-assigned this Dec 2, 2015
@lezama lezama added the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Dec 2, 2015
export default React.createClass( {

displayName: 'InviteAcceptLoggedIn',

render() {
let userObject = user.get(),
const { user } = this.props,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@ebinnion
Copy link
Contributor

ebinnion commented Dec 2, 2015

Code-wise, this LGTM. Functionally, I only noticed the one thing about notices.

I wouldn't expect a green notice for declining here. Pinging @rickybanister for a design review and advice.

@rickybanister
Copy link

I think the blue is-info notice is the correct way to go here.

screen shot 2015-12-02 at 6 31 48 pm

And just to be sure—you'd only be redirected to the Reader if you decline after clicking a link in an email while logged in—that is to say you are redirected to /.

If you click the join or decline button in a notification it would not redirect you anywhere (as in the screenshot above)

@lezama
Copy link
Contributor Author

lezama commented Dec 3, 2015

Thanks! changed it to is-info

And just to be sure—you'd only be redirected to the Reader if you decline after clicking a link in an email while logged in—that is to say you are redirected to /.

Yes

If you click the join or decline button in a notification it would not redirect you anywhere (as in the screenshot above)

We are not dealing with notifications right now, but we will have this in mind..

@lezama lezama removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Dec 3, 2015
lezama added a commit that referenced this pull request Dec 3, 2015
…notice-on-dismiss

People: Invites: Redirect and display notice on invite dismiss
@lezama lezama merged commit 85ca38c into master Dec 3, 2015
@lezama lezama deleted the update/invites-redirect-and-notice-on-dismiss branch December 3, 2015 14:55
@scruffian scruffian removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 3, 2015
scruffian added a commit that referenced this pull request Feb 22, 2016
…components to make it easier to work on as a group.

This is a precursor to #1184 and #2482, so that we can work on these tasks in different PRs without conflicts.
drewblaisdell pushed a commit that referenced this pull request Feb 22, 2016
…components to make it easier to work on as a group.

This is a precursor to #1184 and #2482, so that we can work on these tasks in different PRs without conflicts.
drewblaisdell pushed a commit that referenced this pull request Feb 23, 2016
…components to make it easier to work on as a group.

This is a precursor to #1184 and #2482, so that we can work on these tasks in different PRs without conflicts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants