Skip to content

Commit

Permalink
Merge pull request #2068 from fpabl0/fix/disabled-entry-copy
Browse files Browse the repository at this point in the history
Fixes on widget.Entry
  • Loading branch information
fpabl0 committed Mar 12, 2021
2 parents c18fd15 + b29a295 commit 843d285
Show file tree
Hide file tree
Showing 12 changed files with 190 additions and 17 deletions.
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 @@ -1124,10 +1124,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 @@ -1175,7 +1188,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 @@ -1215,7 +1228,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 @@ -1239,7 +1252,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 @@ -1293,7 +1306,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 @@ -1359,7 +1372,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 @@ -1374,7 +1387,7 @@ func (r *entryContentRenderer) Refresh() {
placeholder.Hide()
}

if focused {
if focusedAppearance {
r.cursor.Show()
r.content.entry.cursorAnim.start()
} else {
Expand All @@ -1384,7 +1397,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 @@ -1408,7 +1421,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 @@ -319,6 +360,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.

0 comments on commit 843d285

Please sign in to comment.