45e0e8a fix(accent-picker): address Copilot review feedback · dumber

M README.md +1 -1


@@ -125,7 +125,7 @@ Some features are very alpha/experimental (notably consume-or-expel panes) and m
125	125		\| Space \| Toggle Favorite \| Omnibox mode: favorite/unfavorite selected item \|
126	126		\| Ctrl+1-9, 0 \| Quick Navigate \| Jump to result by number (1-10) \|
127	127		\| ↑/↓ Arrow \| Navigate Results \| Browse suggestions/matches \|
128		-	\| Hold key 400ms \| Accent Picker \| Shows accented variants (e.g., hold 'e' for è, é, ê, ë) \|
	128	+	\| Hold key 400ms \| Accent Picker \| Shows accented variants in any text input (e.g., hold 'e' for è, é, ê, ë) \|
129	129
130	130		All zoom changes are automatically persisted per-domain and restored on next visit.
131	131


@@ -125,7 +125,7 @@ Some features are very alpha/experimental (notably consume-or-expel panes) and m
125		\| Space \| Toggle Favorite \| Omnibox mode: favorite/unfavorite selected item \|	125		\| Space \| Toggle Favorite \| Omnibox mode: favorite/unfavorite selected item \|
126		\| Ctrl+1-9, 0 \| Quick Navigate \| Jump to result by number (1-10) \|	126		\| Ctrl+1-9, 0 \| Quick Navigate \| Jump to result by number (1-10) \|
127		\| ↑/↓ Arrow \| Navigate Results \| Browse suggestions/matches \|	127		\| ↑/↓ Arrow \| Navigate Results \| Browse suggestions/matches \|
128	-	\| Hold key 400ms \| Accent Picker \| Shows accented variants (e.g., hold 'e' for è, é, ê, ë) \|	128	+	\| Hold key 400ms \| Accent Picker \| Shows accented variants in any text input (e.g., hold 'e' for è, é, ê, ë) \|
129			129
130		All zoom changes are automatically persisted per-domain and restored on next visit.	130		All zoom changes are automatically persisted per-domain and restored on next visit.
131			131

M internal/application/usecase/insert_accent.go +2 -1


@@ -103,8 +103,9 @@ func (uc *InsertAccentUseCase) OnKeyPressed(ctx context.Context, char rune, shif
103	103		Bool("shift", shiftHeld).
104	104		Msg("starting long-press detection")
105	105
	106	+	// Use context.Background() since timer fires after delay and original ctx may be canceled
106	107		uc.timer = time.AfterFunc(LongPressDelay, func() {
107		-	uc.onLongPressTriggered(ctx)
	108	+	uc.onLongPressTriggered(context.Background())
108	109		})
109	110
110	111		// Allow the first key press through (user might just want to type quickly)


@@ -103,8 +103,9 @@ func (uc *InsertAccentUseCase) OnKeyPressed(ctx context.Context, char rune, shif
103		Bool("shift", shiftHeld).	103		Bool("shift", shiftHeld).
104		Msg("starting long-press detection")	104		Msg("starting long-press detection")
105			105
			106	+	// Use context.Background() since timer fires after delay and original ctx may be canceled
106		uc.timer = time.AfterFunc(LongPressDelay, func() {	107		uc.timer = time.AfterFunc(LongPressDelay, func() {
107	-	uc.onLongPressTriggered(ctx)	108	+	uc.onLongPressTriggered(context.Background())
108		})	109		})
109			110
110		// Allow the first key press through (user might just want to type quickly)	111		// Allow the first key press through (user might just want to type quickly)

M internal/domain/entity/accent_test.go +18 -17


@@ -66,28 +66,29 @@ func TestGetAccents(t *testing.T) {
66	66
67	67		func TestGetAccents_UppercaseFromLowercase(t *testing.T) {
68	68		// When passing lowercase char with uppercase=true, should return uppercase accents
69		-	got := GetAccents('e', true)
70		-	if len(got) == 0 {
	69	+	uppercase := GetAccents('e', true)
	70	+	lowercase := GetAccents('e', false)
	71	+
	72	+	if len(uppercase) == 0 {
71	73		t.Fatal("expected accents for 'e' with uppercase=true")
72	74		}
73	75
74		-	// Verify all returned accents are uppercase
75		-	for _, accent := range got {
76		-	lower := GetAccents('e', false)
77		-	found := false
78		-	for _, l := range lower {
79		-	if accent != l {
80		-	found = true
81		-	break
82		-	}
83		-	}
84		-	if !found && accent == got[0] {
85		-	// First accent should be uppercase version of first lowercase accent
86		-	if accent != 'È' {
87		-	t.Errorf("expected uppercase accent È, got %q", accent)
88		-	}
	76	+	if len(uppercase) != len(lowercase) {
	77	+	t.Fatalf("uppercase and lowercase accent counts differ: %d vs %d", len(uppercase), len(lowercase))
	78	+	}
	79	+
	80	+	// Verify each uppercase accent is different from the corresponding lowercase accent
	81	+	for i, upper := range uppercase {
	82	+	lower := lowercase[i]
	83	+	if upper == lower {
	84	+	t.Errorf("accent at index %d is same for uppercase and lowercase: %q", i, upper)
89	85		}
90	86		}
	87	+
	88	+	// Verify first accent is È (uppercase of è)
	89	+	if uppercase[0] != 'È' {
	90	+	t.Errorf("expected first uppercase accent to be È, got %q", uppercase[0])
	91	+	}
91	92		}
92	93
93	94		func TestAccentMapCoverage(t *testing.T) {


@@ -66,28 +66,29 @@ func TestGetAccents(t *testing.T) {
66			66
67		func TestGetAccents_UppercaseFromLowercase(t *testing.T) {	67		func TestGetAccents_UppercaseFromLowercase(t *testing.T) {
68		// When passing lowercase char with uppercase=true, should return uppercase accents	68		// When passing lowercase char with uppercase=true, should return uppercase accents
69	-	got := GetAccents('e', true)	69	+	uppercase := GetAccents('e', true)
70	-	if len(got) == 0 {	70	+	lowercase := GetAccents('e', false)
			71	+
			72	+	if len(uppercase) == 0 {
71		t.Fatal("expected accents for 'e' with uppercase=true")	73		t.Fatal("expected accents for 'e' with uppercase=true")
72		}	74		}
73			75
74	-	// Verify all returned accents are uppercase	76	+	if len(uppercase) != len(lowercase) {
75	-	for _, accent := range got {	77	+	t.Fatalf("uppercase and lowercase accent counts differ: %d vs %d", len(uppercase), len(lowercase))
76	-	lower := GetAccents('e', false)	78	+	}
77	-	found := false	79	+
78	-	for _, l := range lower {	80	+	// Verify each uppercase accent is different from the corresponding lowercase accent
79	-	if accent != l {	81	+	for i, upper := range uppercase {
80	-	found = true	82	+	lower := lowercase[i]
81	-	break	83	+	if upper == lower {
82	-	}	84	+	t.Errorf("accent at index %d is same for uppercase and lowercase: %q", i, upper)
83	-	}
84	-	if !found && accent == got[0] {
85	-	// First accent should be uppercase version of first lowercase accent
86	-	if accent != 'È' {
87	-	t.Errorf("expected uppercase accent È, got %q", accent)
88	-	}
89		}	85		}
90		}	86		}
			87	+
			88	+	// Verify first accent is È (uppercase of è)
			89	+	if uppercase[0] != 'È' {
			90	+	t.Errorf("expected first uppercase accent to be È, got %q", uppercase[0])
			91	+	}
91		}	92		}
92			93
93		func TestAccentMapCoverage(t *testing.T) {	94		func TestAccentMapCoverage(t *testing.T) {

M internal/infrastructure/textinput/webview_target.go +2 -1


@@ -97,10 +97,11 @@ func (t *WebViewTarget) InsertText(ctx context.Context, text string) error {
97	97
98	98		// Restore original clipboard after a short delay
99	99		// This allows the paste operation to complete first
	100	+	// Use context.Background() since this cleanup should complete regardless of ctx cancellation
100	101		go func() {
101	102		time.Sleep(clipboardRestoreDelay)
102	103		if originalClipboard != "" {
103		-	if err := t.clipboard.WriteText(ctx, originalClipboard); err != nil {
	104	+	if err := t.clipboard.WriteText(context.Background(), originalClipboard); err != nil {
104	105		log.Debug().Err(err).Msg("failed to restore clipboard")
105	106		}
106	107		}

 	// Restore original clipboard after a short delay
 	// This allows the paste operation to complete first
+	// Use context.Background() since this cleanup should complete regardless of ctx cancellation
 	go func() {
 		time.Sleep(clipboardRestoreDelay)
 		if originalClipboard != "" {
-			if err := t.clipboard.WriteText(ctx, originalClipboard); err != nil {
+			if err := t.clipboard.WriteText(context.Background(), originalClipboard); err != nil {
 				log.Debug().Err(err).Msg("failed to restore clipboard")
 			}
 		}

M internal/ui/component/accent_picker.go +15 -20


@@ -13,9 +13,6 @@ import (
13	13		const (
14	14		accentPickerSpacing = 8 // Spacing between accent items in pixels.
15	15		maxNumberedAccentItems = 9 // Maximum items that can be selected with number keys (1-9).
16		-	accentIndex6 = 6 // Index for accent at position 7 (0-indexed).
17		-	accentIndex7 = 7 // Index for accent at position 8 (0-indexed).
18		-	accentIndex8 = 8 // Index for accent at position 9 (0-indexed).
19	16		)
20	17
21	18		// AccentPicker displays an overlay with accent character options.

@@ -175,28 +172,26 @@ func (ap *AccentPicker) updateSelection() {
175	172		// selectAccent selects the accent at the given index.
176	173		func (ap *AccentPicker) selectAccent(index int) {
177	174		ap.mu.Lock()
178		-	if index < 0 \|\| index >= len(ap.accents) {
	175	+	if index < 0 \|\| index >= len(ap.accents) \|\| ap.selectedCb == nil {
179	176		ap.mu.Unlock()
180	177		return
181	178		}
182	179		accent := ap.accents[index]
183		-	cb := ap.selectedCb
	180	+	// Invoke callback while holding lock to avoid race with Hide() clearing callbacks
	181	+	ap.selectedCb(accent)
184	182		ap.mu.Unlock()
185		-
186		-	if cb != nil {
187		-	cb(accent)
188		-	}
189	183		}
190	184
191	185		// cancel cancels the accent picker.
192	186		func (ap *AccentPicker) cancel() {
193	187		ap.mu.Lock()
194		-	cb := ap.cancelCb
195		-	ap.mu.Unlock()
196		-
197		-	if cb != nil {
198		-	cb()
	188	+	if ap.cancelCb == nil {
	189	+	ap.mu.Unlock()
	190	+	return
199	191		}
	192	+	// Invoke callback while holding lock to avoid race with Hide() clearing callbacks
	193	+	ap.cancelCb()
	194	+	ap.mu.Unlock()
200	195		}
201	196
202	197		// setupKeyboardHandling sets up key event handling for navigation.

@@ -309,12 +304,12 @@ func (ap *AccentPicker) numberKeyToIndex(keyval uint) (int, bool) {
309	304		uint(gdk.KEY_KP_5): 4,
310	305		uint(gdk.KEY_6): 5,
311	306		uint(gdk.KEY_KP_6): 5,
312		-	uint(gdk.KEY_7): accentIndex6,
313		-	uint(gdk.KEY_KP_7): accentIndex6,
314		-	uint(gdk.KEY_8): accentIndex7,
315		-	uint(gdk.KEY_KP_8): accentIndex7,
316		-	uint(gdk.KEY_9): accentIndex8,
317		-	uint(gdk.KEY_KP_9): accentIndex8,
	307	+	uint(gdk.KEY_7): 6,
	308	+	uint(gdk.KEY_KP_7): 6,
	309	+	uint(gdk.KEY_8): 7,
	310	+	uint(gdk.KEY_KP_8): 7,
	311	+	uint(gdk.KEY_9): 8,
	312	+	uint(gdk.KEY_KP_9): 8,
318	313		}
319	314
320	315		if idx, ok := keyMap[keyval]; ok {


@@ -13,9 +13,6 @@ import (
13		const (	13		const (
14		accentPickerSpacing = 8 // Spacing between accent items in pixels.	14		accentPickerSpacing = 8 // Spacing between accent items in pixels.
15		maxNumberedAccentItems = 9 // Maximum items that can be selected with number keys (1-9).	15		maxNumberedAccentItems = 9 // Maximum items that can be selected with number keys (1-9).
16	-	accentIndex6 = 6 // Index for accent at position 7 (0-indexed).
17	-	accentIndex7 = 7 // Index for accent at position 8 (0-indexed).
18	-	accentIndex8 = 8 // Index for accent at position 9 (0-indexed).
19		)	16		)
20			17
21		// AccentPicker displays an overlay with accent character options.	18		// AccentPicker displays an overlay with accent character options.

@@ -175,28 +172,26 @@ func (ap *AccentPicker) updateSelection() {
175		// selectAccent selects the accent at the given index.	172		// selectAccent selects the accent at the given index.
176		func (ap *AccentPicker) selectAccent(index int) {	173		func (ap *AccentPicker) selectAccent(index int) {
177		ap.mu.Lock()	174		ap.mu.Lock()
178	-	if index < 0 \|\| index >= len(ap.accents) {	175	+	if index < 0 \|\| index >= len(ap.accents) \|\| ap.selectedCb == nil {
179		ap.mu.Unlock()	176		ap.mu.Unlock()
180		return	177		return
181		}	178		}
182		accent := ap.accents[index]	179		accent := ap.accents[index]
183	-	cb := ap.selectedCb	180	+	// Invoke callback while holding lock to avoid race with Hide() clearing callbacks
			181	+	ap.selectedCb(accent)
184		ap.mu.Unlock()	182		ap.mu.Unlock()
185	-
186	-	if cb != nil {
187	-	cb(accent)
188	-	}
189		}	183		}
190			184
191		// cancel cancels the accent picker.	185		// cancel cancels the accent picker.
192		func (ap *AccentPicker) cancel() {	186		func (ap *AccentPicker) cancel() {
193		ap.mu.Lock()	187		ap.mu.Lock()
194	-	cb := ap.cancelCb	188	+	if ap.cancelCb == nil {
195	-	ap.mu.Unlock()	189	+	ap.mu.Unlock()
196	-		190	+	return
197	-	if cb != nil {
198	-	cb()
199		}	191		}
			192	+	// Invoke callback while holding lock to avoid race with Hide() clearing callbacks
			193	+	ap.cancelCb()
			194	+	ap.mu.Unlock()
200		}	195		}
201			196
202		// setupKeyboardHandling sets up key event handling for navigation.	197		// setupKeyboardHandling sets up key event handling for navigation.

@@ -309,12 +304,12 @@ func (ap *AccentPicker) numberKeyToIndex(keyval uint) (int, bool) {
309		uint(gdk.KEY_KP_5): 4,	304		uint(gdk.KEY_KP_5): 4,
310		uint(gdk.KEY_6): 5,	305		uint(gdk.KEY_6): 5,
311		uint(gdk.KEY_KP_6): 5,	306		uint(gdk.KEY_KP_6): 5,
312	-	uint(gdk.KEY_7): accentIndex6,	307	+	uint(gdk.KEY_7): 6,
313	-	uint(gdk.KEY_KP_7): accentIndex6,	308	+	uint(gdk.KEY_KP_7): 6,
314	-	uint(gdk.KEY_8): accentIndex7,	309	+	uint(gdk.KEY_8): 7,
315	-	uint(gdk.KEY_KP_8): accentIndex7,	310	+	uint(gdk.KEY_KP_8): 7,
316	-	uint(gdk.KEY_9): accentIndex8,	311	+	uint(gdk.KEY_9): 8,
317	-	uint(gdk.KEY_KP_9): accentIndex8,	312	+	uint(gdk.KEY_KP_9): 8,
318		}	313		}
319			314
320		if idx, ok := keyMap[keyval]; ok {	315		if idx, ok := keyMap[keyval]; ok {