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

Fix alerts for confirmation when inviting new user #2937

Merged
merged 8 commits into from
Sep 28, 2017

Conversation

jm-wanja
Copy link
Contributor

@jm-wanja jm-wanja commented Sep 25, 2017

Resolve #2928

How To Test

  1. Invite a new user from the accounts admin dashboard and the invite owner form in the shop admin.
  2. Observe that the success alert is a toast and the error alert is an inline alert

screen shot 2017-09-27 at 12 51 43 pm

screen shot 2017-09-27 at 12 51 23 pm

@rymorgan
Copy link
Contributor

@jm-wanja Now that I've thought about this a little more, we should use a toast alert in this instance. It's a temporary alert and confirmation that the email has been sent.

@jm-wanja
Copy link
Contributor Author

@rymorgan this is noted. Should we also use a toast alert for the invite owner form that is on the Shop admin?

@rymorgan
Copy link
Contributor

@mikemurray @kieckhafer -- I'm changing my mind again on this. Do y'all have an opinion on the best UX here? Inline alert or toast alert confirmation when an invite email has been sent.

@kieckhafer
Copy link
Member

I'm a fan of the toasts, as it's less intrusive and doesn't alter any of the existing display when it shows.

@mikemurray
Copy link
Member

The toast is probably better in this situation since it's a success, and you only need to see that it succeed for a second to "get it".

@rymorgan
Copy link
Contributor

@jm-wanja So, yes then let's go to toast for both here and invite owner. The caveat is that if there is an error, that needs to happen inline so that they know where the error is and how to correct it.

@jm-wanja jm-wanja changed the title Fix inline alert position when inviting new user [WIP] Fix inline alert position when inviting new user Sep 27, 2017
@jm-wanja jm-wanja changed the title [WIP] Fix inline alert position when inviting new user Fix inline alert position when inviting new user Sep 27, 2017
@jm-wanja
Copy link
Contributor Author

@rymorgan I have implemented the new changes, toasts alerts for success and inline alert for error. I have done this for both invite admin and invite owner.

@jm-wanja jm-wanja changed the title Fix inline alert position when inviting new user Fix alerts for confirmation when inviting new user Sep 27, 2017
Copy link
Collaborator

@brent-hoover brent-hoover left a comment

Choose a reason for hiding this comment

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

Tested. Verified fixed

@mikemurray mikemurray removed the request for review from rymorgan September 28, 2017 20:33
@mikemurray mikemurray merged commit acd18ef into marketplace Sep 28, 2017
@mikemurray mikemurray deleted the wanja-fix-issue-2928 branch September 28, 2017 20:33
@spencern spencern mentioned this pull request Oct 11, 2017
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.

5 participants