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

Take your account with you UI changes #17221

Merged
merged 3 commits into from Aug 23, 2017
Merged

Conversation

ewjordan
Copy link
Contributor

@ewjordan ewjordan commented Aug 23, 2017

Item 1 in https://docs.google.com/document/d/1R8oBFcixpN1X_FQJq9G5-l3AbaqjGSqvD9oDwL00Wns/edit#heading=h.ezfouxcqg3na

(Update) New text for enter_parent_email: "Enter your parent or guardian's email address (for password recovery)"

Personal word login (with email):
personal-login-words

Personal word login (no email - note, parenthetical "to recover password" is now removed, though it was not when I took this screenshot):
personal-login-words-no-email

Personal picture login:
personal-login-picture

Delete account button:
delete-account

@ewjordan
Copy link
Contributor Author

@poorvasingal please take a look at the images, see if these are what you had in mind. I reordered some elements the way the doc seemed to suggest.

Copy link
Member

@breville breville left a comment

Choose a reason for hiding this comment

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

I'm not very familiar with this UI or the code behind it, but the code changes seem reasonable.

@breville
Copy link
Member

Oh, is there any testing of this UI? Seems like it would be great to have some.

@poorvasingal
Copy link
Contributor

Cool, looks good. Just a couple of comments:

  • Can we actually say "Enter in your parent or guardian's email address (for password recovery)" instead? I realized that the way I had it in the spec makes it sound like we're saying you want to recover your password right now.
  • Why are we changing the note for Deleting accounts to include the fact that this does not delete student accounts? I don't think we should do that since we're planning on changing that behavior soon.

@breville
Copy link
Member

Suggestion: "Enter your parent or..." rather than "Enter in your parent or..."

@ewjordan
Copy link
Contributor Author

@poorvasingal, the deleting accounts text is that way right now in production, that's not part of this changeset (this change just makes the header teal). Would you like me to change that to something else?

@ewjordan
Copy link
Contributor Author

(Spoke with Poorva, we'll leave the delete text alone for now)

@ewjordan ewjordan merged commit d1a1dfa into staging Aug 23, 2017
@ewjordan ewjordan deleted the take-your-account-ui-changes branch August 23, 2017 18:21
.field
= form.label :password, maxlength: 255
= form.password_field :password, autocomplete: 'off', maxlength: 255
.field
= form.label :password_confirmation
= form.password_field :password_confirmation, autocomplete: 'off', maxlength: 255
- if current_user.secret_word_account?
%h3= t('user.confirm_secred_words')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Grr, I typoed this by accident, I don't know how I didn't catch that. Fixing in another PR

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.

None yet

3 participants