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

When name is missing from "Add new server" dialog, only the name field should be highlighted and "Add" button shouldn't be disabled #438

Closed
3 tasks done
jasonblais opened this issue Feb 16, 2017 · 10 comments

Comments

@jasonblais
Copy link
Contributor

jasonblais commented Feb 16, 2017

I confirm (by marking "x" in the [ ] below):


Summary
When name is missing from "Add new server" dialog, only the name field should be highlighted and "Add" button shouldn't be disabled

Steps to reproduce

  1. Go to File > Settings
  2. Press "Add New Server"
  3. Press the "Add" button

Expected behavior
"Name is required." error message with the name field highlighted

Observed behavior
"Name is required." error message with the name and URL fields highlighted, and the "Add" button disabled

@jasonblais
Copy link
Contributor Author

@jnugh a small bug I missed when reviewing #415. Not sure if that's something you'd have time to help with?

@jnugh
Copy link
Contributor

jnugh commented Feb 18, 2017

Actually the current behavior was intentional - When you click the Add button without filling any fields there are actually 2 errors, these fields are marked red. I don't think that it's a good idea to validate the form only until an error has been found.
I don't show the error next to the fields but in the bottom left corner where only one error message fits.
The Add button is disabled until you resolve the errors.

@jasonblais
Copy link
Contributor Author

@jnugh This seems to be inconsistent with Mattermost server error messages, hence making the experience less obvious (or familiar) to the end user.

@jasonblais jasonblais added this to the v3.7.0 milestone Feb 21, 2017
@jasonblais jasonblais changed the title When name is missing from "Add new server" dialog, only the name field should be highlighted and "Add" button shouldn't be disabled [Help Wanted] When name is missing from "Add new server" dialog, only the name field should be highlighted and "Add" button shouldn't be disabled Feb 26, 2017
@jasonblais jasonblais changed the title [Help Wanted] When name is missing from "Add new server" dialog, only the name field should be highlighted and "Add" button shouldn't be disabled When name is missing from "Add new server" dialog, only the name field should be highlighted and "Add" button shouldn't be disabled Mar 22, 2017
@jasonblais jasonblais modified the milestones: v3.9.0/3.10.0, v3.7.0/3.8.0 Mar 29, 2017
@jasonblais jasonblais modified the milestones: v3.9.0, v3.8.0 Jul 21, 2017
@jasonblais jasonblais removed the Hacktoberfest null label Nov 3, 2017
@jasonblais jasonblais modified the milestones: v3.9.0, v3.10.0 Nov 5, 2017
@amyblais amyblais removed this from the v4.2.0 milestone Mar 21, 2018
@kethinov
Copy link
Contributor

I can't reproduce this one in the latest master. I believe it may have been fixed at some point.

@jasonblais
Copy link
Contributor Author

Hey @kethinov, thanks for looking into this! It does seem to reproduce for me

image

(The "Add" button is disabled and the "URL" field is also highlighted red)

@kethinov
Copy link
Contributor

Ah my apologies, you're right. When I read the issue originally, I thought you meant that some time in the past both fields weren't being highlighted and that they should be, but I misread you.

@jasonblais
Copy link
Contributor Author

No problem! Are you interested looking into fixing it?

@yuya-oc
Copy link
Contributor

yuya-oc commented May 16, 2018

Accidentally closed by pushing. The original problem still exists.

@yuya-oc yuya-oc reopened this May 16, 2018
@kethinov
Copy link
Contributor

The reason that #779 was marked as closing this is I argued in #779 that it is a better UX to continue to highlight them all as it currently does, but to clarify the error messages to remove the confusion instead of selectively highlighting.

As I understand it, there is a school of thought in the UX community that once you submit a form, all fields that are not filled out correctly should be highlighted, even the ones you haven't tried to fill out yet. The idea is it draws your attention to everything that needs to be addressed before the form can be submitted.

I'm sure there are good arguments to support both positions, but the react-bootstrap people appear to have taken the position that highlighting them all is the way to go, and have built their API accordingly. To selectively highlight instead, you would probably have to drop that dependency.

@jasonblais
Copy link
Contributor Author

Thanks @yuya-oc and @kethinov - we can close this for now per the reply above, especially now that the text matches the highlighted fields that are missing.

@yuya-oc yuya-oc added this to the v4.1.0 milestone May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants