Skip to content

Name the yield shortcut the way a Mac prints it #225

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

Open
sakinu wants to merge 1 commit into
base: main
Choose a base branch
from
+11 −10
Open

Name the yield shortcut the way a Mac prints it #225

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
13 changes: 7 additions & 6 deletions tests/browser/turn-taking.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,10 +277,11 @@ test("the page names the yield control and its shortcut the way the handler read
button.indexOf("</button>"),
);
assert.equal(label.trim(), "Your turn is done");
assert.match(
button.slice(0, button.indexOf(">")),
/aria-keyshortcuts="Alt\+Enter"/,
);
const attributes = button.slice(0, button.indexOf(">"));
assert.match(attributes, /aria-keyshortcuts="Alt\+Enter"/);
// Apple keyboards print the modifier as option, not alt, so the tooltip
// has to name both spellings or the hint points at no key on a Mac.
assert.match(attributes, /title="Alt\+Enter \/ Option\+Enter"/);

@ColtenOuO ColtenOuO Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assertions compare each label to a literal, not to the handler. If isYieldShortcut changes its chord, the labels go stale and this still passes. Exporting the label from lib.js next to isYieldShortcut and building all three strings from it would tie the label to what the handler matches.

const ring = html.slice(html.indexOf('id="turn-ring"'));
assert.match(
ring.slice(0, ring.indexOf(">")),
Expand All @@ -290,7 +291,7 @@ test("the page names the yield control and its shortcut the way the handler read
const copy = status
.slice(status.indexOf(">") + 1, status.indexOf("</p>"))
.replace(/\s+/g, " ");
assert.match(copy, /Your turn is done \(Alt\+Enter\)/);
assert.match(copy, /Your turn is done \(Alt\+Enter \/ Option\+Enter\)/);
// The shortcut the copy names is the one the handler takes.
const editor = { tagName: "TEXTAREA" };
assert.equal(
Expand All @@ -303,6 +304,6 @@ test("the page names the yield control and its shortcut the way the handler read
// The status line the page restores after a hold says the same.
assert.match(
read("web/interview.js"),
/Choose Your turn is done \(Alt\+Enter\) to let Jim reply early\./,
/Choose Your turn is done \(Alt\+Enter \/ Option\+Enter\) to let Jim reply early\./,
);
});
6 changes: 3 additions & 3 deletions web/interview.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ <h1 id="problem-title">Loading interview...</h1>
id="yield-turn"
class="pill-button"
type="button"
title="Alt+Enter"
title="Alt+Enter / Option+Enter"

@ColtenOuO ColtenOuO Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit says this follows Run tests, but that button shows the Mac chord as the Command sign with no plus (⌘Enter). To match it, this would show the Option sign the same way (⌥Enter), written as the entity &#8997; like Run tests does, so the file stays ASCII. Otherwise the toolbar names Mac chords two different ways.

aria-keyshortcuts="Alt+Enter"
>
Your turn is done
Expand Down Expand Up @@ -467,8 +467,8 @@ <h2>Media preflight</h2>
hidden
></button>
<p id="turn-status" class="muted small" role="status">
Take your time. Choose Your turn is done (Alt+Enter) to let Jim
reply early.
Take your time. Choose Your turn is done (Alt+Enter / Option+Enter)

@ColtenOuO ColtenOuO Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#turn-status is role="status", so a screen reader reads both chords aloud each time Thinking toggles, and a Windows user hears about an Option key they don't have. A tooltip can list both chords, but a live region should name only the one for the platform it's running on.

to let Jim reply early.
</p>
</div>
<div
Expand Down
2 changes: 1 addition & 1 deletion web/interview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1973,7 +1973,7 @@ function applyThinking(thinking) {
nodes.thinking.setAttribute("aria-pressed", String(thinking));
nodes.turnStatus.textContent = thinking
? "Jim will wait. Speak again or choose Continue when ready. The timer keeps running."
: "Take your time. Choose Your turn is done (Alt+Enter) to let Jim reply early.";
: "Take your time. Choose Your turn is done (Alt+Enter / Option+Enter) to let Jim reply early.";

@ColtenOuO ColtenOuO Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence now lives in two places: here and in the initial copy in interview.html. The chord also appears a third time, in the button title. This PR had to edit all three in step, and the test pins each one separately. Could the page set the initial status from JS so the sentence has one source?

recordReplay("lifecycle", {
state: thinking ? "thinking_started" : "thinking_ended",
});
Expand Down