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

Fixed an issue in Group validation: Convert error dialogs to checkers #6418

Closed

Conversation

catarinagomespt
Copy link
Contributor

@catarinagomespt catarinagomespt commented May 4, 2020

Fixes #6221

In the Add subgroup Dialog the name field is now mandatory. I also disabled the dialog validation with the error message, because the dialog was duplicate and I tried to kept consistency in the dialogs app (like the SharedDatabaseLogin).
Now the Group Dialog View only activates OK button when the user writes something.

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

@koppor
Copy link
Member

koppor commented May 5, 2020

Tested it. Works:

Opening dialog:

grafik

Why is there a red marker at the name if something is entered?

grafik

If the field is cleared, the error marker appears:

grafik

Questions:

  1. Can this behavior also be added to shared database dialog? If the dialog opens, there are so many red markers:

    grafik

  2. Bonus level: Wror error dialog. Can this be fixed?

    grafik

    I get this dialog when deleting the group name in the BibTeX:

     1 SearchGroup:\;0\;ZEUS\;0\;0\;1\;\;\;\;;
    

@catarinagomespt
Copy link
Contributor Author

catarinagomespt commented May 5, 2020

Question: Why is there a red marker at the name if something is entered?
Answer: There are two validations to subgroup Name:
First, the subgroup is required;
Second, the subgroup name is unique. Maybe you have already created a subgroup named "Test"?

GroupValidation

@catarinagomespt
Copy link
Contributor Author

catarinagomespt commented May 6, 2020

@catarinagomespt
Copy link
Contributor Author

Like I show in the video (please, see the comment mentioned above), when we click the button to "Add subgroup" the group dialog opens with the red and error marker. That should be the behaviour, right?

@koppor
Copy link
Member

koppor commented May 6, 2020

Would it be possible to share the video using Loom? (I used to use ScreenToGif, but I was pointed to Loom)

@koppor
Copy link
Member

koppor commented May 6, 2020

Like I show in the video (please, see the comment mentioned above), when we click the button to "Add subgroup" the group dialog opens with the red and error marker. That should be the behaviour, right?

I question that behavior. A dialog should show required fields, but not show an error when they are not filled when opened.

See https://medium.com/@andrew.burton/form-validation-best-practices-8e3bec7d0549 for details. Didn't find something better on this, but maybe we can interpret it together and come to the same conclusion:

Firstly, what is form validation

Instant validation which occurs as the user is typing or selecting form elements

Same page reload validation which is used once a form has been submitted, before reloading the page with errors displayed.

There are no other cases; especially not when loading the form the first time.

WDYT?

@Siedlerchr
Copy link
Member

@koppor When you have not entered anything the fields show red (for example in the shared database) because it's a required field.

@koppor
Copy link
Member

koppor commented May 6, 2020 via email

@catarinagomespt
Copy link
Contributor Author

Would it be possible to share the video using Loom? (I used to use ScreenToGif, but I was pointed to Loom)

Add a subgroup video:
https://www.loom.com/share/9a34f7341ee547aea541acfb6b894f2b

@koppor
Copy link
Member

koppor commented May 6, 2020

Thank you for your patience.

I meant following:

grafik

I think, following text summarizes my thinking:

In most cases, avoid showing the error until the user has finished with the field and moved to the next field. It can be annoying to see an error message before being given the opportunity to finish typing.

grafik

Source: https://www.nngroup.com/articles/errors-forms-design-guidelines/

@catarinagomespt
Copy link
Contributor Author

catarinagomespt commented May 8, 2020

@koppor
I'm confused. What should we do?
The SharedDatabaseLogin dialog also does not respect these rules.

Summarize:

  • OK button should be tied to validation (as it is now)

  • This new dialog opens without validation errors.

  • A field should be validated if the user changed something or it is not empty.

My doubts:

  • Mandatory and optional fields should be distinguished somehow, but not by an error marker.
    How? Suggestions?

@tobiasdiez
Copy link
Member

After reading a bit on best practices, I propose the following:

  • Only validate input after leaving the field (or after the user stopped typing for some time)
  • The user shouldn't be able to submit the form if there are errors
  • However, disabling the submit button in case there are errors is also not optimal. Instead, clicking the submit button should highlight the errors.
  • Empty required files shouldn't be marked as invalid until the user a) tries to submit the form or b) focused the field, deleted it contents and then left the field (see https://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_input_required)
  • Ideally, the error message should be shown below the text field and not as a tooltip (so that users quickly understand what's the problem). For example as here https://mdbootstrap.com/docs/jquery/forms/validation/?#custom-styles

What do you think?

@tobiasdiez tobiasdiez added the status: changes required Pull requests that are not yet complete label Jun 18, 2020
@calixtus
Copy link
Member

We should probably add a section to the developers documentation about validation conventions to keep the UI consistent.

@koppor
Copy link
Member

koppor commented Aug 18, 2020

This seems to require much more work. Since nothing happened the last months and we try to reduce the number of stalled PRs, we are closing this. We are hoping, that there will be some updates soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: changes required Pull requests that are not yet complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Group validation: Convert error dialogs to checkers
5 participants