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

CompositeValidator valid percentaje #481

Closed
jpitchardu opened this issue May 9, 2017 · 8 comments
Closed

CompositeValidator valid percentaje #481

jpitchardu opened this issue May 9, 2017 · 8 comments
Milestone

Comments

@jpitchardu
Copy link

Hi, guys

I've come with the need of displaying the percentage of completion of a form and we decided to display the percentage of valid inputs in the form, so I looked on CompositeValidator to see if there was a way to get the validators or at least the number of validators in the composite and there is not.

Although I know this is just a really really really backlog feature, it would be nice to have it.

Thanks guys

@manuel-mauky
Copy link
Collaborator

Hi Jonathan,
we could add a getValidators method that returns an unmodifiable list of the validators of the CompositeValidator. Would this solve your use case?

@jpitchardu
Copy link
Author

Yes it would, it'd a lot easier than saving an List

@manuel-mauky
Copy link
Collaborator

Short update: @gbalderas is working on this.
It's a simple change but I also like to have some tests that show the usage of this feature. He has prepared a test that shows your use case with the percentage of successful validators.
A PullRequest will be ready within a short time.

@manuel-mauky manuel-mauky added this to the 1.7.0 milestone May 15, 2017
@jpitchardu
Copy link
Author

Thanks a lot guys

@gbalderas
Copy link
Contributor

Hello, while doing the test, I noticed that if the same error message is used by different Validators, all of them will be removed from the HashMap of messages in the CompositeValidationStatus (line 136).
This happens when more than one Validator are valid.

When this happens my test fails since my Binding cannot be updated because it is bound to the messages ObservableList. Also the list of messages in the CompositeValidator will be reduced to 1, making it impossible to know how many validators are valid based on the message count. But if I do a for loop for each element in the validators list, I can count how many of them are valid.

Is this intentional? Should every validator message be removed if there are used more than once? Or should every Validator have a different/unique message?

If it is intentional, maybe the CompositeValidator can give a number or an observable value of how many validators are valid?

@manuel-mauky
Copy link
Collaborator

Hi @gbalderas,
thanks for your work. The problem that you describe sounds like you are using the same instance of Error for multiple validators. We introduces the HashMap that you've mentioned as a fix for other problems. But maybe we have overlooked some edge cases and maybe we should document this behaviour better.

However, I don't think this is connected to the actual issue we are trying to solve here. Can you please create a new issue for this particular problem and create the PullRequest for this issue here anyway?

@manuel-mauky
Copy link
Collaborator

@pichardoJ can you please take a look at PR #486. Does this solve your use case?

@jpitchardu
Copy link
Author

jpitchardu commented May 20, 2017

@lestard yes it does, thank you and @gbalderas for the quick response

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

3 participants