Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
577 changes: 573 additions & 4 deletions tests/browser/example-board-ui.test.js

Large diffs are not rendered by default.

248 changes: 248 additions & 0 deletions tests/browser/interview-layout.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,254 @@ test("the wait keeps Jim heard and the recording withdrawable", async (t) => {
}
});

// #579's own case: a 13.6-inch MacBook Air window with the divider at its
// maximum, where the editor panel overflows by a few pixels. The expanded
// board must not scroll with it and show what it covers.
test("the expanded Example drawing stays at the panel's top when the panel could scroll", async (t) => {
if (!browser) return t.skip("playwright chromium unavailable");
const page = await browser.newPage({
viewport: { width: 1250, height: 630 },
});
try {
await page.goto(
`${base}/interview.html?problem=chargeback-pair-match&exampleDrawing=1`,
{ waitUntil: "domcontentloaded" },
);
await page.waitForFunction(
() => document.querySelector("#candidate-case-input").placeholder !== "",
);
await closeSetupCheck(page);
await page.locator("#example-board").evaluate((node) => (node.open = true));
await page.locator(".drawer-resizer").focus();
await page.keyboard.press("End");
const offset = () =>
page.evaluate(() => {
const panel = document.querySelector(".editor-panel");
const pane = document.querySelector(".example-pane");
return Math.round(
pane.getBoundingClientRect().top - panel.getBoundingClientRect().top,
);
});
// Scrolled before expanding, and wheeled over the board after.
const scrolled = await page
.locator(".editor-panel")
.evaluate(
(panel) => (panel.scrollTop = panel.scrollHeight) && panel.scrollTop,
);
assert.ok(scrolled > 0, "the panel can scroll at this size");
await page.getByRole("button", { name: "Expand drawing" }).click();
assert.ok(
Math.abs(await offset()) <= 1,
"the pane starts at the panel's top",
);
const pane = await page.locator(".example-pane").boundingBox();
await page.mouse.move(pane.x + pane.width / 2, pane.y + pane.height / 2);
await page.mouse.wheel(0, 300);
// The wheel scrolls on a later frame.
await page.evaluate(
() =>
new Promise((done) =>
requestAnimationFrame(() => requestAnimationFrame(done)),
),
);
assert.ok(Math.abs(await offset()) <= 1, "and stays there under the wheel");
Comment thread
Disesfgewu marked this conversation as resolved.
Resolved
} finally {
await page.close();
}
});

// The drawer sizes the docked drawing from its own layout, which an expanded
// board is not in: a resize while expanded must not leave a wrong height
// behind for the docked board to come back to.
test("shrinking after a resize while expanded sizes the docked drawing for the new window", async (t) => {
if (!browser) return t.skip("playwright chromium unavailable");
const page = await browser.newPage({
viewport: { width: 1280, height: 800 },
});
try {
await page.goto(
`${base}/interview.html?problem=chargeback-pair-match&exampleDrawing=1`,
{ waitUntil: "domcontentloaded" },
);
await page.waitForFunction(
() => document.querySelector("#candidate-case-input").placeholder !== "",
);
await closeSetupCheck(page);
await page.locator("#example-board").evaluate((node) => (node.open = true));
// The drawer repaints from observers, a frame or two after a change.
const settled = () =>
page.evaluate(
() =>
new Promise((done) =>
requestAnimationFrame(() => requestAnimationFrame(done)),
),
);
const surface = () =>
page
.locator(".example-surface")
.evaluate((node) => node.getBoundingClientRect().height);
await page.getByRole("button", { name: "Expand drawing" }).click();
await page.setViewportSize({ width: 1280, height: 600 });
await settled();
await page.getByRole("button", { name: "Shrink drawing" }).click();
await settled();
const back = await surface();
// What the drawer gives the drawing at this size when it paints afresh.
await page
.locator("#results-body")
.evaluate((node) => (node.hidden = !node.hidden));
await settled();
await page
.locator("#results-body")
.evaluate((node) => (node.hidden = !node.hidden));
await settled();
assert.ok(Math.abs(back - (await surface())) <= 1, `${back}px`);
Comment thread
Disesfgewu marked this conversation as resolved.
Resolved
} finally {
await page.close();
}
});

// Each of the page's own overlays over an expanded board, on the real page:
// the pane sits under them, so whichever one opens is what the candidate sees.
test("an expanded Example drawing stays under the setup check, the ending wait and the report", async (t) => {
if (!browser) return t.skip("playwright chromium unavailable");
const page = await browser.newPage({
viewport: { width: 1280, height: 800 },
});
try {
await page.goto(
`${base}/interview.html?problem=chargeback-pair-match&exampleDrawing=1`,
{ waitUntil: "domcontentloaded" },
);
await page.waitForFunction(
() => document.querySelector("#candidate-case-input").placeholder !== "",
);
await closeSetupCheck(page);
await page.locator("#example-board").evaluate((node) => (node.open = true));
await page.getByRole("button", { name: "Expand drawing" }).click();
for (const overlay of [
"#audio-check",
"#ending-overlay",
"#report-modal",
]) {
const covered = await page.evaluate((overlay) => {
const node = document.querySelector(overlay);
node.hidden = false;
const pane = document
.querySelector(".example-pane")
.getBoundingClientRect();
const hit = document.elementFromPoint(
pane.x + pane.width / 2,
pane.y + pane.height / 2,
);
node.hidden = true;
return node.contains(hit);
}, overlay);
assert.ok(covered, overlay);
}
} finally {
await page.close();
}
});

// On the real page, wide and narrow. The full stage is hidden, so no covered
// control is left in the Tab order, until audio is blocked and Resume audio has
// to be reachable, and while it holds the focus Resume audio gives it; the
// compact card keeps its captions on top of the board.
test("an expanded Example drawing hides Jim's full stage, but not the compact card or Resume audio", async (t) => {
if (!browser) return t.skip("playwright chromium unavailable");
for (const [width, height, compact] of [
[1440, 900, false],
[900, 800, true],
]) {
const size = `${width}x${height}`;
const page = await browser.newPage({ viewport: { width, height } });
try {
await page.goto(
`${base}/interview.html?problem=chargeback-pair-match&exampleDrawing=1`,
{ waitUntil: "domcontentloaded" },
);
await page.waitForFunction(
() =>
document.querySelector("#candidate-case-input").placeholder !== "",
);
await closeSetupCheck(page);
await page
.locator("#example-board")
.evaluate((node) => (node.open = true));
assert.equal(
await page
.locator("#jim-stage")
.evaluate((node) => node.classList.contains("compact")),
compact,
size,
);
const visibility = () =>
page.evaluate(
() =>
getComputedStyle(document.querySelector("#jim-stage")).visibility,
);
// Whether the element at the centre of `selector` is inside it.
const onTop = (selector) =>
page.evaluate((selector) => {
const node = document.querySelector(selector);
const box = node.getBoundingClientRect();
return node.contains(
document.elementFromPoint(
box.x + box.width / 2,
box.y + box.height / 2,
),
);
}, selector);
assert.equal(await visibility(), "visible", size);
await page.getByRole("button", { name: "Expand drawing" }).click();
if (compact) {
assert.equal(await visibility(), "visible", size);
assert.ok(await onTop("#captions-bar"), size);
} else {
assert.equal(await visibility(), "hidden", size);
}
await page.evaluate(
() => (document.querySelector("#audio-playback-notice").hidden = false),
);
assert.ok(await onTop("#resume-audio"), size);
// What audio-playback.js does once Resume audio plays, done here by
// script: focus moves to the stage, then the notice goes. The stage
// keeps focus and stays shown, so its "Audio restored." is not hidden.
await page.locator("#resume-audio").focus();
await page.evaluate(() => {
document.querySelector("#jim-stage").focus();
document.querySelector("#audio-playback-notice").hidden = true;
});
assert.equal(
await page.evaluate(() => document.activeElement.id),
"jim-stage",
size,
);
assert.equal(await visibility(), "visible", size);
assert.ok(
await onTop(compact ? "#captions-bar" : "#jim-stage .hide-avatar"),
size,
);
assert.ok(
await page.evaluate(
() =>
getComputedStyle(document.querySelector("#audio-playback-status"))
.visibility === "visible",
),
size,
);
// Once focus moves on, the full stage goes again.
await page.locator("#yield-turn").focus();
assert.equal(await visibility(), compact ? "visible" : "hidden", size);
await page.getByRole("button", { name: "Shrink drawing" }).click();
assert.equal(await visibility(), "visible", size);
} finally {
await page.close();
}
}
});

// On a phone the compact stage runs along the bottom edge above the wait, so a
// long closing line must not grow it over the wait's own buttons.
test("on a phone Jim's closing lines leave the wait's buttons reachable", async (t) => {
Expand Down
26 changes: 26 additions & 0 deletions tests/browser/run-shortcut.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,32 @@ test("the shortcut does nothing while the media preflight is still up", async (t
}
});

test("the shortcut runs nothing while Example drawing is expanded, and runs again once it shrinks", async (t) => {
const ctx = await interviewPage(t, { params: { exampleDrawing: "1" } });
if (!ctx) return;
const { page, errors } = ctx;
try {
await runButtonReady(page);
await page.locator("#example-board").evaluate((node) => (node.open = true));
await page.getByRole("button", { name: "Expand drawing" }).click();
await page.keyboard.press("ControlOrMeta+Enter");
// Taken, so it presses nothing else, and runs nothing while the board
// covers the results.
assert.equal(await runs(page), 0);
assert.equal(await enterDefaultPrevented(page), true);
await page.getByRole("button", { name: "Shrink drawing" }).click();
await page.keyboard.press("ControlOrMeta+Enter");
assert.equal(await runs(page), 1);
assert.deepEqual(
errors,
[],
`the shortcut threw in the browser: ${errors[0]}`,
);
} finally {
await page.close();
}
});

test("the shortcut does nothing once the interview has ended", async (t) => {
const ctx = await interviewPage(t, { clock: true });
if (!ctx) return;
Expand Down
2 changes: 1 addition & 1 deletion web/board-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,5 +455,5 @@ export function mountBoardText(
canvas.addEventListener("keydown", onDraftKeydown);
const observer = new ResizeObserver(refresh);
observer.observe(canvas);
return { refresh, cancel: close };
return { refresh, cancel: close, busy: () => gesture !== null };
}
64 changes: 63 additions & 1 deletion web/example-board.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ const TOOL_ICONS = {
eraser: '<path d="m4 14 9-10 7 7-9 10H7Z M8 10l7 7 M11 21h10"/>',
undo: '<path d="m9 5-5 5 5 5 M4 10h10a6 6 0 0 1 0 12"/>',
clear: '<path d="M3 6h18 M9 6V3h6v3 M5 6l1 15h12l1-15 M10 10v7 M14 10v7"/>',
expand: '<path d="M4 9V4h5 M15 4h5v5 M20 15v5h-5 M9 20H4v-5"/>',
shrink: '<path d="M9 4v5H4 M20 9h-5V4 M15 20v-5h5 M4 15h5v5"/>',
};

function setToolIcon(button, name) {
Expand Down Expand Up @@ -44,7 +46,13 @@ export function mountExampleBoard(
const model = createBoard();
const pointers = createPointerGate();
const tools = root.querySelector("[data-tools]");
const status = root.querySelector(":scope > [role=status]");
// The interview wraps the board in `.example-pane` so it can lift out of
// the drawer; the task page keeps it directly under the details element.
const status = root.querySelector(
":scope > [role=status], :scope > .example-pane > [role=status]",
Comment thread
Disesfgewu marked this conversation as resolved.
Resolved
);
const expand = root.querySelector("[data-action=expand]");
const pane = root.querySelector(":scope > .example-pane");
let tool = "pen";
let color = PEN_COLORS[0].color;
let drag = null;
Expand Down Expand Up @@ -269,6 +277,60 @@ export function mountExampleBoard(
});
canvas.addEventListener("pointercancel", cancel);
canvas.addEventListener("lostpointercapture", cancel);
// Expanding moves no element: the pane is restyled in place, so the canvas,
// its strokes and anything waiting to reach Jim stay exactly as they were.
// Only the drawn size changes, and `boardPoint` already scales by it.
function setExpanded(on) {
root.toggleAttribute("data-expanded", on);
Comment thread
Disesfgewu marked this conversation as resolved.
Resolved
setToolIcon(expand, on ? "shrink" : "expand");
expand.setAttribute("aria-label", on ? "Shrink drawing" : "Expand drawing");
expand.title = expand.getAttribute("aria-label");
const panel = root.closest(".editor-panel");
// The panel can scroll a little on a short screen, and the pane would
// scroll with it; the stylesheet stops it scrolling while expanded.
if (on && panel) panel.scrollTop = 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Expanding sets the panel's scrollTop to 0, and shrinking never puts it back. On a short window, where the candidate had to scroll the panel down to reach Expand at all (the 1280x500 case in the description), Shrink jumps back to the top and leaves the focused button out of view. Keep the old value on expand and restore it on shrink (with let scrollBack = null; next to drag), and extend the scrolled-panel test in interview-layout.test.js to check the round trip.

Suggested change
if (on && panel) panel.scrollTop = 0;
if (on && panel) {
scrollBack = panel.scrollTop;
panel.scrollTop = 0;
} else if (panel && scrollBack !== null) {
panel.scrollTop = scrollBack;
scrollBack = null;
}

// What the pane covers leaves the tab order with it, or Shift+Tab walks
// into a code editor nobody can see. The sidebar is outside the panel.
for (let node = pane; panel && node !== panel; node = node.parentElement)
for (const sibling of node.parentElement.children)
if (sibling !== node) sibling.inert = on;
Comment on lines +294 to +296

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shrinking sets inert = false on every sibling, and so does the setExpanded(false) call at mount, whatever the sibling was before. Nothing under the panel is made inert by other code today (holdWorkspaceBehindDialogs writes the panel itself), but the first thing that does will be undone by a shrink. Record the nodes this call changed and restore only those (with let covered = []; beside the other state):

Suggested change
for (let node = pane; panel && node !== panel; node = node.parentElement)
for (const sibling of node.parentElement.children)
if (sibling !== node) sibling.inert = on;
if (on) {
covered = [];
for (let node = pane; panel && node !== panel; node = node.parentElement)
for (const sibling of node.parentElement.children)
if (sibling !== node && !sibling.inert) {
sibling.inert = true;
covered.push(sibling);
}
} else {
for (const sibling of covered) sibling.inert = false;
covered = [];
}

}
if (expand) {
setExpanded(false);
// Nothing on the page closes it while expanded, but whatever does must
// not leave the editor inert under a board that is gone.
root.addEventListener("toggle", () => {
if (!root.open) setExpanded(false);
});
expand.addEventListener("click", () => {
if (!drag && !textDraft.busy())
setExpanded(!root.hasAttribute("data-expanded"));
});
// Not a modal dialog: the sidebar's turn and mute controls stay usable
// while the board is large, so Escape is handled here rather than by
// `showModal`. A text draft claims Escape first and stops it. An ink
// stroke takes no focus, so after a sidebar button the key still arrives
// there. A field or a dialog keeps Escape for itself, and so does an
// input method, a modifier chord or a held key.
document.addEventListener("keydown", (event) => {
if (
event.key !== "Escape" ||
event.isComposing ||
event.keyCode === 229 ||
event.repeat ||
event.altKey ||
event.ctrlKey ||
event.metaKey ||
event.shiftKey ||
drag ||
textDraft.busy() ||
!root.hasAttribute("data-expanded") ||
event.target.closest?.("input, textarea, select, [role=dialog]")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This listener is on document and never checks event.defaultPrevented. So a later handler that consumes Escape with preventDefault() alone also shrinks the board. The text draft only escapes this because it also calls stopPropagation(). A native <dialog> has no role attribute, so [role=dialog] misses it too. Both are cheap to close off:

Suggested change
event.target.closest?.("input, textarea, select, [role=dialog]")
event.defaultPrevented ||
event.target.closest?.("input, textarea, select, dialog, [role=dialog]")

)
return;
setExpanded(false);
});
}
root.addEventListener("toggle", () => {
if (!root.open) cancelTurn?.();
if (!root.open && drag) {
Expand Down
6 changes: 6 additions & 0 deletions web/interview-drawer.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ export function mountInterviewDrawer(panel) {
});
for (const details of drawer.querySelectorAll("details"))
details.addEventListener("toggle", paint);
// A shrinking board comes back to a drawer that may have been resized while
// it was out, so its docked height is measured again.
new MutationObserver(paint).observe(example, {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

While the board is expanded, this observer (and the panel's ResizeObserver) still runs the example.open branch of paint. That branch measures .example-surface inside the absolutely positioned pane and writes a docked height based on it into surface.style.height. The board still looks right only because flex: 1 in the expanded rule overrides the inline height, and the next shrink measures again. Skipping that branch while example has data-expanded would stop the docked height being computed from the wrong box.

attributes: true,
attributeFilter: ["data-expanded"],
});
new MutationObserver(paint).observe(results, {
attributes: true,
attributeFilter: ["hidden"],
Expand Down
Loading
Loading