Skip to content

fix: retain concurrent links in the browser picker - #14

Merged
jnahian merged 3 commits into
jnahian:mainfrom
alimuzzaman:codex/fix-concurrent-link-picker
Sep 7, 2026
Merged

jnahian merged 3 commits into
jnahian:mainfrom
alimuzzaman:codex/fix-concurrent-link-picker

Conversation

@alimuzzaman

@alimuzzaman alimuzzaman commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Multiple link-open events replaced the picker on each arrival, leaving only the last URL. Keep links routed to the picker in one visible batch. Choosing a browser opens the ordered batch in one native request; Copy Links copies the complete newline-separated list. Escape/click-away cancels the displayed batch, and Create Rule remains available for one link.

Resolve the destination/profile once and launch one process for a profile batch. Completion owns retry handling: failures preserve the original batch, new arrivals wait while opening, and stale/duplicate completions are ignored. Keep the single-URL dispatcher API compatible.

Constrain the picker to the screen's usable bounds and scroll long browser lists. Keyboard navigation reveals the selected row and keeps its selection when links arrive. Copy confirmation reports the number of links.

Validation:

  • swift test: 109 tests passed, including 22 app/dispatcher tests.
  • swift build and universal Scripts/bundle-app.sh: passed.
  • Relocated bundle CLI/resource smoke passed with .build temporarily absent.
  • web: npm test and npm run build passed.
  • Native 21-choice fixture: the old batch grew to 862 points on an 841-point visible screen; the corrected panel stays within the 825-point inset bounds.
  • Hosted NSPanel tests exercise the real responder's copy/Escape/arrows/Return methods, verify the entire final browser row is visible, and append links after navigation before selection.
  • Controlled native activation reproduced the old mixed-success/retry loss. One native request removes that per-link race. A stationary-pointer scroll check selected the intended final browser.

Actual browser-tab delivery, OS key-equivalent routing, and broader OS activation timing remain unverified. GitHub Actions and Vercel require maintainer authorization for this fork.

The preview below uses two normal browser choices. Repeated Chrome rows used during overflow testing were fixture data and are not part of the app.

Picker retaining three incoming links

@vercel

vercel Bot commented Sep 7, 2026

Copy link
Copy Markdown

@alimuzzaman is attempting to deploy a commit to the Julkar Naen Nahian's projects Team on Vercel.

A member of the Team first needs to authorize it.

alimuzzaman and others added 2 commits September 7, 2026 14:58
Review of jnahian#14 surfaced a path that still loses links. finishOpening
presents the links that arrived during a launch, then the browser we just
opened — launched with activates = true — takes key a beat later.
windowDidResignKey read that as click-away and discarded a batch the user
had never seen: not opened, not copied, not in the fallback.

A batch surfaced by finishOpening is now marked unseen and ignores focus
loss until the user proves they have looked at it, by pressing a key or
clicking the panel. Hover deliberately does not count: the panel opens at
the cursor, so mouse drift would clear the flag in the same moment the
browser takes key. A click cannot come from a browser activating, so
accepting it keeps click-away dismissal working as it has since 0.8.2 —
without it, an unseen panel would need a click *and* an Esc to clear.
Pickers the user opened themselves are unaffected.

Also from the review:

- The no-browsers degrade path issued one NSWorkspace request per link,
  which is the race the batching was meant to remove. It now goes through
  a single Dispatcher.openInBrowser(urls:) like every other path.
- Arriving links appended below the fold of the link list and were never
  revealed. The list now scrolls to the newest arrival.
- The browser list was force-scrolled back to the selection on every
  arrival; that redundant scroll is gone. Note this does not make a manual
  scroll survive an arrival — SwiftUI restores its own remembered offset
  when the content changes, which this does not address.
- Row heights were hardcoded at 17/34pt and drive the panel's fittingSize,
  so Larger Text clipped the list. They scale with the text size now.
- resizePanel ran twice per arrival and read fittingSize in the same turn
  it set maximumHeight, so a panel moved between screens could be measured
  against the constraint it just left.
- installPanelForTesting is behind #if DEBUG, the dead AppState field is
  gone, Environment.live is a static let, JunctionAppTests declares
  JunctionMacKit explicitly, and the Dispatcher tests move to their own
  target rather than resolving the module transitively.
- Drops docs/picker-multiple-links.png, which nothing in the repo
  references, and replaces the layout tests' fixed 50ms settle with
  bounded polling so a loaded runner doesn't flake.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQGKvo7zQ2D5sV31uAxfnM
@jnahian
jnahian merged commit d02c90a into jnahian:main Sep 7, 2026
5 of 6 checks passed
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.

2 participants