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

Decouple the Clipboard API from fyne.Window #5121

Merged
merged 51 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
83427a8
Add fyne.App.Clipboard method, fixes #4418
Sep 8, 2024
2999a08
add missing Clipboard for various platforms; update tests
Sep 10, 2024
dd612d7
test: return proper Clipboard
Sep 11, 2024
3482cae
Clipboard: remove singleton
Sep 12, 2024
992f113
app: add missing NewClipboard()
Sep 12, 2024
616fff2
app: instantiate a window to initialize glfw for clipboard tests
Sep 12, 2024
d6a142f
app: Clipboard: use test.NewTempApp and NewTempWindow
Sep 13, 2024
395444b
app: remove duplicat import decl
Sep 14, 2024
a26d502
Clipboard: do not return pointer for a clipboard instance
Sep 14, 2024
6cdb472
internal/driver/mobile: use NewClipboard() instead of returning a new…
pierrec Sep 25, 2024
da68bb0
Fix an indexing error on Darwin introduced some time ago
andydotxyz Sep 12, 2024
f8f8717
Remember last location and go back to it when file dialog is opened a…
andydotxyz Sep 12, 2024
5136aa3
Add test for remembering start dir
andydotxyz Sep 12, 2024
f83fd29
Fix order when code requests a starting directory
andydotxyz Sep 12, 2024
06ced73
Add missing doc for the last two additions
andydotxyz Sep 12, 2024
c277e9b
Bring calendar in from fyne-x
andydotxyz Sep 6, 2024
916592c
Add basic DateEntry type with a static format for picking or typing a…
andydotxyz Sep 6, 2024
36ff5fd
Add missed icon
andydotxyz Sep 7, 2024
c2b04f7
Fix accidental import loop
andydotxyz Sep 7, 2024
23f0724
Allow Date to be nil
andydotxyz Sep 12, 2024
106cdeb
Fix warnings
andydotxyz Sep 12, 2024
7064f5c
Localise weekday names
andydotxyz Sep 12, 2024
18fe7b3
Don't assume disablable
andydotxyz Sep 12, 2024
bfc8ecc
Only create the minSize label once
andydotxyz Sep 12, 2024
1d90c9f
Remove a few excess whitelines
andydotxyz Sep 13, 2024
144528e
widget.Entry: validate when pasting from clipboard
Sep 14, 2024
fb10b2b
Fix issue where about replace code could cause items to remove if ref…
andydotxyz Sep 15, 2024
6be64f2
cache: use one lock/unclock cycle to remove expired items.
Sep 14, 2024
72e7179
test: Add RenderObjectToMarkup and RenderToMarkup functions
flimzy Sep 9, 2024
32a9423
Work around Windows line endings in tests
flimzy Sep 13, 2024
4802592
Fix possible crash with badly formatted json translation
andydotxyz Sep 22, 2024
dfdf8f7
Add installDir (-o) option to get
afh Sep 20, 2024
8d2017e
Add data binding to Select widget
jimorc Sep 8, 2024
54e11f6
Update TeastNewSelectWithData
jimorc Sep 8, 2024
7eb3402
Add Since line to public APIs
jimorc Sep 8, 2024
fff9c45
Change Since to 2.6 from 2.0
jimorc Sep 9, 2024
453d3cb
Change Since 2.6 to Since: 2.6
jimorc Sep 23, 2024
e30975a
Added translation using Weblate (Swedish)
prifre Sep 23, 2024
7738644
Translated using Weblate (Swedish)
prifre Sep 23, 2024
7b6f833
internal/driver/mobile: return storage.ErrNotExists instead of custom…
Sep 17, 2024
219e798
Update root package GoDoc to include internal links
flimzy Sep 9, 2024
b764796
Improve doc strings for file dialogs
ErikKalkoken Sep 28, 2024
54a2d88
Further improve comments for file dialogs
ErikKalkoken Sep 30, 2024
70984a8
Update to the latest breaking API changes in go-text
andydotxyz Sep 28, 2024
ddf4252
Modernise CI infrastructure with Go 1.22
Jacalz Sep 30, 2024
7ba5aa5
Update CI and code to use Staticcheck v0.5.1
Jacalz Sep 30, 2024
4bfe10d
Add a missing import
Jacalz Sep 30, 2024
5e52896
Bump Go version in CI to 1.23
Jacalz Sep 30, 2024
c2cdd54
Fix staticcheck error for deprecation
Jacalz Oct 1, 2024
70576c8
Merge remote-tracking branch 'upstream/develop' into issue4418
Jacalz Oct 1, 2024
50b23bb
cmd/fyne_demo: set the clipboard for Copy/Cut/Paste shortcuts
pierrec Oct 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ type App interface {
//
// Since: 2.3
SetCloudProvider(CloudProvider) // configure cloud for this app

// Clipboard returns the system clipboard.
//
// Since: 2.6
Clipboard() Clipboard
}

var app atomic.Pointer[App]
Expand Down
15 changes: 10 additions & 5 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import (
var _ fyne.App = (*fyneApp)(nil)

type fyneApp struct {
driver fyne.Driver
icon fyne.Resource
uniqueID string
driver fyne.Driver
clipboard fyne.Clipboard
icon fyne.Resource
uniqueID string

cloud fyne.CloudProvider
lifecycle app.Lifecycle
Expand Down Expand Up @@ -109,6 +110,10 @@ func (a *fyneApp) newDefaultPreferences() *preferences {
return p
}

func (a *fyneApp) Clipboard() fyne.Clipboard {
return a.clipboard
}

// New returns a new application instance with the default driver and no unique ID (unless specified in FyneApp.toml)
func New() fyne.App {
if meta.ID == "" {
Expand Down Expand Up @@ -137,8 +142,8 @@ func makeStoreDocs(id string, s *store) *internal.Docs {
}
}

func newAppWithDriver(d fyne.Driver, id string) fyne.App {
newApp := &fyneApp{uniqueID: id, driver: d}
func newAppWithDriver(d fyne.Driver, clipboard fyne.Clipboard, id string) fyne.App {
newApp := &fyneApp{uniqueID: id, clipboard: clipboard, driver: d}
fyne.SetCurrentApp(newApp)

newApp.prefs = newApp.newDefaultPreferences()
Expand Down
2 changes: 1 addition & 1 deletion app/app_gl.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ import (
// NewWithID returns a new app instance using the appropriate runtime driver.
// The ID string should be globally unique to this app.
func NewWithID(id string) fyne.App {
return newAppWithDriver(glfw.NewGLDriver(), id)
return newAppWithDriver(glfw.NewGLDriver(), glfw.NewClipboard(), id)
}
2 changes: 1 addition & 1 deletion app/app_mobile.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
// The ID string should be globally unique to this app.
func NewWithID(id string) fyne.App {
d := mobile.NewGoMobileDriver()
a := newAppWithDriver(d, id)
a := newAppWithDriver(d, mobile.NewClipboard(), id)
d.(mobile.ConfiguredDriver).SetOnConfigurationChanged(func(c *mobile.Configuration) {
internalapp.SystemTheme = c.SystemTheme

Expand Down
2 changes: 1 addition & 1 deletion app/app_software.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ import (
// NewWithID returns a new app instance using the test (headless) driver.
// The ID string should be globally unique to this app.
func NewWithID(id string) fyne.App {
return newAppWithDriver(test.NewDriverWithPainter(software.NewPainter()), id)
return newAppWithDriver(test.NewDriverWithPainter(software.NewPainter()), test.NewClipboard(), id)
}
25 changes: 24 additions & 1 deletion app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"github.com/stretchr/testify/assert"

"fyne.io/fyne/v2"
_ "fyne.io/fyne/v2/test"
"fyne.io/fyne/v2/test"
)

func TestDummyApp(t *testing.T) {
Expand Down Expand Up @@ -48,3 +48,26 @@ func TestFyneApp_SetIcon(t *testing.T) {

assert.Equal(t, setIcon, app.Icon())
}

func TestFynaApp_Clipboard(t *testing.T) {
app := test.NewTempApp(t)
test.NewTempWindow(t, nil)

text := "My content from test window"
cb := app.Clipboard()

cliboardContent := cb.Content()
if cliboardContent != "" {
// Current environment has some content stored in clipboard,
// set temporary to an empty string to allow test and restore later.
cb.SetContent("")
}

assert.Empty(t, cb.Content())

cb.SetContent(text)
assert.Equal(t, text, cb.Content())

// Restore clipboardContent, if any
cb.SetContent(cliboardContent)
}
4 changes: 4 additions & 0 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ func (dummyApp) Metadata() AppMetadata {
return AppMetadata{}
}

func (dummyApp) Clipboard() Clipboard {
return nil
}

func TestSetCurrentApp(t *testing.T) {
a := &dummyApp{}
SetCurrentApp(a)
Expand Down
22 changes: 11 additions & 11 deletions cmd/fyne_demo/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,19 @@ func makeMenu(a fyne.App, w fyne.Window) *fyne.MainMenu {
openSettings()
})

cutShortcut := &fyne.ShortcutCut{Clipboard: w.Clipboard()}
cutShortcut := &fyne.ShortcutCut{Clipboard: a.Clipboard()}
cutItem := fyne.NewMenuItem("Cut", func() {
shortcutFocused(cutShortcut, w)
shortcutFocused(cutShortcut, a.Clipboard(), w.Canvas().Focused())
})
cutItem.Shortcut = cutShortcut
copyShortcut := &fyne.ShortcutCopy{Clipboard: w.Clipboard()}
copyShortcut := &fyne.ShortcutCopy{Clipboard: a.Clipboard()}
copyItem := fyne.NewMenuItem("Copy", func() {
shortcutFocused(copyShortcut, w)
shortcutFocused(copyShortcut, a.Clipboard(), w.Canvas().Focused())
})
copyItem.Shortcut = copyShortcut
pasteShortcut := &fyne.ShortcutPaste{Clipboard: w.Clipboard()}
pasteShortcut := &fyne.ShortcutPaste{Clipboard: a.Clipboard()}
pasteItem := fyne.NewMenuItem("Paste", func() {
shortcutFocused(pasteShortcut, w)
shortcutFocused(pasteShortcut, a.Clipboard(), w.Canvas().Focused())
})
pasteItem.Shortcut = pasteShortcut
performFind := func() { fmt.Println("Menu Find") }
Expand Down Expand Up @@ -251,16 +251,16 @@ func makeNav(setTutorial func(tutorial tutorials.Tutorial), loadPrevious bool) f
return container.NewBorder(nil, themes, nil, nil, tree)
}

func shortcutFocused(s fyne.Shortcut, w fyne.Window) {
func shortcutFocused(s fyne.Shortcut, cb fyne.Clipboard, f fyne.Focusable) {
switch sh := s.(type) {
case *fyne.ShortcutCopy:
sh.Clipboard = w.Clipboard()
Copy link
Member

Choose a reason for hiding this comment

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

Removing this code looks like it is possible that the shortcut for copy.paste etc could some times be missing the clipboard info which as previously there..?

Copy link
Member

Choose a reason for hiding this comment

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

From what I can tell from the code, it was always setting the clipboard twice. First when creating the struct and then this function made a type switch and set it again?

Copy link
Author

Choose a reason for hiding this comment

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

@andydotxyz the shortcut is already setup with the clipboard at instanciation. No need to do it a second time right?

Copy link
Member

Choose a reason for hiding this comment

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

the shortcut is already setup with the clipboard at instanciation.

In the instantiation you make, yes - but check other usages too - like in Entry...

e.shortcut.AddShortcut(&fyne.ShortcutCopy{}, ...)

Copy link
Author

Choose a reason for hiding this comment

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

I see. It may be better to get ShortcutCopy/Cut/Paste from the App though instead of relying on the user to properly instantiate it. In the meantime, I will restore the check in the fyne_demo app.

Copy link
Member

Choose a reason for hiding this comment

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

It may be better to get ShortcutCopy/Cut/Paste from the App though instead of relying on the user to properly instantiate it

I agree with this statement, however having the fields be nil would be a breaking change, so thanks for re-instating it.

sh.Clipboard = cb
case *fyne.ShortcutCut:
sh.Clipboard = w.Clipboard()
sh.Clipboard = cb
case *fyne.ShortcutPaste:
sh.Clipboard = w.Clipboard()
sh.Clipboard = cb
}
if focused, ok := w.Canvas().Focused().(fyne.Shortcutable); ok {
if focused, ok := f.(fyne.Shortcutable); ok {
focused.TypedShortcut(s)
}
}
14 changes: 9 additions & 5 deletions internal/driver/glfw/clipboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@ import (
)

// Declare conformity with Clipboard interface
var _ fyne.Clipboard = (*clipboard)(nil)
var _ fyne.Clipboard = clipboard{}

func NewClipboard() fyne.Clipboard {
return clipboard{}
}

// clipboard represents the system clipboard
type clipboard struct{}

// Content returns the clipboard content
func (c *clipboard) Content() string {
func (c clipboard) Content() string {
// This retry logic is to work around the "Access Denied" error often thrown in windows PR#1679
if runtime.GOOS != "windows" {
return c.content()
Expand All @@ -34,7 +38,7 @@ func (c *clipboard) Content() string {
return ""
}

func (c *clipboard) content() string {
func (c clipboard) content() string {
content := ""
runOnMain(func() {
content = glfw.GetClipboardString()
Expand All @@ -43,7 +47,7 @@ func (c *clipboard) content() string {
}

// SetContent sets the clipboard content
func (c *clipboard) SetContent(content string) {
func (c clipboard) SetContent(content string) {
// This retry logic is to work around the "Access Denied" error often thrown in windows PR#1679
if runtime.GOOS != "windows" {
c.setContent(content)
Expand All @@ -59,7 +63,7 @@ func (c *clipboard) SetContent(content string) {
fyne.LogError("GLFW clipboard set failed", nil)
}

func (c *clipboard) setContent(content string) {
func (c clipboard) setContent(content string) {
runOnMain(func() {
glfw.SetClipboardString(content)
})
Expand Down
10 changes: 7 additions & 3 deletions internal/driver/glfw/clipboard_goxjs.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,19 @@ import (
)

// Declare conformity with Clipboard interface
var _ fyne.Clipboard = (*clipboard)(nil)
var _ fyne.Clipboard = clipboard{}

func NewClipboard() fyne.Clipboard {
return clipboard{}
}

// clipboard represents the system clipboard
type clipboard struct {
window *glfw.Window
}

// Content returns the clipboard content
func (c *clipboard) Content() string {
func (c clipboard) Content() string {
content := ""
runOnMain(func() {
content, _ = c.window.GetClipboardString()
Expand All @@ -25,7 +29,7 @@ func (c *clipboard) Content() string {
}

// SetContent sets the clipboard content
func (c *clipboard) SetContent(content string) {
func (c clipboard) SetContent(content string) {
runOnMain(func() {
c.window.SetClipboardString(content)
})
Expand Down
12 changes: 6 additions & 6 deletions internal/driver/glfw/window.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (w *window) ShowAndRun() {

// Clipboard returns the system clipboard
func (w *window) Clipboard() fyne.Clipboard {
return &clipboard{}
dweymouth marked this conversation as resolved.
Show resolved Hide resolved
return NewClipboard()
}

func (w *window) Content() fyne.CanvasObject {
Expand Down Expand Up @@ -858,17 +858,17 @@ func (w *window) triggersShortcut(localizedKeyName fyne.KeyName, key fyne.KeyNam
case fyne.KeyV:
// detect paste shortcut
shortcut = &fyne.ShortcutPaste{
Clipboard: w.Clipboard(),
Clipboard: NewClipboard(),
}
case fyne.KeyC, fyne.KeyInsert:
// detect copy shortcut
shortcut = &fyne.ShortcutCopy{
Clipboard: w.Clipboard(),
Clipboard: NewClipboard(),
}
case fyne.KeyX:
// detect cut shortcut
shortcut = &fyne.ShortcutCut{
Clipboard: w.Clipboard(),
Clipboard: NewClipboard(),
}
case fyne.KeyA:
// detect selectAll shortcut
Expand All @@ -881,12 +881,12 @@ func (w *window) triggersShortcut(localizedKeyName fyne.KeyName, key fyne.KeyNam
case fyne.KeyInsert:
// detect paste shortcut
shortcut = &fyne.ShortcutPaste{
Clipboard: w.Clipboard(),
Clipboard: NewClipboard(),
}
case fyne.KeyDelete:
// detect cut shortcut
shortcut = &fyne.ShortcutCut{
Clipboard: w.Clipboard(),
Clipboard: NewClipboard(),
}
}
}
Expand Down
28 changes: 3 additions & 25 deletions internal/driver/glfw/window_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1621,28 +1621,6 @@ func TestWindow_ManualFocus(t *testing.T) {
assert.Equal(t, 1, content.unfocusedTimes)
}

func TestWindow_Clipboard(t *testing.T) {
w := createWindow("Test")

text := "My content from test window"
cb := w.Clipboard()

cliboardContent := cb.Content()
if cliboardContent != "" {
// Current environment has some content stored in clipboard,
// set temporary to an empty string to allow test and restore later.
cb.SetContent("")
}

assert.Empty(t, cb.Content())

cb.SetContent(text)
assert.Equal(t, text, cb.Content())

// Restore clipboardContent, if any
cb.SetContent(cliboardContent)
}

func TestWindow_ClipboardCopy_DisabledEntry(t *testing.T) {
w := createWindow("Test").(*window)
e := widget.NewEntry()
Expand All @@ -1662,7 +1640,7 @@ func TestWindow_ClipboardCopy_DisabledEntry(t *testing.T) {
w.keyPressed(nil, glfw.KeyC, 0, glfw.Repeat, ctrlMod)
w.WaitForEvents()

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

e.SetText("Testing2")
e.DoubleTapped(nil)
Expand All @@ -1673,14 +1651,14 @@ func TestWindow_ClipboardCopy_DisabledEntry(t *testing.T) {
w.WaitForEvents()

assert.Equal(t, "Testing2", e.Text)
assert.Equal(t, "Testing", w.Clipboard().Content())
assert.Equal(t, "Testing", NewClipboard().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())
assert.Equal(t, "Testing", NewClipboard().Content())
}

func TestWindow_CloseInterception(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion internal/driver/mobile/clipboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import (
)

// Declare conformity with Clipboard interface
var _ fyne.Clipboard = (*mobileClipboard)(nil)
var _ fyne.Clipboard = mobileClipboard{}

func NewClipboard() fyne.Clipboard {
return mobileClipboard{}
}

// mobileClipboard represents the system mobileClipboard
type mobileClipboard struct {
Expand Down
4 changes: 2 additions & 2 deletions internal/driver/mobile/clipboard_android.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

// Content returns the clipboard content for Android
func (c *mobileClipboard) Content() string {
func (c mobileClipboard) Content() string {
content := ""
app.RunOnJVM(func(vm, env, ctx uintptr) error {
chars := C.getClipboardContent(C.uintptr_t(vm), C.uintptr_t(env), C.uintptr_t(ctx))
Expand All @@ -34,7 +34,7 @@ func (c *mobileClipboard) Content() string {
}

// SetContent sets the clipboard content for Android
func (c *mobileClipboard) SetContent(content string) {
func (c mobileClipboard) SetContent(content string) {
contentStr := C.CString(content)
defer C.free(unsafe.Pointer(contentStr))

Expand Down
4 changes: 2 additions & 2 deletions internal/driver/mobile/clipboard_desktop.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ package mobile
import "fyne.io/fyne/v2"

// Content returns the clipboard content for mobile simulator runs
func (c *mobileClipboard) Content() string {
func (c mobileClipboard) Content() string {
fyne.LogError("Clipboard is not supported in mobile simulation", nil)
return ""
}

// SetContent sets the clipboard content for mobile simulator runs
func (c *mobileClipboard) SetContent(content string) {
func (c mobileClipboard) SetContent(content string) {
fyne.LogError("Clipboard is not supported in mobile simulation", nil)
}
Loading
Loading