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

user-form: only show "Passwords do not match" when there is confirmation password; make required when password #314

Merged
merged 1 commit into from
Mar 9, 2021

Conversation

himdel
Copy link
Collaborator

@himdel himdel commented Mar 3, 2021

This fixes part of https://issues.redhat.com/browse/AAH-362 ...

"Passwords do not match" alert under password confirmation appears when user types in the "password" field

... and part of https://issues.redhat.com/browse/AAH-360

Password confirmation field should be mandatory

@sbuenafe-rh , @ZitaNemeckova I don't see any patternfly guidelines on how the password field + confirm password field combo should behave, so this is my best guess based on the assumption that the "Passwords do not match" message is not supposed to appear without any password there. (Which could be wrong given some of the effects below.)

So, this describes the create user/edit user form behaviour with this change in, in all the variants, I'm pretty sure the Save button is correct, not necessarily the rest :) .. I've added a question mark where I think something may be wrong, but if we make the field invalid when there is a password but no confirmation password, it implies that the message would appear when the user types in the password field.

  • Create new user:

    • empty password:
      • empty confirm:
        Username, Password, Confirm are required
        Save is disabled
        no message, no invalid fields
      • nonempty confirm:
        Username, Password, Confirm are required
        Save is disabled
        yes message, confirm invalid
    • nonempty password:
      • empty confirm:
        Username, Password, Confirm are required
        Save is disabled
        no message, no invalid fields ❓
      • nonempty bad confirm:
        Username, Password, Confirm are required
        Save is disabled
        yes message, confirm invalid
      • nonempty good confirm:
        Username, Password, Confirm are required
        Save is enabled
        no message, confirm invalid
  • Edit user:

    • placeholder password:
      • placeholder confirm:
        only Username required
        Save is enabled
        no message, no invalid fields
      • nonempty confirm:
        only Username required
        Save is disabled
        yes message, confirm invalid
    • nonempty password:
      • placeholder confirm:
        Username and Confirm are required
        Save is disabled
        no message, no invalid fields ❓
        maybe there should not be a placeholder confirm with nonempty password and appear empty ❓
      • nonempty bad confirm:
        Username and Confirm are required
        Save is disabled
        yes message, confirm invalid
      • nonempty good confirm:
        Username and Confirm are required
        Save is enabled
        no message, confirm invalid

As for the mandatory confirmation field, I've made it mandatory only when the password is required (new user), or already entered (edit user, after changing the password), hope that makes sense :).


https://issues.redhat.com/browse/AAH-362 also contains...

All fields should be mandatory(double check it's needed)

As far as the api/model is concerned, only username/username+password is required.
So the question is: should we require First name, Last name, and Email as well? (If so, I'll add it, it's a trivial change.)

…ion password; make required when password

Create new user:
  empty password:
    empty confirm:
      Username, Password, Confirm are required; Save is disabled; no message, no invalid fields
    nonempty confirm:
      Username, Password, Confirm are required; Save is disabled; yes message, confirm invalid
  nonempty password:
    empty confirm:
      Username, Password, Confirm are required; Save is disabled; no message, no invalid fields
    nonempty bad confirm:
      Username, Password, Confirm are required; Save is disabled; yes message, confirm invalid
    nonempty good confirm:
      Username, Password, Confirm are required; Save is enabled; no message, confirm invalid

Edit user:
  placeholder password:
    placeholder confirm:
      only Username required; Save is enabled; no message, no invalid fields
    nonempty confirm:
      only Username required; Save is disabled; yes message, confirm invalid
  nonempty password:
    placeholder confirm:
      Username and Confirm are required; Save is disabled; no message, no invalid fields
    nonempty bad confirm:
      Username and Confirm are required; Save is disabled; yes message, confirm invalid
    nonempty good confirm:
      Username and Confirm are required; Save is enabled; no message, confirm invalid
Copy link
Member

@ZitaNemeckova ZitaNemeckova left a comment

Choose a reason for hiding this comment

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

LGTM 👍 UX approved as well 👍

@ZitaNemeckova ZitaNemeckova merged commit f9ec575 into ansible:master Mar 9, 2021
@himdel himdel deleted the user-form-passwords-362 branch March 9, 2021 13:12
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

2 participants