-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix(web): keep code block word wrap across virtualizer remounts #8004
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -67,7 +67,7 @@ import { LRUCache } from "../lib/lruCache"; | |||||
| import { getSyntaxHighlighterPromise } from "../lib/syntaxHighlighting"; | ||||||
| import { RenderErrorBoundary } from "./RenderErrorBoundary"; | ||||||
| import { useTheme } from "../hooks/useTheme"; | ||||||
| import { getClientSettings } from "../hooks/useSettings"; | ||||||
| import { getClientSettings, updateClientSettings, useClientSettings } from "../hooks/useSettings"; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Posted via Macroscope — UI Consistency |
||||||
| import { | ||||||
| chatMarkdownClipboardPayload, | ||||||
| serializeTableElementToCsv, | ||||||
|
|
@@ -427,14 +427,21 @@ function estimateHighlightedSize(html: string, code: string): number { | |||||
| return Math.max(html.length * 2, code.length * 3); | ||||||
| } | ||||||
|
|
||||||
| function readInitialWordWrapSetting(): boolean { | ||||||
| return getClientSettings().wordWrap; | ||||||
| // Live word-wrap preference: survives virtualizer remounts, unlike component state. | ||||||
| function useWordWrapPreference(): [boolean, (value: boolean) => void] { | ||||||
| const wordWrap = useClientSettings((settings) => settings.wordWrap); | ||||||
| return [ | ||||||
| wordWrap, | ||||||
| useCallback((value: boolean) => { | ||||||
| updateClientSettings({ wordWrap: value }); | ||||||
| }, []), | ||||||
| ]; | ||||||
| } | ||||||
|
Comment on lines
+430
to
439
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each markdown code block's wrap chip and each table's expand chip now write the user's persisted global If the goal is only to survive virtualizer remounts, consider keeping the toggles ephemeral: a module-local Posted via Macroscope — UI Consistency |
||||||
|
|
||||||
| function MarkdownTable({ children, ...props }: React.ComponentProps<"table">) { | ||||||
| const containerRef = useRef<HTMLDivElement | null>(null); | ||||||
| const tableRef = useRef<HTMLTableElement | null>(null); | ||||||
| const [expanded, setExpanded] = useState(readInitialWordWrapSetting); | ||||||
| const [expanded, setExpanded] = useWordWrapPreference(); | ||||||
| const [copied, setCopied] = useState(false); | ||||||
| const copiedTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null); | ||||||
| const expandLabel = expanded ? "Collapse table cells" : "Expand table cells"; | ||||||
|
|
@@ -461,7 +468,7 @@ function MarkdownTable({ children, ...props }: React.ComponentProps<"table">) { | |||||
| }); | ||||||
| } | ||||||
|
|
||||||
| setExpanded((value) => !value); | ||||||
| setExpanded(!expanded); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Table expand skips width lockMedium Severity Column Additional Locations (1)Reviewed by Cursor Bugbot for commit fcd943a. Configure here. |
||||||
| } | ||||||
|
|
||||||
| const handleCopy = useCallback((format: "markdown" | "csv") => { | ||||||
|
|
@@ -664,7 +671,7 @@ function MarkdownCodeBlock({ | |||||
| children: ReactNode; | ||||||
| }) { | ||||||
| const [copied, setCopied] = useState(false); | ||||||
| const [wrapped, setWrapped] = useState(readInitialWordWrapSetting); | ||||||
| const [wrapped, setWrapped] = useWordWrapPreference(); | ||||||
| const copiedTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null); | ||||||
| const wrapLabel = wrapped ? "Disable line wrap" : "Wrap lines"; | ||||||
| const copyLabel = copied ? "Copied" : "Copy code"; | ||||||
|
|
@@ -731,7 +738,7 @@ function MarkdownCodeBlock({ | |||||
| size="icon-xs" | ||||||
| className="chat-markdown-chrome-action" | ||||||
| aria-pressed={wrapped} | ||||||
| onClick={() => setWrapped((value) => !value)} | ||||||
| onClick={() => setWrapped(!wrapped)} | ||||||
| aria-label={wrapLabel} | ||||||
| /> | ||||||
| } | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -185,6 +185,11 @@ export function getClientSettings(): ClientSettings { | |
| return getClientSettingsSnapshot(); | ||
| } | ||
|
|
||
| /** Imperative word-wrap preference update that any component (hook or not) can call. */ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium
🤖 Copy this AI Prompt to have your agent fix this: |
||
| export function updateClientSettings(patch: ClientSettingsPatch): void { | ||
| persistClientSettings({ ...getClientSettingsSnapshot(), ...patch }); | ||
| } | ||
|
|
||
| /** | ||
| * Resolves once client settings have been read from disk. | ||
| * | ||
|
|
||


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.
toggleExpandedpins headermin-widthfrom measured collapsed column widths before expanding, so the table keeps its layout. Now thatexpandedcomes from shared settings, any other writer (another table, a code block's wrap chip, the Settings toggle) flips this table todata-expanded="true"without that measurement, so its columns reflow instead of holding their widths.Smallest fix: keep the table's expand state per-instance (its own
useStateseeded from the preference), or pin widths in a layout effect that measures beforedata-expandedchanges so externally driven expansion is handled too.Posted via Macroscope — UI Consistency