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

Fix Entry with NotEmpty Validator and Form Hints #2245

Merged
merged 3 commits into from May 24, 2021
Merged

Fix Entry with NotEmpty Validator and Form Hints #2245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a128957
fix notEmpty validator behavior in widget.Entry
fpabl0 May 15, 2021
20a6cb6
fix form hint for widget.Entry's
fpabl0 May 15, 2021
51fdd37
replace onFocusGained and onFocusLost by onFocusChanged in widget.Entry
fpabl0 May 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 26 additions & 9 deletions widget/entry.go
View file
Open in desktop
Expand Up @@ -59,12 +59,17 @@ type Entry struct {

cursorAnim *entryCursorAnimation

dirty bool
focused bool
text *textProvider
placeholder *textProvider
content *entryContent
scroll *widget.Scroll

// useful for Form validation (as the error text should only be shown when
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change in behaviour - before now we would show the details of an error if there was any.
We hid the error status if the user had not yet edited the field.

Is this the desired change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now we would show the details of an error if there was any.

Not sure what you mean here, that is already the entry behavior.

We hid the error status if the user had not yet edited the field.

The only behavior change is that now Entry is going to show the error status (if there is any error) after the user has visited the input by the first time. In that way we can ensure that "not empty" validators are shown as soon as the user focused the input and passed it without solving the error.

// the entry is unfocused)
onFocusChanged func(bool)

// selectRow and selectColumn represent the selection start location
// The selection will span from selectRow/Column to CursorRow/Column -- note that the cursor
// position may occur before or after the select start position in the text.
Expand Down Expand Up @@ -142,7 +147,7 @@ func (e *Entry) Bind(data binding.String) {
val, err := data.Get()
if err != nil {
convertErr = err
e.SetValidationError(e.Validate())
e.Validate()
return
}
e.Text = val
Expand All @@ -156,7 +161,7 @@ func (e *Entry) Bind(data binding.String) {

e.OnChanged = func(s string) {
convertErr = data.Set(s)
e.SetValidationError(e.Validate())
e.Validate()
}
}

Expand Down Expand Up @@ -303,8 +308,12 @@ func (e *Entry) ExtendBaseWidget(wid fyne.Widget) {
// Implements: fyne.Focusable
func (e *Entry) FocusGained() {
e.setFieldsAndRefresh(func() {
e.dirty = true
e.focused = true
})
if e.onFocusChanged != nil {
e.onFocusChanged(true)
}
}

// FocusLost is called when the Entry has had focus removed.
Expand All @@ -315,6 +324,9 @@ func (e *Entry) FocusLost() {
e.focused = false
e.selectKeyDown = false
})
if e.onFocusChanged != nil {
e.onFocusChanged(false)
}
}

// Hide hides the entry.
Expand Down Expand Up @@ -1033,6 +1045,10 @@ func (e *Entry) textProvider() *textProvider {
return e.text
}

if e.Text != "" {
e.dirty = true
}

text := newTextProvider(e.Text, e)
text.ExtendBaseWidget(text)
text.inset = fyne.NewSize(0, theme.InputBorderSize())
Expand Down Expand Up @@ -1080,14 +1096,16 @@ func (e *Entry) updateText(text string) {
changed := e.Text != text
e.Text = text

if e.Text != "" {
e.dirty = true
}

if changed {
callback = e.OnChanged
}
})

if validate := e.Validator; validate != nil {
e.SetValidationError(validate(text))
}
e.Validate()

if callback != nil {
callback(text)
Expand Down Expand Up @@ -1287,7 +1305,7 @@ func (r *entryRenderer) Refresh() {
}

if r.entry.Validator != nil {
if !r.entry.focused && !r.entry.Disabled() && r.entry.Text != "" && r.entry.validationError != nil {
if !r.entry.focused && !r.entry.Disabled() && r.entry.dirty && r.entry.validationError != nil {
r.line.FillColor = theme.ErrorColor()
}
r.ensureValidationSetup()
Expand All @@ -1306,9 +1324,8 @@ func (r *entryRenderer) ensureValidationSetup() {
r.objects = append(r.objects, r.entry.validationStatus)
r.Layout(r.entry.size)

if r.entry.Text != "" {
r.entry.Validate()
}
r.entry.Validate()

r.Refresh()
}
}
Expand Down
6 changes: 3 additions & 3 deletions widget/entry_validation.go
View file
Open in desktop
Expand Up @@ -95,15 +95,15 @@ func (r *validationStatusRenderer) MinSize() fyne.Size {
func (r *validationStatusRenderer) Refresh() {
r.entry.propertyLock.RLock()
defer r.entry.propertyLock.RUnlock()
if r.entry.Text == "" || r.entry.disabled {
if r.entry.disabled {
r.icon.Hide()
return
}

if r.entry.validationError == nil {
if r.entry.validationError == nil && r.entry.Text != "" {
r.icon.Resource = theme.ConfirmIcon()
r.icon.Show()
} else if !r.entry.focused {
} else if r.entry.validationError != nil && !r.entry.focused && r.entry.dirty {
r.icon.Resource = theme.NewErrorThemedResource(theme.ErrorIcon())
r.icon.Show()
} else {
Expand Down
24 changes: 24 additions & 0 deletions widget/entry_validation_test.go
View file
Open in desktop
Expand Up @@ -63,6 +63,30 @@ func TestEntry_Validate(t *testing.T) {
assert.Equal(t, entry.Validate(), entry.Validator(entry.Text))
}

func TestEntry_NotEmptyValidator(t *testing.T) {
test.NewApp()
defer test.NewApp()
entry := widget.NewEntry()
entry.Validator = func(s string) error {
if s == "" {
return errors.New("should not be empty")
}
return nil
}
w := test.NewWindow(entry)
defer w.Close()

test.AssertRendersToMarkup(t, "entry/validator_not_empty_initial.xml", w.Canvas())

w.Canvas().Focus(entry)

test.AssertRendersToMarkup(t, "entry/validator_not_empty_focused.xml", w.Canvas())

w.Canvas().Focus(nil)

test.AssertRendersToMarkup(t, "entry/validator_not_empty_unfocused.xml", w.Canvas())
}

func TestEntry_SetValidationError(t *testing.T) {
entry, window := setupImageTest(t, false)
fyne.CurrentApp().Settings().SetTheme(theme.LightTheme())
Expand Down
36 changes: 23 additions & 13 deletions widget/form.go
View file
Open in desktop
Expand Up @@ -190,29 +190,39 @@ func (f *Form) checkValidation(err error) {
}

func (f *Form) setUpValidation(widget fyne.CanvasObject, i int) {
updateValidation := func(err error) {
if err == errFormItemInitialState {
return
}
f.Items[i].validationError = err
f.Items[i].invalid = err != nil
f.checkValidation(err)
f.updateHelperText(f.Items[i])
}
if w, ok := widget.(fyne.Validatable); ok {
f.Items[i].invalid = w.Validate() != nil
if e, ok := w.(*Entry); ok && e.Validator != nil && f.Items[i].invalid {
// set initial state error to guarantee next error (if triggers) is always different
e.SetValidationError(errFormItemInitialState)
}
w.SetOnValidationChanged(func(err error) {
if err == errFormItemInitialState {
return
if e, ok := w.(*Entry); ok {
e.onFocusChanged = func(bool) {
updateValidation(e.validationError)
}
f.Items[i].validationError = err
f.Items[i].invalid = err != nil
f.checkValidation(err)
f.updateHelperText(f.Items[i])
})
if e.Validator != nil && f.Items[i].invalid {
// set initial state error to guarantee next error (if triggers) is always different
e.SetValidationError(errFormItemInitialState)
}
}
w.SetOnValidationChanged(updateValidation)
}
}

func (f *Form) updateHelperText(item *FormItem) {
if item.helperOutput == nil {
return // testing probably, either way not rendered yet
}
if item.validationError == nil {
showHintIfError := false
if e, ok := item.Widget.(*Entry); ok && (!e.dirty || e.focused) {
showHintIfError = true
}
if item.validationError == nil || showHintIfError {
item.helperOutput.Text = item.HintText
item.helperOutput.Color = theme.PlaceHolderColor()
} else {
Expand Down
3 changes: 0 additions & 3 deletions widget/form_test.go
View file
Open in desktop
Expand Up @@ -213,9 +213,6 @@ func TestForm_EntryValidation_FirstTypeValid(t *testing.T) {
w := test.NewWindow(form)
defer w.Close()

assert.Equal(t, errFormItemInitialState, entry1.validationError)
assert.Equal(t, errFormItemInitialState, entry2.validationError)

test.AssertImageMatches(t, "form/validation_entry_first_type_initial.png", w.Canvas().Capture())

test.Type(entry1, "H")
Expand Down
21 changes: 21 additions & 0 deletions widget/testdata/entry/validator_not_empty_focused.xml
View file
Open in desktop
@@ -0,0 +1,21 @@
<canvas padded size="70x45">
<content>
<widget pos="4,4" size="62x37" type="*widget.Entry">
<rectangle fillColor="rgba(102,102,102,255)" pos="0,2" size="62x33"/>
<rectangle fillColor="primary" pos="0,35" size="62x2"/>
<widget pos="0,2" size="34x33" type="*widget.Scroll">
<widget size="34x33" type="*widget.entryContent">
<widget size="34x33" type="*widget.textProvider">
<text color="placeholder" pos="8,6" size="22x21"></text>
</widget>
<widget size="34x33" type="*widget.textProvider">
<text pos="8,6" size="22x21"></text>
</widget>
<rectangle fillColor="primary" pos="7,6" size="2x21"/>
</widget>
</widget>
<widget pos="34,8" size="20x20" type="*widget.validationStatus">
</widget>
</widget>
</content>
</canvas>
20 changes: 20 additions & 0 deletions widget/testdata/entry/validator_not_empty_initial.xml
View file
Open in desktop
@@ -0,0 +1,20 @@
<canvas padded size="70x45">
<content>
<widget pos="4,4" size="62x37" type="*widget.Entry">
<rectangle fillColor="rgba(102,102,102,255)" pos="0,2" size="62x33"/>
<rectangle fillColor="shadow" pos="0,35" size="62x2"/>
<widget pos="0,2" size="34x33" type="*widget.Scroll">
<widget size="34x33" type="*widget.entryContent">
<widget size="34x33" type="*widget.textProvider">
<text color="placeholder" pos="8,6" size="22x21"></text>
</widget>
<widget size="34x33" type="*widget.textProvider">
<text pos="8,6" size="22x21"></text>
</widget>
</widget>
</widget>
<widget pos="34,8" size="20x20" type="*widget.validationStatus">
</widget>
</widget>
</content>
</canvas>
21 changes: 21 additions & 0 deletions widget/testdata/entry/validator_not_empty_unfocused.xml
View file
Open in desktop
@@ -0,0 +1,21 @@
<canvas padded size="70x45">
<content>
<widget pos="4,4" size="62x37" type="*widget.Entry">
<rectangle fillColor="rgba(102,102,102,255)" pos="0,2" size="62x33"/>
<rectangle fillColor="error" pos="0,35" size="62x2"/>
<widget pos="0,2" size="34x33" type="*widget.Scroll">
<widget size="34x33" type="*widget.entryContent">
<widget size="34x33" type="*widget.textProvider">
<text color="placeholder" pos="8,6" size="22x21"></text>
</widget>
<widget size="34x33" type="*widget.textProvider">
<text pos="8,6" size="22x21"></text>
</widget>
</widget>
</widget>
<widget pos="34,8" size="20x20" type="*widget.validationStatus">
<image rsc="errorIcon" size="iconInlineSize" themed="error"/>
</widget>
</widget>
</content>
</canvas>
Binary file modified widget/testdata/form/hint_invalid.png
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified widget/testdata/form/validation_entry_first_type_invalid.png
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.