Skip to content

Make import candidate selection a native radiogroup #136

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.

Sign up for GitHub

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

Merged
AdamXweb merged 1 commit into from
Jul 23, 2026
Merged

Make import candidate selection a native radiogroup #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
204 changes: 112 additions & 92 deletions app/components/OnboardWizard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8990,6 +8990,7 @@ function SearchResultBlock({
const [isEditing, setIsEditing] = useState(false);
const [draft, setDraft] = useState(result.query);
const [draftDeveloper, setDraftDeveloper] = useState(developerHint);
const candidateRadioKeyDown = useRovingRadioGroup();

// When collapsed and the user has chosen a candidate, show THAT one — not
// the iTunes #1 pick — so "Show less" after selecting a non-top match
Expand Down Expand Up @@ -9313,75 +9314,93 @@ function SearchResultBlock({
</div>
) : (
<>
{candidates.map((candidate) => {
// Bundle-ID fallback catches the legacy-import duplicate
// case where the same physical app exists under a
// different App Store track ID — see TrackedApp comment.
const candidateTracked =
trackedByAppleId.get(candidate.appleId) ??
(candidate.bundleId
? trackedByBundleId?.get(candidate.bundleId)
: undefined);
const bundleMismatch = Boolean(
result.sourceBundleId &&
candidate.bundleId &&
result.sourceBundleId.toLowerCase() !==
candidate.bundleId.toLowerCase()
);
return (
<div
// The `tracked` modifier applies row-level styling (tint
// + left border). The inline "Tracked" chip next to the
// candidate name is back on top of that — removing it
// made the selected-candidate case ambiguous when the
// block-level "Re-sync App info" pill scrolled off-
// screen on long lists, so the per-row chip earns its
// keep even with some visual duplication.
className={`candidate-row ${chosen?.appleId === candidate.appleId ? "chosen" : ""} ${candidateTracked ? "tracked" : ""}`}
key={candidate.appleId}
onClick={() =>
onChoose(
chosen?.appleId === candidate.appleId ? null : candidate
)
}
>
<span
style={{
width: 18,
height: 18,
borderRadius: 4,
border: `2px solid ${chosen?.appleId === candidate.appleId ? "var(--blue)" : "var(--border-strong)"}`,
background:
chosen?.appleId === candidate.appleId
? "var(--blue)"
: "transparent",
display: "flex",
alignItems: "center",
justifyContent: "center",
fontSize: 10,
color: "#fff",
flexShrink: 0,
transition: "all 0.15s",
}}
{/* Native radio semantics for candidate selection (roving
radiogroup pattern — see lib/use-roving-radiogroup.ts and
the biome.jsonc a11y-override rationale): Tab enters the
group on the chosen row, arrows move with selection-
follows-focus, and clicking/Space on the chosen row again
still clears it (toggle radios — the hook's checked guard
keeps arrow passes from clearing on the way through). */}
<div
aria-label={t("candidates_group_aria", { query: result.query })}
onKeyDown={candidateRadioKeyDown}
role="radiogroup"
>
{candidates.map((candidate, candidateIndex) => {
// Bundle-ID fallback catches the legacy-import duplicate
// case where the same physical app exists under a
// different App Store track ID — see TrackedApp comment.
const candidateTracked =
trackedByAppleId.get(candidate.appleId) ??
(candidate.bundleId
? trackedByBundleId?.get(candidate.bundleId)
: undefined);
const bundleMismatch = Boolean(
result.sourceBundleId &&
candidate.bundleId &&
result.sourceBundleId.toLowerCase() !==
candidate.bundleId.toLowerCase()
);
const isChosen = chosen?.appleId === candidate.appleId;
return (
<button
aria-checked={isChosen}
// The `tracked` modifier applies row-level styling (tint
// + left border). The inline "Tracked" chip next to the
// candidate name is back on top of that — removing it
// made the selected-candidate case ambiguous when the
// block-level "Re-sync App info" pill scrolled off-
// screen on long lists, so the per-row chip earns its
// keep even with some visual duplication.
className={`candidate-row ${isChosen ? "chosen" : ""} ${candidateTracked ? "tracked" : ""}`}
key={candidate.appleId}
onClick={() => onChoose(isChosen ? null : candidate)}
role="radio"
tabIndex={rovingTabIndex(
isChosen,
candidateIndex,
chosenIsVisibleWhenCollapsed
)}
type="button"
>
{chosen?.appleId === candidate.appleId ? "✓" : ""}
</span>
<span
aria-hidden="true"
style={{
width: 18,
height: 18,
borderRadius: 4,
border: `2px solid ${isChosen ? "var(--blue)" : "var(--border-strong)"}`,
background: isChosen ? "var(--blue)" : "transparent",
display: "flex",
alignItems: "center",
justifyContent: "center",
fontSize: 10,
color: "#fff",
flexShrink: 0,
transition: "all 0.15s",
}}
>
{isChosen ? "✓" : ""}
</span>

{candidate.iconUrl && (
<Image
alt={candidate.name}
className="candidate-icon"
height={40}
src={candidate.iconUrl}
style={{ objectFit: "cover" }}
unoptimized
width={40}
/>
)}
<div className="candidate-body">
<div className="candidate-name">
{candidate.name}
{/* Inline "already tracking" chip. Renders for every
{candidate.iconUrl && (
<Image
// Decorative inside the radio button — the adjacent
// candidate-name text IS the accessible name; a
// non-empty alt would read the name twice.
alt=""
className="candidate-icon"
height={40}
src={candidate.iconUrl}
style={{ objectFit: "cover" }}
unoptimized
width={40}
/>
)}
<div className="candidate-body">
<div className="candidate-name">
{candidate.name}
{/* Inline "already tracking" chip. Renders for every
tracked candidate (not just the chosen one) so
users browsing alternate matches can still tell
which rows would re-sync rather than add a
Expand All @@ -9391,33 +9410,34 @@ function SearchResultBlock({
deliberate: the chip is visible alongside the
name even on long lists where the block header
has scrolled off. */}
{candidateTracked && (
<span
aria-label={t("candidate_tracking_aria")}
className="candidate-tracked-chip"
>
{t("candidate_tracking_chip")}
</span>
)}
{bundleMismatch && (
<span className="candidate-bundle-warning">
Bundle differs
</span>
{candidateTracked && (
<span
aria-label={t("candidate_tracking_aria")}
className="candidate-tracked-chip"
>
{t("candidate_tracking_chip")}
</span>
)}
{bundleMismatch && (
<span className="candidate-bundle-warning">
Bundle differs
</span>
)}
</div>
<div className="candidate-dev">{candidate.developer}</div>
{result.sourceBundleId && (
<div className="candidate-dev">
Imported {result.sourceBundleId}
{candidate.bundleId
? ` · App Store ${candidate.bundleId}`
: ""}
</div>
)}
</div>
<div className="candidate-dev">{candidate.developer}</div>
{result.sourceBundleId && (
<div className="candidate-dev">
Imported {result.sourceBundleId}
{candidate.bundleId
? ` · App Store ${candidate.bundleId}`
: ""}
</div>
)}
</div>
</div>
);
})}
</button>
);
})}
</div>

{result.candidates.length > 1 && (
<button
Expand Down
10 changes: 10 additions & 0 deletions app/globals.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5705,8 +5705,18 @@ html[data-theme-override="high-contrast"] .nav {
display: flex;
gap: 12px;
align-items: center;
width: 100%;
padding: 8px 10px;
font: inherit;
color: inherit;
text-align: left;
/* The row is a <button role="radio"> (native semantics for the
candidate picker) — neutralise the UA button chrome so it keeps
rendering as the flex row it always was. */
appearance: none;
cursor: pointer;
background: transparent;
border: 0;
border-radius: var(--r-md);
transition: background 0.12s;
}
Expand Down
1 change: 1 addition & 0 deletions locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,7 @@
"task_steps_label": "{done} / {total} steps"
},
"search_block": {
"candidates_group_aria": "App Store matches for {query}",
"edit_app_name": "App name",
"edit_seller": "Seller",
"edit_seller_csv": "(from CSV — tweak to re-rank)",
Expand Down
1 change: 1 addition & 0 deletions locales/zh.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,7 @@
"task_steps_label": "{done} / {total} 步"
},
"search_block": {
"candidates_group_aria": "“{query}”的 App Store 匹配结果",
"edit_app_name": "应用名称",
"edit_seller": "开发者",
"edit_seller_csv": "（来自 CSV — 调整以重新排序）",
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/a11y.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ browserFlow(
});

// Step 3 — matched block with the candidate list expanded, so the
// candidate rows (the surface awaiting native controls) are in
// candidate rows (a roving radiogroup of role=radio buttons) are in
// the scanned DOM.
await page.getByTestId("onboard-app-names").fill("Notes");
await page.getByTestId("imported-apps-add").click();
Expand Down
102 changes: 57 additions & 45 deletions tests/e2e/onboarding-keyboard.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ import { tabTo } from "./helpers/keyboard";
* "Keyboard navigation" system setting makes WebKit skip buttons and
* links entirely, which is an OS behaviour, not a bug (see AGENTS.md).
*
* The candidate-selection spec at the bottom is `fixme`-gated: it is
* the acceptance test for the tracked candidate-selection fix
* (candidate rows are bare clickable <div>s today). Remove the `fixme`
* in the PR that converts them to native controls.
* Covers the full manual-entry path including candidate selection —
* the candidate rows are a roving radiogroup, so the last spec guards
* the arrow-key selection contract.
*/

const sameOriginHeaders = {
Expand Down Expand Up @@ -186,49 +185,62 @@ browserFlow(
);

// ---------------------------------------------------------------------------
// Acceptance test for the tracked candidate-selection fix
// Candidate selection — regression guard for the radiogroup conversion
// ---------------------------------------------------------------------------
//
// The candidate rows in SearchResultBlock are bare clickable <div>s
// today (no role, no tabIndex, no keyboard handler), so this spec
// cannot pass. It documents the required end state: expand the
// alternate candidates and choose one using only the keyboard. When
// the fix converts the rows to native controls (radiogroup pattern, like
// the wizard's method cards), remove this `fixme` and the spec becomes
// the regression guard for that fix.

// `test.fixme` (not `browserFlow.fixme`) — the CODEX_SANDBOX alias is
// `test.skip`, which has no `.fixme` property; fixme already never runs.
test.fixme("keyboard-only: pick a non-default candidate in the match step (pending candidate-row fix)", async ({
page,
}) => {
await mockSearchFromFixtures(page);
await keyboardToTextEntry(page);

const textarea = page.getByTestId("onboard-app-names");
await tabTo(page, textarea);
await page.keyboard.type("Notes");
const addBtn = page.getByTestId("imported-apps-add");
await tabTo(page, addBtn);
await page.keyboard.press("Enter");
const searchBtn = page.getByTestId("onboard-search");
await tabTo(page, searchBtn);
await page.keyboard.press("Enter");
// The candidate rows are `<button role="radio">` inside a roving
// radiogroup (lib/use-roving-radiogroup.ts): Tab enters the group on
// the chosen row, arrows move with selection-follows-focus. This spec
// picks the non-default Apple candidate using only the keyboard.

const block = page
.locator(".search-result-item")
.filter({ hasText: "Notes" });
await expect(block).toHaveCount(1);
browserFlow(
"keyboard-only: pick a non-default candidate in the match step",
async ({ page }) => {
await mockSearchFromFixtures(page);
await keyboardToTextEntry(page);

// Expand the alternate candidates from the keyboard.
const showMore = block.locator(".show-more-btn");
await tabTo(page, showMore);
await page.keyboard.press("Enter");
const textarea = page.getByTestId("onboard-app-names");
await tabTo(page, textarea);
await page.keyboard.type("Notes");
const addBtn = page.getByTestId("imported-apps-add");
await tabTo(page, addBtn);
await page.keyboard.press("Enter");
const searchBtn = page.getByTestId("onboard-search");
await tabTo(page, searchBtn);
await page.keyboard.press("Enter");

// Reach the Apple candidate row and select it — this is the part
// that requires the candidate rows to become native controls.
const appleRow = block.locator(".candidate-row").filter({ hasText: "Apple" });
await tabTo(page, appleRow);
await page.keyboard.press("Space");
await expect(appleRow).toHaveClass(/chosen/);
});
const block = page
.locator(".search-result-item")
.filter({ hasText: "Notes" });
await expect(block).toHaveCount(1);

// The auto-chosen top candidate is the group's tab stop.
const defaultRow = block
.locator(".candidate-row")
.filter({ hasText: "Random Notes Co" });

// Expand the alternate candidates from the keyboard.
const showMore = block.locator(".show-more-btn");
await tabTo(page, showMore);
await page.keyboard.press("Enter");

// Radios sit before the show-more button in DOM order, so step
// BACK into the group — focus lands on the chosen radio (the
// roving tab stop), then ArrowDown moves to the Apple row and
// selects it (selection follows focus).
await page.keyboard.press("Shift+Tab");
await expect(defaultRow).toBeFocused();
await page.keyboard.press("ArrowDown");

const appleRow = block
.locator(".candidate-row")
.filter({ hasText: "Apple" });
await expect(appleRow).toBeFocused();
await expect(appleRow).toHaveClass(/chosen/);
await expect(appleRow).toHaveAttribute("aria-checked", "true");
await expect(defaultRow).toHaveAttribute("aria-checked", "false");

// The import CTA reflects the keyboard-made choice.
await expect(page.getByTestId("onboard-confirm-import")).toBeEnabled();
}
);
Loading