Conversation
A lightweight global draft-paper canvas (pen/eraser/clear) overlaid on the whole page, reachable from the top-right toolbar right after the language switcher. Conceived as a digital scratchpad for the cognitive test: it does not touch scoring, the result payload, or sharing. - Single global draft layer; discarded on exit (no persistence) - Explicit pen toggle (default off) keeps answer options clickable - Toolbar at z-50 sits above the z-30 canvas, stays clickable while drawing - During testing only search/language/theme hide; whiteboard toolbar stays
审阅者指南此 PR 添加了一个全局的、非持久化的白板图层(canvas),其工具栏挂载在顶部导航中,并调整了测试时的导航隐藏行为,以在隐藏搜索/语言/主题控件时保持白板工具可见。 GlobalWhiteboard 画笔切换与绘制交互的时序图sequenceDiagram
actor User
participant GlobalWhiteboard
participant Canvas
User->>GlobalWhiteboard: click pen button (toggle("pen"))
GlobalWhiteboard->>GlobalWhiteboard: setTool("pen")
GlobalWhiteboard->>Canvas: enable pointerEvents auto
User->>Canvas: pointerDown (onPointerDown)
Canvas->>GlobalWhiteboard: onPointerDown
GlobalWhiteboard->>GlobalWhiteboard: getNorm
GlobalWhiteboard->>GlobalWhiteboard: push Stroke to strokesRef
GlobalWhiteboard->>Canvas: draw
User->>Canvas: pointerMove (onPointerMove)
Canvas->>GlobalWhiteboard: onPointerMove
GlobalWhiteboard->>GlobalWhiteboard: append Point to currentStrokeRef
GlobalWhiteboard->>Canvas: draw
User->>GlobalWhiteboard: click pen button again (toggle("pen"))
GlobalWhiteboard->>GlobalWhiteboard: clearAll
GlobalWhiteboard->>Canvas: draw
GlobalWhiteboard->>GlobalWhiteboard: setTool("none")
GlobalWhiteboard->>Canvas: disable pointerEvents (none)
文件级变更
提示与命令与 Sourcery 交互
自定义你的体验访问你的 仪表盘 以:
获取帮助Original review guide in EnglishReviewer's GuideThis PR adds a global, non-persistent whiteboard layer (canvas) with a toolbar mounted in the top nav, and adjusts testing-time nav hiding to keep the whiteboard tools visible while search/language/theme are hidden. Sequence diagram for GlobalWhiteboard pen toggle and drawing interactionsequenceDiagram
actor User
participant GlobalWhiteboard
participant Canvas
User->>GlobalWhiteboard: click pen button (toggle("pen"))
GlobalWhiteboard->>GlobalWhiteboard: setTool("pen")
GlobalWhiteboard->>Canvas: enable pointerEvents auto
User->>Canvas: pointerDown (onPointerDown)
Canvas->>GlobalWhiteboard: onPointerDown
GlobalWhiteboard->>GlobalWhiteboard: getNorm
GlobalWhiteboard->>GlobalWhiteboard: push Stroke to strokesRef
GlobalWhiteboard->>Canvas: draw
User->>Canvas: pointerMove (onPointerMove)
Canvas->>GlobalWhiteboard: onPointerMove
GlobalWhiteboard->>GlobalWhiteboard: append Point to currentStrokeRef
GlobalWhiteboard->>Canvas: draw
User->>GlobalWhiteboard: click pen button again (toggle("pen"))
GlobalWhiteboard->>GlobalWhiteboard: clearAll
GlobalWhiteboard->>Canvas: draw
GlobalWhiteboard->>GlobalWhiteboard: setTool("none")
GlobalWhiteboard->>Canvas: disable pointerEvents (none)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我发现了两个问题,并留下了一些整体性的反馈:
- 在 GlobalWhiteboard 中,
getNorm、onPointerDown和onPointerMove使用了React.PointerEvent,但没有导入 React 本身;可以选择从 "react" 中导入type React,或者改用 DOM 的PointerEvent类型,以避免类型错误。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In GlobalWhiteboard, `getNorm`, `onPointerDown`, and `onPointerMove` use `React.PointerEvent` but React itself isn’t imported; either import `type React` from "react" or switch these to the DOM `PointerEvent` type to avoid type errors.
## Individual Comments
### Comment 1
<location path="src/components/GlobalWhiteboard.tsx" line_range="184-193" />
<code_context>
+ const exitCanvas = () => {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The "再次点击退出画布" affordance is a non-button span, which limits keyboard accessibility.
The exit hint is implemented as a clickable `<span>` without `role` or `tabIndex`, so it’s not operable via keyboard. Since it triggers an action, it should be a `<button>` or an element with an appropriate ARIA role, focusability, and keyboard activation handling, while preserving the current styling.
Suggested implementation:
```typescript
const exitCanvas = () => {
const t = toolRef.current;
if (t !== "none") toggle(t);
};
const active = tool !== "none";
const btnBase =
"flex h-7 w-7 cursor-pointer items-center justify-center rounded-md transition-colors";
const btnIdle = "text-muted-foreground hover:text-foreground hover:bg-accent";
const btnActive = "bg-primary text-primary-foreground shadow-xs";
```
```typescript
<button
type="button"
className="text-xs text-muted-foreground cursor-pointer"
onClick={exitCanvas}
>
再次点击退出画布
</button>
```
I can’t see the exact markup where the "再次点击退出画布" hint is rendered, so you’ll need to adapt the SEARCH block to match your current `<span>` exactly (including its `className` and any other props).
When doing so:
1. Make sure the new element is a `<button>` with `type="button"` and `onClick={exitCanvas}`.
2. Preserve all existing styling-related classes on the button so the UI remains unchanged.
3. If the span had conditional rendering or was nested in other elements, only replace the span’s opening/closing tags and attributes; keep the surrounding layout intact.
</issue_to_address>
### Comment 2
<location path="src/components/GlobalWhiteboard.tsx" line_range="43" />
<code_context>
+ const strokesRef = useRef<Stroke[]>([]);
+ const drawingRef = useRef(false);
+ const currentStrokeRef = useRef<Stroke | null>(null);
+ const [tool, setTool] = useState<Tool>("none");
+ const toolRef = useRef<Tool>("none");
+ useEffect(() => {
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the whiteboard by using a single `tool` state, shared stroke/eraser helpers, and one exit hint element to reduce duplicated logic and mental overhead.
The dual `tool` state + `toolRef` and some duplicated logic do add unnecessary complexity. You can simplify while preserving all current behavior (including “click same tool clears + exits”).
### 1. Remove dual tool tracking (`tool` + `toolRef`)
You don’t need `toolRef` + syncing `useEffect` if you make your handlers depend on `tool` via `useCallback`. That keeps a single source of truth and removes the sync effect.
```ts
const [tool, setTool] = useState<Tool>("none");
const onPointerDown = useCallback(
(e: React.PointerEvent) => {
if (tool === "none") return;
e.preventDefault();
try {
(e.target as Element).setPointerCapture?.(e.pointerId);
} catch {}
drawingRef.current = true;
const p = getNorm(e);
if (tool === "eraser") {
eraseAt(p);
return;
}
const stroke: Stroke = { points: [p], color: PEN_COLOR, size: PEN_SIZE };
currentStrokeRef.current = stroke;
strokesRef.current.push(stroke);
draw();
},
[tool, draw],
);
const onPointerMove = useCallback(
(e: React.PointerEvent) => {
if (!drawingRef.current) return;
const p = getNorm(e);
if (tool === "eraser") {
eraseAt(p);
return;
}
currentStrokeRef.current?.points.push(p);
draw();
},
[tool, draw],
);
const endStroke = useCallback(() => {
drawingRef.current = false;
currentStrokeRef.current = null;
}, []);
```
Then `toggle`/`exitCanvas` can use `tool` directly:
```ts
const toggle = useCallback(
(t: Exclude<Tool, "none">) => {
if (tool === t) {
// 再次点击同一工具 = 退出画布,清空已画内容
strokesRef.current = [];
draw();
setTool("none");
} else {
setTool(t);
}
},
[tool, draw],
);
const exitCanvas = useCallback(() => {
if (tool !== "none") toggle(tool as Exclude<Tool, "none">);
}, [tool, toggle]);
```
This drops `toolRef` and the sync `useEffect` entirely.
### 2. Extract small stroke/eraser helpers to reduce imperative duplication
You can encapsulate the stroke lifecycle so `onPointerDown`/`onPointerMove` only dispatch based on the current tool, keeping side-effects in one place.
```ts
const startStroke = (p: Point) => {
const stroke: Stroke = { points: [p], color: PEN_COLOR, size: PEN_SIZE };
currentStrokeRef.current = stroke;
strokesRef.current.push(stroke);
draw();
};
const continueStroke = (p: Point) => {
if (!currentStrokeRef.current) return;
currentStrokeRef.current.points.push(p);
draw();
};
const eraseAt = (p: Point) => {
const w = window.innerWidth;
const h = window.innerHeight;
const strokes = strokesRef.current;
const r2 = ERASER_RADIUS * ERASER_RADIUS;
let changed = false;
for (let i = strokes.length - 1; i >= 0; i--) {
const hit = strokes[i].points.some((pt) => {
const dx = (pt.x - p.x) * w;
const dy = (pt.y - p.y) * h;
return dx * dx + dy * dy <= r2;
});
if (hit) {
strokes.splice(i, 1);
changed = true;
}
}
if (changed) draw();
};
```
Handlers then become thin:
```ts
const onPointerDown = useCallback(
(e: React.PointerEvent) => {
if (tool === "none") return;
e.preventDefault();
try {
(e.target as Element).setPointerCapture?.(e.pointerId);
} catch {}
drawingRef.current = true;
const p = getNorm(e);
if (tool === "eraser") {
eraseAt(p);
} else {
startStroke(p);
}
},
[tool, draw],
);
const onPointerMove = useCallback(
(e: React.PointerEvent) => {
if (!drawingRef.current) return;
const p = getNorm(e);
if (tool === "eraser") {
eraseAt(p);
} else {
continueStroke(p);
}
},
[tool, draw],
);
```
### 3. Deduplicate the exit hint rendering
The “再次点击退出画布” span is duplicated for pen and eraser with identical behavior. You can render it once when any tool is active without changing semantics:
```tsx
const active = tool !== "none";
return (
<div className="relative z-50 flex items-center gap-0.5 rounded-lg border bg-background/80 p-0.5 backdrop-blur-sm">
<button
type="button"
aria-label="Pen"
title="画笔(划线)"
aria-pressed={tool === "pen"}
onClick={() => toggle("pen")}
className={`${btnBase} ${tool === "pen" ? btnActive : btnIdle}`}
>
<Pen className="h-4 w-4" />
</button>
<button
type="button"
aria-label="Eraser"
title="橡皮(擦除整条线)"
aria-pressed={tool === "eraser"}
onClick={() => toggle("eraser")}
className={`${btnBase} ${tool === "eraser" ? btnActive : btnIdle}`}
>
<Eraser className="h-4 w-4" />
</button>
{active && (
<span
onClick={exitCanvas}
className="cursor-pointer select-none whitespace-nowrap pl-0.5 pr-1.5 text-xs text-foreground"
>
再次点击退出画布
</span>
)}
<button
type="button"
aria-label="Clear all"
title="清空全部草稿"
onClick={clearAll}
className={`${btnBase} ${btnIdle}`}
>
<Trash2 className="h-4 w-4" />
</button>
</div>
);
```
These small refactors keep the feature set intact but reduce mental overhead by eliminating dual tool sources, centralizing stroke logic, and removing duplicated UI.
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据这些反馈改进后续的评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- In GlobalWhiteboard,
getNorm,onPointerDown, andonPointerMoveuseReact.PointerEventbut React itself isn’t imported; either importtype Reactfrom "react" or switch these to the DOMPointerEventtype to avoid type errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In GlobalWhiteboard, `getNorm`, `onPointerDown`, and `onPointerMove` use `React.PointerEvent` but React itself isn’t imported; either import `type React` from "react" or switch these to the DOM `PointerEvent` type to avoid type errors.
## Individual Comments
### Comment 1
<location path="src/components/GlobalWhiteboard.tsx" line_range="184-193" />
<code_context>
+ const exitCanvas = () => {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The "再次点击退出画布" affordance is a non-button span, which limits keyboard accessibility.
The exit hint is implemented as a clickable `<span>` without `role` or `tabIndex`, so it’s not operable via keyboard. Since it triggers an action, it should be a `<button>` or an element with an appropriate ARIA role, focusability, and keyboard activation handling, while preserving the current styling.
Suggested implementation:
```typescript
const exitCanvas = () => {
const t = toolRef.current;
if (t !== "none") toggle(t);
};
const active = tool !== "none";
const btnBase =
"flex h-7 w-7 cursor-pointer items-center justify-center rounded-md transition-colors";
const btnIdle = "text-muted-foreground hover:text-foreground hover:bg-accent";
const btnActive = "bg-primary text-primary-foreground shadow-xs";
```
```typescript
<button
type="button"
className="text-xs text-muted-foreground cursor-pointer"
onClick={exitCanvas}
>
再次点击退出画布
</button>
```
I can’t see the exact markup where the "再次点击退出画布" hint is rendered, so you’ll need to adapt the SEARCH block to match your current `<span>` exactly (including its `className` and any other props).
When doing so:
1. Make sure the new element is a `<button>` with `type="button"` and `onClick={exitCanvas}`.
2. Preserve all existing styling-related classes on the button so the UI remains unchanged.
3. If the span had conditional rendering or was nested in other elements, only replace the span’s opening/closing tags and attributes; keep the surrounding layout intact.
</issue_to_address>
### Comment 2
<location path="src/components/GlobalWhiteboard.tsx" line_range="43" />
<code_context>
+ const strokesRef = useRef<Stroke[]>([]);
+ const drawingRef = useRef(false);
+ const currentStrokeRef = useRef<Stroke | null>(null);
+ const [tool, setTool] = useState<Tool>("none");
+ const toolRef = useRef<Tool>("none");
+ useEffect(() => {
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the whiteboard by using a single `tool` state, shared stroke/eraser helpers, and one exit hint element to reduce duplicated logic and mental overhead.
The dual `tool` state + `toolRef` and some duplicated logic do add unnecessary complexity. You can simplify while preserving all current behavior (including “click same tool clears + exits”).
### 1. Remove dual tool tracking (`tool` + `toolRef`)
You don’t need `toolRef` + syncing `useEffect` if you make your handlers depend on `tool` via `useCallback`. That keeps a single source of truth and removes the sync effect.
```ts
const [tool, setTool] = useState<Tool>("none");
const onPointerDown = useCallback(
(e: React.PointerEvent) => {
if (tool === "none") return;
e.preventDefault();
try {
(e.target as Element).setPointerCapture?.(e.pointerId);
} catch {}
drawingRef.current = true;
const p = getNorm(e);
if (tool === "eraser") {
eraseAt(p);
return;
}
const stroke: Stroke = { points: [p], color: PEN_COLOR, size: PEN_SIZE };
currentStrokeRef.current = stroke;
strokesRef.current.push(stroke);
draw();
},
[tool, draw],
);
const onPointerMove = useCallback(
(e: React.PointerEvent) => {
if (!drawingRef.current) return;
const p = getNorm(e);
if (tool === "eraser") {
eraseAt(p);
return;
}
currentStrokeRef.current?.points.push(p);
draw();
},
[tool, draw],
);
const endStroke = useCallback(() => {
drawingRef.current = false;
currentStrokeRef.current = null;
}, []);
```
Then `toggle`/`exitCanvas` can use `tool` directly:
```ts
const toggle = useCallback(
(t: Exclude<Tool, "none">) => {
if (tool === t) {
// 再次点击同一工具 = 退出画布,清空已画内容
strokesRef.current = [];
draw();
setTool("none");
} else {
setTool(t);
}
},
[tool, draw],
);
const exitCanvas = useCallback(() => {
if (tool !== "none") toggle(tool as Exclude<Tool, "none">);
}, [tool, toggle]);
```
This drops `toolRef` and the sync `useEffect` entirely.
### 2. Extract small stroke/eraser helpers to reduce imperative duplication
You can encapsulate the stroke lifecycle so `onPointerDown`/`onPointerMove` only dispatch based on the current tool, keeping side-effects in one place.
```ts
const startStroke = (p: Point) => {
const stroke: Stroke = { points: [p], color: PEN_COLOR, size: PEN_SIZE };
currentStrokeRef.current = stroke;
strokesRef.current.push(stroke);
draw();
};
const continueStroke = (p: Point) => {
if (!currentStrokeRef.current) return;
currentStrokeRef.current.points.push(p);
draw();
};
const eraseAt = (p: Point) => {
const w = window.innerWidth;
const h = window.innerHeight;
const strokes = strokesRef.current;
const r2 = ERASER_RADIUS * ERASER_RADIUS;
let changed = false;
for (let i = strokes.length - 1; i >= 0; i--) {
const hit = strokes[i].points.some((pt) => {
const dx = (pt.x - p.x) * w;
const dy = (pt.y - p.y) * h;
return dx * dx + dy * dy <= r2;
});
if (hit) {
strokes.splice(i, 1);
changed = true;
}
}
if (changed) draw();
};
```
Handlers then become thin:
```ts
const onPointerDown = useCallback(
(e: React.PointerEvent) => {
if (tool === "none") return;
e.preventDefault();
try {
(e.target as Element).setPointerCapture?.(e.pointerId);
} catch {}
drawingRef.current = true;
const p = getNorm(e);
if (tool === "eraser") {
eraseAt(p);
} else {
startStroke(p);
}
},
[tool, draw],
);
const onPointerMove = useCallback(
(e: React.PointerEvent) => {
if (!drawingRef.current) return;
const p = getNorm(e);
if (tool === "eraser") {
eraseAt(p);
} else {
continueStroke(p);
}
},
[tool, draw],
);
```
### 3. Deduplicate the exit hint rendering
The “再次点击退出画布” span is duplicated for pen and eraser with identical behavior. You can render it once when any tool is active without changing semantics:
```tsx
const active = tool !== "none";
return (
<div className="relative z-50 flex items-center gap-0.5 rounded-lg border bg-background/80 p-0.5 backdrop-blur-sm">
<button
type="button"
aria-label="Pen"
title="画笔(划线)"
aria-pressed={tool === "pen"}
onClick={() => toggle("pen")}
className={`${btnBase} ${tool === "pen" ? btnActive : btnIdle}`}
>
<Pen className="h-4 w-4" />
</button>
<button
type="button"
aria-label="Eraser"
title="橡皮(擦除整条线)"
aria-pressed={tool === "eraser"}
onClick={() => toggle("eraser")}
className={`${btnBase} ${tool === "eraser" ? btnActive : btnIdle}`}
>
<Eraser className="h-4 w-4" />
</button>
{active && (
<span
onClick={exitCanvas}
className="cursor-pointer select-none whitespace-nowrap pl-0.5 pr-1.5 text-xs text-foreground"
>
再次点击退出画布
</span>
)}
<button
type="button"
aria-label="Clear all"
title="清空全部草稿"
onClick={clearAll}
className={`${btnBase} ${btnIdle}`}
>
<Trash2 className="h-4 w-4" />
</button>
</div>
);
```
These small refactors keep the feature set intact but reduce mental overhead by eliminating dual tool sources, centralizing stroke logic, and removing duplicated UI.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
新增草稿纸功能
为 Cortex 增加一层全站级的「数字草稿纸」白板,方便用户在答题或浏览页面时划线、演算草稿。
功能
设计定位
定位为「考试草稿纸」:对所有受试者一视同仁,不影响 IRT 计分、结果存储与分享链路。草稿为纯内存、结束即弃,对现有逻辑零侵入。
变更文件
src/components/GlobalWhiteboard.tsxsrc/app/[locale]/layout.tsx:工具条挂载到顶栏(语言切换之后)src/app/globals.css:测试中仅隐藏.test-hideableSummary by Sourcery
在整个网站中添加一个全局白板图层,提供类似考试环境的数字草稿板。
New Features:
Enhancements:
Chores:
globals.css中进行轻微的 CSS 格式调整和动画样式统一。Original summary in English
Summary by Sourcery
Add a global whiteboard layer that provides an exam-style digital scratchpad across the site.
New Features:
Enhancements:
Chores: