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

Fixes on widget.Entry #2068

Merged
merged 11 commits into from Mar 12, 2021
7 changes: 6 additions & 1 deletion internal/app/focus_manager.go
Expand Up @@ -52,7 +52,12 @@ func (f *FocusManager) Focus(obj fyne.Focusable) bool {
return true
}
if dis, ok := obj.(fyne.Disableable); ok && dis.Disabled() {
return true
type selectableText interface {
SelectedText() string
}
if _, isSelectableText := obj.(selectableText); !isSelectableText || fyne.CurrentDevice().IsMobile() {
return true
}
}
}
f.focus(obj)
Expand Down
13 changes: 11 additions & 2 deletions internal/driver/glfw/window.go
Expand Up @@ -1123,10 +1123,19 @@ func (w *window) keyPressed(_ *glfw.Window, key glfw.Key, scancode int, action g

if shortcut != nil {
if focused, ok := w.canvas.Focused().(fyne.Shortcutable); ok {
w.queueEvent(func() { focused.TypedShortcut(shortcut) })
shouldRunShortcut := true
type selectableText interface {
fyne.Disableable
SelectedText() string
}
if selectableTextWid, ok := focused.(selectableText); ok && selectableTextWid.Disabled() {
shouldRunShortcut = shortcut.ShortcutName() == "Copy"
}
if shouldRunShortcut {
w.queueEvent(func() { focused.TypedShortcut(shortcut) })
}
return
}

w.queueEvent(func() { w.canvas.shortcut.TypedShortcut(shortcut) })
return
}
Expand Down
40 changes: 40 additions & 0 deletions internal/driver/glfw/window_test.go
Expand Up @@ -992,6 +992,46 @@ func TestWindow_Clipboard(t *testing.T) {
cb.SetContent(cliboardContent)
}

func TestWindow_ClipboardCopy_DisabledEntry(t *testing.T) {
w := createWindow("Test").(*window)
e := widget.NewEntry()
e.SetText("Testing")
e.Disable()
w.SetContent(e)
repaintWindow(w)

w.canvas.Focus(e)
e.DoubleTapped(nil)
assert.Equal(t, "Testing", e.SelectedText())

ctrlMod := glfw.ModControl
if runtime.GOOS == "darwin" {
ctrlMod = glfw.ModSuper
}
w.keyPressed(nil, glfw.KeyC, 0, glfw.Repeat, ctrlMod)
w.waitForEvents()

assert.Equal(t, "Testing", w.Clipboard().Content())

e.SetText("Testing2")
e.DoubleTapped(nil)
assert.Equal(t, "Testing2", e.SelectedText())

// any other shortcut should be forbidden (Cut)
w.keyPressed(nil, glfw.KeyX, 0, glfw.Repeat, ctrlMod)
w.waitForEvents()

assert.Equal(t, "Testing2", e.Text)
assert.Equal(t, "Testing", w.Clipboard().Content())

// any other shortcut should be forbidden (Paste)
w.keyPressed(nil, glfw.KeyV, 0, glfw.Repeat, ctrlMod)
w.waitForEvents()

assert.Equal(t, "Testing2", e.Text)
assert.Equal(t, "Testing", w.Clipboard().Content())
}

func TestWindow_CloseInterception(t *testing.T) {
d := NewGLDriver()
w := d.CreateWindow("test").(*window)
Expand Down
28 changes: 27 additions & 1 deletion internal/driver/gomobile/canvas_test.go
Expand Up @@ -4,6 +4,7 @@ package gomobile

import (
"image/color"
"os"
"testing"
"time"

Expand All @@ -19,6 +20,14 @@ import (
"github.com/stretchr/testify/assert"
)

func TestMain(m *testing.M) {
currentApp := fyne.CurrentApp()
fyne.SetCurrentApp(newTestMobileApp())
ret := m.Run()
fyne.SetCurrentApp(currentApp)
os.Exit(ret)
}

func TestCanvas_ChildMinSizeChangeAffectsAncestorsUpToRoot(t *testing.T) {
c := NewCanvas().(*mobileCanvas)
leftObj1 := canvas.NewRectangle(color.Black)
Expand Down Expand Up @@ -170,12 +179,13 @@ func TestCanvas_Dragged(t *testing.T) {

assert.True(t, dragged)
assert.Equal(t, scroll, draggedObj)
// TODO find a way to get the test driver to report as mobile
dragged = false
c.tapMove(fyne.NewPos(32, 5), 0, func(wid fyne.Draggable, ev *fyne.DragEvent) {
wid.Dragged(ev)
dragged = true
})
assert.True(t, dragged)
assert.Equal(t, fyne.NewPos(0, 5), scroll.Offset)
}

func TestCanvas_Tappable(t *testing.T) {
Expand Down Expand Up @@ -359,3 +369,19 @@ func simulateTap(c *mobileCanvas) {
}, func(wid fyne.Draggable) {
})
}

type mobileApp struct {
fyne.App
driver fyne.Driver
}

func (a *mobileApp) Driver() fyne.Driver {
return a.driver
}

func newTestMobileApp() fyne.App {
return &mobileApp{
App: fyne.CurrentApp(),
driver: NewGoMobileDriver(),
}
}
37 changes: 25 additions & 12 deletions widget/entry.go
Expand Up @@ -237,10 +237,19 @@ func (e *Entry) DoubleTapped(p *fyne.PointEvent) {
})
}

// DragEnd is called at end of a drag event. It does nothing.
// DragEnd is called at end of a drag event.
//
// Implements: fyne.Draggable
func (e *Entry) DragEnd() {
e.propertyLock.Lock()
if e.CursorColumn == e.selectColumn && e.CursorRow == e.selectRow {
e.selecting = false
}
shouldRefresh := !e.selecting
e.propertyLock.Unlock()
if shouldRefresh {
e.Refresh()
}
}

// Dragged is called when the pointer moves while a button is held down.
Expand Down Expand Up @@ -283,9 +292,6 @@ func (e *Entry) ExtendBaseWidget(wid fyne.Widget) {
//
// Implements: fyne.Focusable
func (e *Entry) FocusGained() {
if e.Disabled() {
return
}
e.setFieldsAndRefresh(func() {
e.focused = true
})
Expand All @@ -297,6 +303,7 @@ func (e *Entry) FocusGained() {
func (e *Entry) FocusLost() {
e.setFieldsAndRefresh(func() {
e.focused = false
e.selectKeyDown = false
})
}

Expand Down Expand Up @@ -329,6 +336,9 @@ func (e *Entry) Keyboard() mobile.KeyboardType {
//
// Implements: desktop.Keyable
func (e *Entry) KeyDown(key *fyne.KeyEvent) {
if e.Disabled() {
return
}
// For keyboard cursor controlled selection we now need to store shift key state and selection "start"
// Note: selection start is where the highlight started (if the user moves the selection up or left then
// the selectRow/Column will not match SelectionStart)
Expand All @@ -345,6 +355,9 @@ func (e *Entry) KeyDown(key *fyne.KeyEvent) {
//
// Implements: desktop.Keyable
func (e *Entry) KeyUp(key *fyne.KeyEvent) {
if e.Disabled() {
return
}
// Handle shift release for keyboard selection
// Note: if shift is released then the user may repress it without moving to adjust their old selection
if key.Name == desktop.KeyShiftLeft || key.Name == desktop.KeyShiftRight {
Expand Down Expand Up @@ -1185,7 +1198,7 @@ func (r *entryRenderer) Refresh() {
provider := r.entry.textProvider()
text := r.entry.Text
content := r.entry.content
focused := r.entry.focused
focusedAppearance := r.entry.focused && !r.entry.disabled
size := r.entry.size
wrapping := r.entry.Wrapping
r.entry.propertyLock.RUnlock()
Expand Down Expand Up @@ -1225,7 +1238,7 @@ func (r *entryRenderer) Refresh() {
}

r.box.FillColor = theme.InputBackgroundColor()
if focused {
if focusedAppearance {
r.line.FillColor = theme.PrimaryColor()
} else {
if r.entry.Disabled() {
Expand All @@ -1249,7 +1262,7 @@ func (r *entryRenderer) Refresh() {
}

if r.entry.Validator != nil {
if !r.entry.focused && r.entry.Text != "" && r.entry.validationError != nil {
if !r.entry.focused && !r.entry.Disabled() && r.entry.Text != "" && r.entry.validationError != nil {
r.line.FillColor = theme.ErrorColor()
}
r.ensureValidationSetup()
Expand Down Expand Up @@ -1303,7 +1316,7 @@ func (e *entryContent) CreateRenderer() fyne.WidgetRenderer {
return r
}

// DragEnd is called at end of a drag event. It does nothing.
// DragEnd is called at end of a drag event.
//
// Implements: fyne.Draggable
func (e *entryContent) DragEnd() {
Expand Down Expand Up @@ -1369,7 +1382,7 @@ func (r *entryContentRenderer) Refresh() {
provider := r.content.entry.textProvider()
placeholder := r.content.entry.placeholderProvider()
content := r.content.entry.Text
focused := r.content.entry.focused
focusedAppearance := r.content.entry.focused && !r.content.entry.disabled
selections := r.selection
r.updateScrollDirections()
r.content.entry.propertyLock.RUnlock()
Expand All @@ -1384,7 +1397,7 @@ func (r *entryContentRenderer) Refresh() {
placeholder.Hide()
}

if focused {
if focusedAppearance {
r.cursor.Show()
r.content.entry.cursorAnim.start()
} else {
Expand All @@ -1394,7 +1407,7 @@ func (r *entryContentRenderer) Refresh() {
r.moveCursor()

for _, selection := range selections {
selection.(*canvas.Rectangle).Hidden = !r.content.entry.focused && !r.content.entry.disabled
selection.(*canvas.Rectangle).Hidden = !r.content.entry.focused
selection.(*canvas.Rectangle).FillColor = theme.PrimaryColor()
}

Expand All @@ -1418,7 +1431,7 @@ func (r *entryContentRenderer) buildSelection() {
}
r.content.entry.propertyLock.RUnlock()

if selectRow == -1 {
if selectRow == -1 || (cursorRow == selectRow && cursorCol == selectCol) {
r.selection = r.selection[:0]

return
Expand Down
52 changes: 52 additions & 0 deletions widget/entry_internal_test.go
Expand Up @@ -93,6 +93,47 @@ func TestEntry_DragSelect(t *testing.T) {
assert.Equal(t, "r the laz", entry.SelectedText())
}

func TestEntry_DragSelectEmpty(t *testing.T) {
entry := NewEntry()
entry.SetText("Testing")

ev1 := getClickPosition("T", 0)
ev2 := getClickPosition("Testing", 0)

// Test empty selection - drag from 'e' to 'e' (empty)
de := &fyne.DragEvent{PointEvent: *ev1, Dragged: fyne.NewDelta(1, 0)}
entry.Dragged(de)
de = &fyne.DragEvent{PointEvent: *ev1, Dragged: fyne.NewDelta(1, 0)}
entry.Dragged(de)

entry.propertyLock.RLock()
assert.True(t, entry.selecting)
entry.propertyLock.RUnlock()

entry.DragEnd()
assert.Equal(t, "", entry.SelectedText())
entry.propertyLock.RLock()
assert.False(t, entry.selecting)
entry.propertyLock.RUnlock()

// Test non-empty selection - drag from 'T' to 'g' (empty)
ev1 = getClickPosition("", 0)
de = &fyne.DragEvent{PointEvent: *ev1, Dragged: fyne.NewDelta(1, 0)}
entry.Dragged(de)
de = &fyne.DragEvent{PointEvent: *ev2, Dragged: fyne.NewDelta(1, 0)}
entry.Dragged(de)

entry.propertyLock.RLock()
assert.True(t, entry.selecting)
entry.propertyLock.RUnlock()

entry.DragEnd()
assert.Equal(t, "Testing", entry.SelectedText())
entry.propertyLock.RLock()
assert.True(t, entry.selecting)
entry.propertyLock.RUnlock()
}

func TestEntry_DragSelectWithScroll(t *testing.T) {
entry := NewEntry()
entry.SetText("The quick brown fox jumped over and over the lazy dog.")
Expand Down Expand Up @@ -290,6 +331,17 @@ func TestEntry_TabSelection(t *testing.T) {
test.AssertImageMatches(t, "entry/tab-select.png", w.Canvas().Capture())
}

func TestEntry_ShiftSelection_ResetOnFocusLost(t *testing.T) {
e := NewEntry()
e.SetText("Hello")

e.KeyDown(&fyne.KeyEvent{Name: desktop.KeyShiftLeft})
assert.True(t, e.selectKeyDown)

e.FocusLost()
assert.False(t, e.selectKeyDown)
}

func getClickPosition(str string, row int) *fyne.PointEvent {
x := fyne.MeasureText(str, theme.TextSize(), fyne.TextStyle{}).Width + theme.Padding()

Expand Down
16 changes: 16 additions & 0 deletions widget/entry_test.go
Expand Up @@ -195,6 +195,22 @@ func TestEntry_Disableable(t *testing.T) {
test.AssertRendersToMarkup(t, "entry/disableable_enabled_custom_value.xml", c)
}

func TestEntry_Disabled_TextSelection(t *testing.T) {
entry, window := setupImageTest(t, false)
defer teardownImageTest(window)
entry.SetText("Testing")
entry.Disable()
c := window.Canvas()

assert.True(t, entry.Disabled())
test.DoubleTap(entry)

test.AssertImageMatches(t, "entry/disabled_text_selected.png", c.Capture())

entry.FocusLost()
test.AssertImageMatches(t, "entry/disabled_text_unselected.png", c.Capture())
}

func TestEntry_EmptySelection(t *testing.T) {
entry := widget.NewEntry()
entry.SetText("text")
Expand Down
2 changes: 1 addition & 1 deletion widget/entry_validation.go
Expand Up @@ -95,7 +95,7 @@ func (r *validationStatusRenderer) MinSize() fyne.Size {
func (r *validationStatusRenderer) Refresh() {
r.entry.propertyLock.RLock()
defer r.entry.propertyLock.RUnlock()
if r.entry.Text == "" {
if r.entry.Text == "" || r.entry.disabled {
r.icon.Hide()
return
}
Expand Down
12 changes: 12 additions & 0 deletions widget/entry_validation_test.go
Expand Up @@ -15,6 +15,18 @@ import (

var validator = validation.NewRegexp(`^\d{4}-\d{2}-\d{2}$`, "Input is not a valid date")

func TestEntry_DisabledHideValidation(t *testing.T) {
entry, window := setupImageTest(t, false)
defer teardownImageTest(window)
c := window.Canvas()

entry.Validator = validator
entry.SetText("invalid text")
entry.Disable()

test.AssertImageMatches(t, "entry/validation_disabled.png", c.Capture())
}

func TestEntry_ValidatedEntry(t *testing.T) {
entry, window := setupImageTest(t, false)
defer teardownImageTest(window)
Expand Down
Binary file added widget/testdata/entry/disabled_text_selected.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added widget/testdata/entry/validation_disabled.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.