Open Cookie04DE opened 3 years ago
I am wondering here if there perhaps could be a better solution to this. I'm not sure if exposing even more very specialised validator fields is a good idea. Wouldn't it be better to implement some way of chaining together validators (something like the group validator suggested in #1268)? You could then do something like this for example:
entry1 := widget.NewEntry()
entry2 := widget.NewEntry()
entry1.Validator = validation.NewGroup(newCustomAbove10(entry1, entry2), newCustomNotTwo())
entry2.Validator = validation.NewGroup(newCustomAbove10(entry1, entry2), newCustomAbove(4))
I now have https://github.com/fyne-io/fyne/pull/2567 ready. Feel free to check it out and try to see if you can get the desired functionality using the rough example provided above. I hope it can be used to solve your use case.
I think this is different to what was proposed. An example would be entries-must-be-equal, this is about grouping different elements rather than different validations for the same widget.
But my suggestion above shows how that can be done by creating a validator that takes references to more than one widget, while also solving the other issue about the widgets having separate validators on their own. I think that it is a much cleaner solution than adding a bunch of new functions to either the form or widgets.
Let us suppose instead that you have a validation that confirms that an entry is non-nil if a checkbox is checked.
On which object does this belong? which is checked first? When the checkbox changes you must fire the entry changed etc - this is not the same thing as multiple validations on a single item.
I'm not sure "a bunch of new functions" is really required - a Form.Validator
field would surely suffice?
It just feels like the wrong way around the problem to me. How would you even have the form function run on every event when something changes in the widgets? How can we make that generic regardless of what widget is there? It just feels like state that should be handled inside the widget.
Speaking about checkboxes, I'm currently working on adding a new kind of validated for requiring RadioGroup and CheckGroup to require a certain amount of items (only one for Radio) to be selected before making the form submitable.
Maybe we need to come back to this when there are more types of widgets that support validation? I don't know if what I was proposing is the right solution, but the alternative of attaching group validation to single widgets does not feel like it is a great solution for all cases as well, because it cannot update when widgets other than the one it attaches to are updated...
How about adding a Lock method to the form? It takes a string and prevents the user from submitting the form, displaying the string in red besides the disabled submit button. Then we could add some new form of validator that takes another validator but instead of itself returning the error from the inner validator, locks a form with the error string. This prevents the error from being displayed next to the entry / checkbox / whatever and instead displays it below the form. We would then also need an Unlock method that removes the red string and makes the form submittable again. I am not sure however when (or if) this extended validator should unlock the form again.
Taking that approach would require that the user hook into all change events manually to control the state on change.
Using the existing Validator
mechanism does this automatically - we put more logic into Form so that developers don't have to write this all themselves.
I don't think this would require much more effort on the user side. Here is an example how I would solve my scenario described above with my new proposed solution.
package main
import (
"errors"
"fyne.io/fyne/v2/"
"fyne.io/fyne/v2/app"
"fyne.io/fyne/v2/data/validation"
"fyne.io/fyne/v2/widget"
)
func main() {
a := app.New()
w := a.NewWindow("Form example")
var numberOne int
numberOneEntry := widget.NewEntryWithData(binding.IntToString(binding.NewInt(&numberOne)))
var numberTwo int
numberTwoEntry := widget.NewEntryWithData(binding.IntToString(binding.NewInt(&numberTwo)))
var form *widget.Form
//This validator never returns an error but if the inner validator returns one, it Locks the form with the error message
notAboveTenValidator := validation.NewLocking(form, func(string) error {
if (numberOne + numberTwo) > 10 {
return errors.New("Number one plus number two must not be bigger than 10")
}
return nil
})
notTwoValidator := func(string) error {
if numberOne == 2 {
return errors.New("Number one must not be two")
}
return nil
}
numberOneEntry.Validator = validation.NewGroup(notTwoValidator, notAboveTenValidator)
aboveFourValidator := func(string) error {
if numberTwo <= 4 {
return errors.New("Number two must be bigger than four")
}
return nil
}
numberTwoEntry.Validator = validation.NewGroup(aboveFourValidator, notAboveTenValidator)
form = widget.NewForm(
widget.NewFormItem("Number 1", numberOneEntry),
widget.NewFormItem("Number 2", numberTwoEntry),
)
form.SubmitText = "Submit"
form.OnSubmit = func() {
//Process the two numbers here
}
w.SetContent(form)
w.ShowAndRun()
}
I don't understand how this is illustrating your proposal - I don't see any Lock or Unlock methods which is what I thought you were suggesting.
@Cookie04DE Isn't that almost exactly what I had proposed above?
@andydotxyz The Lock() method is used internally by the validator created by validation.NewLocking(). You can also use it manually but the example shows that you don't have to. I hoped to illustrate with the code I posted that my proposal doesn't put much burden on the user if they choose to use it.
@Jacalz In a way it is. If you look into the code it even uses the NewGroup() method. But the important distinction is that the error is displayed next to the submit button of the form. And it isn't displayed twice if the user edits both entries.
Oh I see this all happens within validation.NewLocking(form
?
I'm not sure that is a clear solution, this almost inverts the control or flow that validation typically holds.
I don't think that the user will understand this is applying the validator to a form when validator application is otherwise done by setting a field on a widget.
Yes that's true. The reason the validator takes the form as an argument and not the other way around is that the validator automatically receives updates when the widget changes, but in its current state the form is oblivious to that change. If we can somehow let the form know when a change happens, the validator could be an attribute of the form. (This also seems more natural to me)
I don't think locking in the validation to only be for form is a good idea. It should preferably be generic over any type of widget (where it makes sense to use it).
Perhaps the NewLocking() method could take a Lockable
type instead of a *widget.Form
. The interface would be defined as follows:
type Lockable interface {
Lock(msg string)
Unlock()
}
*widget.Form
will implement that interface but others can too.
If we choose to use the validator as an attribute on the form that get's evaluated instead, the above interface would be unnecessary since all widgets wanting this kind of validation can expose such an attribute.
This seems like a lot of new API just to avoid Validator Validatable
on widget.Form
- what do the other approaches offer over the simple suggestion?
For one the widgets inside the form would need some way to inform it of some change so the form can run the validator. This is the main point for me.
Secondly if one widget changes the widget has to rerun the validator, even if the widgets the validator is checking for didn't change. This isn't too bad, but could be more dramatic if multiple validators for different widgets and expensive operations are involved.
Is your feature request related to a problem? Please describe:
Sometimes validating each entry of a form on its own isn't enough.
Imagine the following scenario: A form with two entries for numbers and the requirement that those numbers added up can't be above ten.
Is it possible to construct a solution with the existing API?
You can work around the issue by adding the validator mentioned above to all relevant entries. This is especially difficult if those entries already have validators on their own. For example if the first number can't be two and the second one needs to be above four.
Describe the solution you'd like to see:
A form wide validator would solve the problem. It would be of the type
func() error
and be evaluated every time one entry, checkbox etc. is changed.If the function returns an error it should prevent the user from submitting the form (by disabling the submit button) and displaying the error message in red below the form similar to how an error on validating an entry is displayed.