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.

Sign up for GitHub

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

Jump to bottom

Group validation: Convert error dialogs to checkers #6221

Closed
koppor opened this issue Mar 31, 2020 · 5 comments
Closed

Group validation: Convert error dialogs to checkers #6221

koppor opened this issue Mar 31, 2020 · 5 comments
Labels
component: groups good first issue An issue intended for project-newcomers. Varies in difficulty. [outdated] type: enhancement

Comments

@koppor
Copy link
Member

koppor commented Mar 31, 2020 •
edited
Loading

In well-designed dialogs we don't show any error message: instead we disable the ok buton until all required fields are set. An additional popup dialog is unneccsarry.

See for example the SharedDatabaseLogin Dialog which is implemented correctly.

Follow up from #6110
@tobiasdiez tobiasdiez added good first issue An issue intended for project-newcomers. Varies in difficulty. component: groups [outdated] type: enhancement labels Mar 31, 2020
@MootezSaaD
Copy link
Contributor

Hello,
So the requirement is to show an alert instead of disabling the OK (connect button in the case of SharedDatabaseLogin)?
i.e
image

@Siedlerchr
Copy link
Member

Siedlerchr commented Apr 26, 2020 •
edited
Loading

No the other way round. The alert dialog is superfluous, the validation icons show already the required fields with a hover message.
So for the groups dialog this should be the same. Disabling the ok button until required fields are set and show the validation icon. No alert dialog should be shown

@nsmt09
Copy link
Contributor

nsmt09 commented Apr 28, 2020

Hello,

I was looking for a good first issue to contribute to your project and found this one. I checked the behavior and it seems like it already works like it should.
Is it possible that the issue has already been fixed?

Best regards
Niklas

@catarinagomespt
Copy link
Contributor

Hi,
I would like to collaborate with your project. I already understand the problem.
Is this issue still open, right?

Thanks

@catarinagomespt catarinagomespt mentioned this issue May 4, 2020
Closed
5 tasks
@koppor
Copy link
Member Author

koppor commented May 5, 2020

The challange of this issue is to understand the issue.

I try to clarify:

The shared database dialog is implemented (more or less) correctly. There are field checkers in place. And no pop ups.

When following the link in the description, one finds #6110 (comment)

Disabled button should work, except for when the dialog is opened at first. The checks only come into place when one has entered data in the mandatory fields. We need to trigger the validation after initialization to change this.

I think, this is the only place where the issue appears.

A more enduring bug hunter would try to open all JabRef dialogs and check whether there are any popups. This might be too much for a new comer. Thus, I close this issue. We will reopen new issues in case we encounte error popups.

We will discuss at #6418 whether the group dialog issue was solved.

@koppor koppor closed this as completed May 5, 2020
koppor pushed a commit that referenced this issue Oct 1, 2022
Squashed 'buildres/csl/csl-styles/' changes from 201e022..7bde3e4
7486037 
7bde3e4 Add style for the Geographical Analysis journal (#6145)
6fa1551 Create taylor-and-francis-chicago-b-author-date.csl (#6232)
eba2e8c Create taylor-and-francis-ama.csl (#6221)
dda9d57 ACS, AMA, Vancouver: Remove hardcoded space after `citation-number` (#6248)
8f5fe92 GitHub Workflows security hardening (#6246)
284bc10 Create angiology.csl (#6122)
eb141cc Update society-of-biblical-literature-fullnote-bibliography.csl (#6157)
dddb459 Rewrite law-citation-manual.csl (#6171) (#6171)
b975c96 Update Cell to numeric-superscript style (#6245)
3a41b08 Create isara-iso-690.csl (#6201)
5a128fe Create Biomembranes.csl (#6200)
da2e0c0 Capitalize-first short titles for legislation ("Statute"). (#6241)
af7f08d Create proceedings-of-the-estonian-academy-of-sciences-author-date.csl (#6209)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 7bde3e4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: groups good first issue An issue intended for project-newcomers. Varies in difficulty. [outdated] type: enhancement
Projects
Features & Enhancements
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixed an issue in Group validation: Convert error dialogs to checkers catarinagomespt/jabref
6 participants
@Siedlerchr @koppor @tobiasdiez @catarinagomespt @MootezSaaD @nsmt09