app,io,widget: [android/macos] fix focus issues #138

inkeliz · 2024-06-08T23:38:02Z

This patch is intended to fix two issues related to focus and
software-keyboard.

The focus is now reseted if it's lost due to external event,
such as clicking on non-Gio window/activity/fragment.

The software-keyboard will now re-open when clicked, except
if it's ReadOnly.

On macOS it will request focus again if the focus is lose
due to another View in the same Window. However, currently,
it's done by using ShowTextInput.

Fixes: https://todo.sr.ht/~eliasnaur/gio/591

This patch is intended to fix two issues related to focus and software-keyboard. The focus is now reseted if it's lost due to external event, such as clicking on non-Gio window/activity/fragment. The software-keyboard will now re-open when clicked, except if it's ReadOnly. On macOS it will request focus again if the focus is lose due to another View in the same Window. However, currently, it's done by using ShowTextInput. Fixes: https://todo.sr.ht/~eliasnaur/gio/591 Signed-off-by: inkeliz <[email protected]>

inkeliz · 2024-06-11T12:59:28Z

app/os_macos.m

 - (void)windowDidBecomeKey:(NSNotification *)notification {
 	NSWindow *window = (NSWindow *)[notification object];
-  GioView *view = (GioView *)window.contentView;
-	gio_onFocus(view.handle, 1);
+	GioView *view = (GioView *)window.contentView;
+	if ([window firstResponder] == view) {
+		gio_onFocus(view.handle, 1);
+	}
 }
 - (void)windowDidResignKey:(NSNotification *)notification {
 	NSWindow *window = (NSWindow *)[notification object];
-  GioView *view = (GioView *)window.contentView;
-	gio_onFocus(view.handle, 0);
+	GioView *view = (GioView *)window.contentView;
+	if ([window firstResponder] == view) {
+		gio_onFocus(view.handle, 0);
+	}


Reason to change: This is relative to the window, not the view. If you have multiple views in the same window, it used to incorrectly report that Gio had the focus.

inkeliz · 2024-06-11T13:01:25Z

widget/editor.go

+			if !e.ReadOnly {
+				gtx.Execute(key.SoftKeyboardCmd{Show: true})
+			}


Doesn't make sense to request SoftKeyboard when it's ReadOnly. Maybe I need to do it on a separated commit?

inkeliz · 2024-06-11T13:03:21Z

app/os_macos.go

+func (w *window) ShowTextInput(show bool) {
+	if show && !w.config.Focused {
+		C.setFocus(w.view)
+	}
+}


This is macOS, which doesn't have SoftwareKeyboard. However, I didn't find any specific function to request focus, and adding a new function will require changes on multiple OSes, even as no-op.

I'm not sure why you need this here and not on other desktop platforms. Why can't C.setFocus be called automatically in the cases where focus was lost?

It's possible to call it when the GioView is clicked, and doesn't have focus. I don't think Gio should immediate request for Focus, because it's possible to have multiple non-Gio View in the same Window.

Extracted from #138 by inkeliz. Signed-off-by: Elias Naur <[email protected]>

Extracted from #138 by Inkeliz. References: https://todo.sr.ht/~eliasnaur/gio/591 Signed-off-by: Elias Naur <[email protected]>

eliasnaur

I extracted the non-controversial changes, leaving only the two that confuse me. Please rebase.

eliasnaur · 2024-06-16T13:27:35Z

io/input/router.go

+		e := &evts[i]
+		if fe, ok := e.event.(key.FocusEvent); ok {
+			if !fe.Focus {
+				state.keyState.focus = nil


It seems to me this should be done in Router.processEvent, near the lines

case key.EditEvent, key.FocusEvent, key.SelectionEvent: var evts []taggedEvent if f := state.focus; f != nil { evts = append(evts, taggedEvent{tag: f, event: e}) } q.changeState(e, state, evts)

But I'm not perfectly sure what use-case you're fixing. Can you add a test to io/input/key_test.go for my understanding and to guard against regressions?

What I'm fixing is reseting the focus when receiving key.FocusEvent from "the OS".

Consider that you have one FocusCmd{Tag: TextArea}. If Gio gets one FocusEvent from "the OS" (say: macos), it will not remove the focus from TextArea. That is the issue here.

So, this change changes the state (state.keyState.focus) to nil when the FocusEvent is false. Since it's messing with state I use the changeState function.

changeState doesn't change state per se, it adds it to the list of states. A better name would be addStateChange.

See https://github.com/gioui/gio/pull/138/files/101cf1aa478601e9c823f86a86b652b09d768a56#diff-c04c544d7677b4f40b53717995fb8f2e3d67212768e651e229ef2c020fc0d357L434 where state is changed by processEvent, in response to a pointer.Event.

eliasnaur · 2024-06-16T13:30:41Z

app/os_macos.go

+func (w *window) ShowTextInput(show bool) {
+	if show && !w.config.Focused {
+		C.setFocus(w.view)
+	}
+}


I'm not sure why you need this here and not on other desktop platforms. Why can't C.setFocus be called automatically in the cases where focus was lost?

inkeliz force-pushed the fix-focus-0.6 branch from 6433dc3 to 8dde51f Compare June 9, 2024 12:25

inkeliz changed the title ~~io,widget: [android] improve focus~~ app,io,widget: [android/macos] fix focus issues Jun 9, 2024

inkeliz force-pushed the fix-focus-0.6 branch from 8dde51f to db28ac7 Compare June 9, 2024 12:38

inkeliz force-pushed the fix-focus-0.6 branch from db28ac7 to 101cf1a Compare June 9, 2024 12:39

inkeliz commented Jun 11, 2024

View reviewed changes

whereswaldon pushed a commit that referenced this pull request Jun 16, 2024

app: [macOS] ignore focus changes not meant for the Gio view

e878dbc

Extracted from #138 by inkeliz. Signed-off-by: Elias Naur <[email protected]>

whereswaldon pushed a commit that referenced this pull request Jun 16, 2024

widget: show software keyboard when a writable Editor is clicked

38fca9a

Extracted from #138 by Inkeliz. References: https://todo.sr.ht/~eliasnaur/gio/591 Signed-off-by: Elias Naur <[email protected]>

eliasnaur reviewed Jun 16, 2024

View reviewed changes

eliasnaur mentioned this pull request Jun 16, 2024

app: [macos] improve focus #93

Open

whereswaldon force-pushed the main branch 2 times, most recently from f8029f2 to 026d3f9 Compare June 20, 2024 07:54

whereswaldon force-pushed the main branch 13 times, most recently from 3d36537 to 74ccc9c Compare June 27, 2024 14:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

app,io,widget: [android/macos] fix focus issues #138

app,io,widget: [android/macos] fix focus issues #138

inkeliz commented Jun 8, 2024 •

edited

Loading

inkeliz Jun 11, 2024 •

edited

Loading

inkeliz Jun 11, 2024 •

edited

Loading

inkeliz Jun 11, 2024

eliasnaur Jun 16, 2024

inkeliz Jun 18, 2024

eliasnaur left a comment

eliasnaur Jun 16, 2024

inkeliz Jun 18, 2024 •

edited

Loading

eliasnaur Jun 20, 2024

eliasnaur Jun 16, 2024

app,io,widget: [android/macos] fix focus issues #138

Are you sure you want to change the base?

app,io,widget: [android/macos] fix focus issues #138

Conversation

inkeliz commented Jun 8, 2024 • edited Loading

inkeliz Jun 11, 2024 • edited Loading

Choose a reason for hiding this comment

inkeliz Jun 11, 2024 • edited Loading

Choose a reason for hiding this comment

inkeliz Jun 11, 2024

Choose a reason for hiding this comment

eliasnaur Jun 16, 2024

Choose a reason for hiding this comment

inkeliz Jun 18, 2024

Choose a reason for hiding this comment

eliasnaur left a comment

Choose a reason for hiding this comment

eliasnaur Jun 16, 2024

Choose a reason for hiding this comment

inkeliz Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

eliasnaur Jun 20, 2024

Choose a reason for hiding this comment

eliasnaur Jun 16, 2024

Choose a reason for hiding this comment

inkeliz commented Jun 8, 2024 •

edited

Loading

inkeliz Jun 11, 2024 •

edited

Loading

inkeliz Jun 11, 2024 •

edited

Loading

inkeliz Jun 18, 2024 •

edited

Loading