Skip to content

fix(app,web): put the caret in the target picker's search when it opens - #721

Merged
efiten merged 2 commits into
efiten:masterfrom
khagele:fix/714-picker-search-focus
Sep 30, 2026
Merged

efiten merged 2 commits into
efiten:masterfrom
khagele:fix/714-picker-search-focus

Conversation

@khagele

@khagele khagele commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

Closes #714

What

Reaching a node by name took three actions: open the picker, press its search field, type. Opening now puts the caret in the search in the same tap, so a phone raises its keyboard and the next keystroke searches.

app. createTargetList gets open(), which the target chip calls where it called reset():

  • the query is cleared, so every open starts from the full list; a query left from the last open used to come back with a list narrowed by it and nothing saying why;
  • the list repaints at once when there was a query, so the narrowed list never shows for the second until the next tick;
  • #ts-search takes the focus (preventScroll).

A sheet that closes with the focus inside lets go of it (releaseFocus), so the caret and the keyboard do not stay on a hidden field.

web. wirePopover takes a focusEl. The sender picker passes #f-sender, which keeps its value: on web it is the sender filter itself, bound to ?sender=. Closing with the focus inside hands it back to the toggle; that holds for the hunter and ignore pickers too, which pass no focusEl.

Verification

  • targetlist.test.js (new): open() focuses the field, clears the query and repaints. Each line of open() red when removed.
  • multiselect.test.js: focus on open with preventScroll and the value kept; focus back to the toggle on Escape, and left alone when it was elsewhere. Red under mutation.
  • e2e/targetpicker.spec.js: in Chromium the caret is in #f-sender after the toggle, typing lands there, Escape hands focus back. Red without the focusEl.
  • App at 390x844 with eight senders: focus on #ts-search, dik narrows to one row, the field and the matches stay above a 300 px keyboard; reopening shows an empty field and the full list.
  • app: 1625 tests green, eslint clean
  • web: 765 tests green, full e2e 298 passed

🤖 Generated with Claude Code

@khagele
khagele force-pushed the fix/714-picker-search-focus branch from f22bd7a to cb3f212 Compare September 28, 2026 14:08
@khagele

khagele commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Amended (cb3f212): in the app, a sheet that closes with the focus inside now hands it back to its toggle (releaseFocus(sheet, toggle) in app.js), as wirePopover does on web. Before, it blurred to the body, so a keyboard user on desktop lost their place. App: 1625 tests, eslint, build.

@efiten

efiten commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Review before merge: two points to change, two suggestions. Found by reading the code at cb3f212, not run in a browser.

1. Escape in the web sender picker can clear the sender filter (change requested)
web/multiselect.js:264 closes the panel on Escape without preventDefault(), and #f-sender is type="search" (web/index.html:144). In Chromium and WebKit, Escape in a non-empty search field clears it and fires input, which map.js binds to ?sender=. Before this PR the focus stayed on the toggle, so Escape only closed the panel. Now the caret is in the field, so open, glance, Escape can wipe an active sender filter.
Proposed: e.preventDefault() before close(), and in e2e/targetpicker.spec.js assert that #f-sender still holds cc after the Escape.

2. The app sheet's own close button bypasses releaseFocus (change requested)
buildTargetSheet in app/src/app.js still wires ts-close as sheet.hidden = true. On desktop Chrome the click focuses #ts-close, and hiding the sheet then drops focus to the body: the case the amend describes fixing. The same path exists where filter-pill and settings-btn hide the target sheet directly (app.js:3932, 3942).

3. Suggestion: a test for releaseFocus
The helper and its two call sites have no test (AGENTS.md §5.1). A fake-DOM test like the web one would pin it.

4. Suggestion: the repaint test
targetlist.test.js checks that the list goes from 1 child to 0, not that browseEl becomes visible again, which is the part a user sees.

Reaching a node by name took three actions: open the picker, press its
search field, type. Opening the app's target sheet and the web's sender
picker now focuses the search in the same tap, so a phone raises its
keyboard and the next keystroke searches. The app's sheet also opens on an
empty query every time; a query left from the last open used to come back
with a list narrowed by it. On web the field keeps its value, since it is
the sender filter bound to ?sender=. Closing with the focus inside lets go
of it rather than leaving it on a hidden field.

Closes efiten#714

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@khagele
khagele force-pushed the fix/714-picker-search-focus branch from cb3f212 to 5d6d451 Compare September 30, 2026 17:52
@khagele

khagele commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 5d6d451 for the review, rebased on master. Per point:

  1. Escape clears the sender filter. wirePopover calls preventDefault() before close(). e2e/targetpicker.spec.js now asserts that #f-sender still holds cc after the Escape (it read "" before the fix), and the unit test checks defaultPrevented.
  2. Close paths that bypassed it. closeSheet(sheet, toggle) in app/src/sheetfocus.js hides a sheet and hands the focus back to its toggle when it was inside. Every path that hides one of the three sheets goes through it: the close buttons (fs-close, ts-close, ss-close), a toggle closing its own sheet, a sheet closing the other two when it opens, a tap outside, and the Disconnect and "How to use" buttons in Settings. No sheet.hidden = true is left in app.js.
  3. A test for it. sheetfocus.test.js with a fake sheet and toggle: focus inside goes to the toggle, focus elsewhere stays. Red with the focus line removed.
  4. The repaint test. It now passes a browseEl and checks that it is hidden under the query and visible again after open(). Red without the repaint.

App: 1632 tests, eslint, build. Web: 765 tests, eslint, Playwright 298 passed. The app's sheets were not clicked through in a browser.

@efiten
efiten merged commit 6bd899c into efiten:master Sep 30, 2026
6 checks passed
@github-actions github-actions Bot mentioned this pull request Sep 30, 2026
efiten pushed a commit that referenced this pull request Sep 30, 2026
🤖 I have created a release *beep* *boop*
---


<details><summary>app: 1.30.0</summary>

##
[1.30.0](app-v1.29.1...app-v1.30.0)
(2026-09-30)


### Features

* **app,web:** ease what the map shows by zoom, on one hex system
([#734](#734))
([4c2a57b](4c2a57b))
* **app,web:** name a relay on the map the way the app does
([#733](#733))
([a9aa01a](a9aa01a))
* **app:** send one broker to several collectors under one switch
([#722](#722))
([8c6da30](8c6da30))
* **web:** export the repeaters heard in view as a 1200×1200 PNG
([#725](#725))
([d6186bc](d6186bc))


### Bug Fixes

* **app,web:** hang a Discover-heard repeater's star from its own
position ([#724](#724))
([15a6ada](15a6ada))
* **app,web:** put the caret in the target picker's search when it opens
([#721](#721))
([6bd899c](6bd899c))
* **app:** drop the float window's previous and next buttons
([#717](#717))
([c957ff3](c957ff3))
* **app:** give the sound the playback buffer instead of the smallest
([#712](#712))
([dee5484](dee5484))
* **app:** hide Chrome's pause on the fullscreen readout
([#715](#715))
([836d91e](836d91e))
* **app:** keep a space under the HUD on Android
([#710](#710))
([2754716](2754716))
* **app:** let the fullscreen readout turn with the phone
([#718](#718))
([fae3749](fae3749))
* **app:** put the transmit pop under the receptions it sat over
([#738](#738))
([2bfc5fa](2bfc5fa))
* **server:** keep the node registry in memory on a loop of its own
([#736](#736))
([4f3e988](4f3e988))
* **web:** keep the map bar on one row between 641 and 767 px
([#732](#732))
([c44dc1d](c44dc1d))
</details>

<details><summary>server: 1.10.1</summary>

##
[1.10.1](server-v1.10.0...server-v1.10.1)
(2026-09-30)


### Bug Fixes

* **server:** keep the node registry in memory on a loop of its own
([#736](#736))
([4f3e988](4f3e988))
* **server:** read a refused frame again when the message-id decoder
changes ([#737](#737))
([8586c71](8586c71))
</details>

<details><summary>web: 1.26.0</summary>

##
[1.26.0](web-v1.25.1...web-v1.26.0)
(2026-09-30)


### Features

* **app,web:** ease what the map shows by zoom, on one hex system
([#734](#734))
([4c2a57b](4c2a57b))
* **app,web:** name a relay on the map the way the app does
([#733](#733))
([a9aa01a](a9aa01a))
* **app:** send one broker to several collectors under one switch
([#722](#722))
([8c6da30](8c6da30))
* **web:** export the repeaters heard in view as a 1200×1200 PNG
([#725](#725))
([d6186bc](d6186bc))


### Bug Fixes

* **app,web:** hang a Discover-heard repeater's star from its own
position ([#724](#724))
([15a6ada](15a6ada))
* **app,web:** put the caret in the target picker's search when it opens
([#721](#721))
([6bd899c](6bd899c))
* **app:** drop the float window's previous and next buttons
([#717](#717))
([c957ff3](c957ff3))
* **app:** give the sound the playback buffer instead of the smallest
([#712](#712))
([dee5484](dee5484))
* **app:** hide Chrome's pause on the fullscreen readout
([#715](#715))
([836d91e](836d91e))
* **app:** keep a space under the HUD on Android
([#710](#710))
([2754716](2754716))
* **app:** let the fullscreen readout turn with the phone
([#718](#718))
([fae3749](fae3749))
* **app:** put the transmit pop under the receptions it sat over
([#738](#738))
([2bfc5fa](2bfc5fa))
* **server:** keep the node registry in memory on a loop of its own
([#736](#736))
([4f3e988](4f3e988))
* **web:** keep the map bar on one row between 641 and 767 px
([#732](#732))
([c44dc1d](c44dc1d))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

app,web: the target picker opens with its search field unfocused, and the app's keeps the last query

2 participants