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

successMessages vs errorMessages #37

Open
ayselafsar opened this issue Mar 13, 2016 · 7 comments
Open

successMessages vs errorMessages #37

ayselafsar opened this issue Mar 13, 2016 · 7 comments

Comments

@ayselafsar
Copy link
Collaborator

We were setting errorMessages even if input value is validated but I think errorMessages should include only errors. I created successMessages to store result of validation but we are not using successMessages except activeEntryTests.js. We might set errorMessages as false if validation is acceptable and check whether keys of errorMessages are false or not. What do you think?

@awatson1978
Copy link

Starting to get through backlog. This sudden announcement by MDG that they're ending the free *.meteor.com hosting has derailed the last two weeks of work. Sorry about that.

So, yeah... this is sort of how I originally envisioned things would work. Only one variable is needed for login, and two variables creates opportunities for indeterminate states.... where things are succeeding and failing at the same time. With only a single variable, we just decide what the default state is... are people assumed to be logged in by default, in which case we check for errors; or are people assumed to be denied access by default, in which case we check for credentialed success.

At this point, I would be prone to completing the refactor, switching to successMessages, and dropping errorMessages altogether if possible. We started down this path, so might as well finish it.

@awatson1978
Copy link

The bigger issue, in my mind, is that we started down this refactor without a UI goal or business goal clearly articulated. So, I've been collecting some screenshots of UI that successMessages could be used to implement, and here's what I found:

screen shot 2016-03-28 at 11 57 10 pm

screen shot 2016-03-02 at 9 41 27 am 2

Think we could converge on a UI like this?

@ayselafsar
Copy link
Collaborator Author

Keeping only one variable is the best, checking more than one variable is painful. The confusing part for me is how we can store and check success/error messages in one variable. Because we need to show the errors to user if Sign In or Sign Up is failed. Besides that, we need a success message if a reset link mail is sent when the user forgot the password. I need your suggestion to achieve a mechanism like that?

@awatson1978
Copy link

Ignore the reset link for now. That should be in a separate issue, and we'll get to it next.

The benefit of having the errorMessage is that it can catch and display any error. The benefit of the successMessage is that we can check off the exact requirements for successful login.

Lets just assume that the user is denied access by default (because HIPAA and other security stuff), and that successMessage keeps reverting back to false. When we validate the input, we go through and check our list of requirements; if our list/dictionary of successMessages is completely positive, we log in.

Narrow down the errorMessage variable so that the only thing it does is display data to the UI, and isn't involved in the login flow.

@ayselafsar
Copy link
Collaborator Author

That makes sense. What about converging on the UI above? Should we replace the existed UI with the new?

@awatson1978
Copy link

We have to think about that, and give some design consideration to it. The UI is currently pretty good, albeit plain. One thing to keep in mind is that buttons don't move around on mobile when error messages are displayed. Changing layout dynamically is bad. Otherwise, we'll just need to figure out someplace to put the extra GUI elements.

@ayselafsar
Copy link
Collaborator Author

I think so. It is clear now. Thanks!

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

No branches or pull requests

2 participants