[ENG-2150] Nested pages in tldraw canvas #1308
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| // Screen-fixed breadcrumb + back bar for nested sub-pages. Real UI chrome, not | ||
| // a drawn shape: registered as the tldraw `HelperButtons` UI component (the | ||
| // slot under the page menu, verified visible in 2.4.6), composed with the | ||
| // default helper buttons rather than replacing them. | ||
| import React from "react"; | ||
| import { DefaultHelperButtons, useEditor, useValue } from "tldraw"; | ||
| import { enterPage, getLineage } from "./nestedPageNavigation"; | ||
|
|
||
| const DgSubpageBreadcrumb = () => { | ||
| const editor = useEditor(); | ||
| const chain = useValue("dg-subpage-breadcrumb", () => getLineage(editor), [ | ||
| editor, | ||
| ]); | ||
| // Root page (no dgNested.parentPageId): render nothing. | ||
| if (chain.length <= 1) return null; | ||
|
|
||
| const go = (id: string) => { | ||
| if (id !== editor.getCurrentPageId()) enterPage(editor, id); | ||
| }; | ||
| const parent = chain[chain.length - 2]; | ||
|
|
||
| return ( | ||
| <div | ||
| style={{ | ||
| pointerEvents: "all", | ||
| display: "flex", | ||
| alignItems: "center", | ||
| gap: 8, | ||
| margin: "6px 0 0 8px", | ||
| padding: "5px 10px", | ||
| background: "rgba(255,255,255,0.94)", | ||
| border: "1px solid #e3e5e9", | ||
| borderRadius: 9, | ||
| boxShadow: "0 1px 6px rgba(20,20,40,0.10)", | ||
| font: "13px var(--tl-font-sans, Inter, system-ui, sans-serif)", | ||
| backdropFilter: "blur(6px)", | ||
| maxWidth: "70vw", | ||
| overflow: "hidden", | ||
| width: "fit-content", | ||
| }} | ||
|
Comment on lines
+24
to
+40
Contributor
There was a problem hiding this comment. 🟡 New canvas breadcrumb bar introduces its own colors and hand-rolled styling instead of the host app's design system The nested sub-page breadcrumb bar is styled with a brand-new inline color/shadow palette ( Repository styling rules that this violates
The breadcrumb container ( Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| > | ||
| <button | ||
| title={`Back to ${parent.name}`} | ||
| onPointerDown={(e) => { | ||
| e.stopPropagation(); | ||
| go(parent.id); | ||
| }} | ||
|
Comment on lines
+42
to
+47
Contributor
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,140p' apps/roam/src/components/canvas/DgSubpageBreadcrumb.tsx
printf '\n--- related handlers and tests ---\n'
rg -n -C 3 "DgSubpageBreadcrumb|HelperButtons|onPointerDown|Back to" apps/roam/src --glob '*.{ts,tsx}'Repository: DiscourseGraphs/discourse-graph Length of output: 20867 🏁 Script executed: sed -n '1,220p' apps/roam/src/components/canvas/nestedPageNavigation.ts
printf '\n--- package versions and test configuration ---\n'
rg -n '"react"|react-dom|`@testing-library`|vitest|jest|playwright|cypress' package.json apps/roam/package.json pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null | head -80Repository: DiscourseGraphs/discourse-graph Length of output: 12554 🌐 Web query:
💡 Result: The native HTML button element provides built-in activation behaviors for keyboard and pointer interactions, but these mechanisms are distinct and do not inherently trigger each other. Keyboard Activation When a native button is focused, it can be activated using the Enter or Space keys [1][2]. These interactions are managed by the browser's internal logic to simulate a click, but they do not fire pointer events like pointerdown: - Enter Key: Typically triggers the button's activation behavior immediately upon the keydown event [3]. - Space Key: Typically triggers the button's activation behavior upon the keyup event [3]. This allows the user to abort the action by sliding the pointer off the button (or moving focus) before releasing the key. Pointer Activation The pointerdown event is a low-level event associated specifically with pointing devices (mouse, touch, pen) [4]. It fires when a pointer becomes active—for example, when a mouse button is pressed down or physical contact is made with a touchscreen [4]. Key Relationship Crucially, pressing the Enter or Space keys on a focused button does not fire a pointerdown event [4]. The native click event is a higher-level, device-independent abstraction [5]. Browsers synthesize a click event when a button is activated via keyboard, but this synthesis is separate from the pointer event lifecycle [5]. For developers, relying on pointerdown to trigger functionality is generally discouraged for standard UI controls because it skips the implicit "pointer cancellation" safeguards (like those built into the native click event) [6][7]. To ensure consistent behavior across all input methods (keyboard, mouse, touch), you should attach event listeners to the click event rather than pointerdown [7]. Citations:
Handle keyboard activation with The native 🤖 Prompt for AI Agents |
||
| style={{ | ||
| border: "1px solid #dfe1e6", | ||
| background: "#f7f8fa", | ||
| borderRadius: 7, | ||
| padding: "3px 9px", | ||
| cursor: "pointer", | ||
| font: "inherit", | ||
| fontWeight: 600, | ||
| color: "#3a3d42", | ||
| whiteSpace: "nowrap", | ||
| }} | ||
| > | ||
| ⬅ back | ||
| </button> | ||
| <div | ||
| style={{ | ||
| display: "flex", | ||
| alignItems: "center", | ||
| flexWrap: "nowrap", | ||
| overflow: "hidden", | ||
| }} | ||
| > | ||
| {chain.map((page, i) => { | ||
| const isLast = i === chain.length - 1; | ||
| return ( | ||
| <span | ||
| key={page.id} | ||
| style={{ display: "inline-flex", alignItems: "center", gap: 6 }} | ||
| > | ||
| {i > 0 ? <span style={{ color: "#b9bdc4" }}>▸</span> : null} | ||
| <button | ||
| onPointerDown={(e) => { | ||
| e.stopPropagation(); | ||
| if (!isLast) go(page.id); | ||
| }} | ||
| style={{ | ||
| border: "none", | ||
| background: "transparent", | ||
| padding: "2px 4px", | ||
| font: "inherit", | ||
| cursor: isLast ? "default" : "pointer", | ||
| color: isLast ? "#1d1d1f" : "#5b6bd6", | ||
| fontWeight: isLast ? 600 : 500, | ||
| maxWidth: 220, | ||
| whiteSpace: "nowrap", | ||
| overflow: "hidden", | ||
| textOverflow: "ellipsis", | ||
| }} | ||
| > | ||
| {page.name} | ||
| </button> | ||
| </span> | ||
| ); | ||
| })} | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| // The HelperButtons slot override: keep the default content (back-to-content | ||
| // etc.) and add the breadcrumb under it. | ||
| export const NestedPageHelperButtons = () => ( | ||
| <> | ||
| <DefaultHelperButtons /> | ||
| <DgSubpageBreadcrumb /> | ||
| </> | ||
| ); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Several new functions omit explicit return types required by the repository style guide
The new breadcrumb components and navigation helpers are declared without explicit return types (for example the component declaration at
apps/roam/src/components/canvas/DgSubpageBreadcrumb.tsx:9), which the repository style guide requires for all functions.Impact: Contributors lose the guaranteed, self-documenting signatures the project standardises on, making accidental signature changes easier to miss.
Affected declarations and the rule
Root
AGENTS.md→ TypeScript Guidelines: "Use explicit return types for functions".Missing return types in this PR:
DgSubpageBreadcrumb(apps/roam/src/components/canvas/DgSubpageBreadcrumb.tsx:9) and thegohelper (:17)NestedPageHelperButtons(apps/roam/src/components/canvas/DgSubpageBreadcrumb.tsx:109)enterPage(apps/roam/src/components/canvas/nestedPageNavigation.ts:24)DgSubpageUtil.readPreviewModel/component/indicator(apps/roam/src/components/canvas/DgSubpageUtil.tsx:269,:275,:707)Existing code in the same area follows the rule (e.g.
SyncModeMenuSwitchItemreturnsReactElementinapps/roam/src/components/canvas/uiOverrides.tsx:85-96).Was this helpful? React with 👍 or 👎 to provide feedback.