fix(web): keep code block word wrap across virtualizer remounts - #8004
fix(web): keep code block word wrap across virtualizer remounts#8004Wraient wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| return getClientSettingsSnapshot(); | ||
| } | ||
|
|
||
| /** Imperative word-wrap preference update that any component (hook or not) can call. */ |
There was a problem hiding this comment.
🟡 Medium hooks/useSettings.ts:188
updateClientSettings can overwrite a user's new wordWrap value with the older persisted value, causing the toggle to visibly revert. It reads the default snapshot before hydrateClientSettings() completes, while the in-flight hydration later replaces that snapshot; await hydration before applying the patch.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/hooks/useSettings.ts around line 188:
`updateClientSettings` can overwrite a user's new `wordWrap` value with the older persisted value, causing the toggle to visibly revert. It reads the default snapshot before `hydrateClientSettings()` completes, while the in-flight hydration later replaces that snapshot; await hydration before applying the patch.
There was a problem hiding this comment.
UI consistency review found issues with the word-wrap ownership change in apps/web/src/components/ChatMarkdown.tsx. The fix for virtualizer remounts is sound in intent, but routing per-block chrome toggles straight into the persisted global wordWrap preference changes control semantics across several surfaces and breaks the table's column-width pinning invariant. Details inline.
Posted via Macroscope — UI Consistency
| // 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 }); | ||
| }, []), | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Each markdown code block's wrap chip and each table's expand chip now write the user's persisted global wordWrap setting, so one click changes every mounted code block and table, flips Settings → Word wrap (including its "modified from default" reset affordance), and immediately re-lays-out FilePreviewPanel, which reads useClientSettings((s) => s.wordWrap) live. Previously these were per-instance chrome toggles seeded from the preference.
If the goal is only to survive virtualizer remounts, consider keeping the toggles ephemeral: a module-local useSyncExternalStore store in this file, seeded once from getClientSettings().wordWrap, survives remounts without overwriting a user-facing setting or driving unrelated surfaces. If writing through to settings is intended, that should be a deliberate product decision documented here, since the chip is not presented as a global preference control.
Posted via Macroscope — UI Consistency
| import { RenderErrorBoundary } from "./RenderErrorBoundary"; | ||
| import { useTheme } from "../hooks/useTheme"; | ||
| import { getClientSettings } from "../hooks/useSettings"; | ||
| import { getClientSettings, updateClientSettings, useClientSettings } from "../hooks/useSettings"; |
There was a problem hiding this comment.
getClientSettings no longer has a call site in this file after readInitialWordWrapSetting was removed; consider dropping it from the import.
| import { getClientSettings, updateClientSettings, useClientSettings } from "../hooks/useSettings"; | |
| import { updateClientSettings, useClientSettings } from "../hooks/useSettings"; |
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
toggleExpanded pins header min-width from measured collapsed column widths before expanding, so the table keeps its layout. Now that expanded comes from shared settings, any other writer (another table, a code block's wrap chip, the Settings toggle) flips this table to data-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 useState seeded from the preference), or pin widths in a layout effect that measures before data-expanded changes so externally driven expansion is handled too.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fcd943a. Configure here.
| } | ||
|
|
||
| setExpanded((value) => !value); | ||
| setExpanded(!expanded); |
There was a problem hiding this comment.
Table expand skips width lock
Medium Severity
Column minWidth locking still runs only inside toggleExpanded when this table is clicked. expanded now tracks shared wordWrap, so another table’s expand control or a code-block wrap toggle can set data-expanded without that measurement. Collapsed tables that expand through the shared preference can reflow differently than a directly expanded table.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit fcd943a. Configure here.
ApprovabilityVerdict: Skipped Macroscope did not run approvability analysis for this PR. Macroscope could not determine whether this PR modifies its approvability configuration, so the PR was not approved automatically. A PR that may change the rules that govern approval is never approved automatically. Not approved because:
|
1 participant
What
Code block and table wrap toggles used local state seeded from the word wrap setting. The chat list is virtualized, so scrolling unmounts/remounts messages and the toggle reset to the default after a few seconds of scrolling.
Fix
Toggles now read/write the persisted
wordWrapclient setting instead of local state, so the choice survives remounts.ChatMarkdown.test.tsx7/7 passing.Note
Low Risk
UI preference wiring only. Toggles now persist the global
wordWrapsetting, which can affect other surfaces that read that key.Overview
Code-block and table wrap toggles in chat markdown no longer live in local component state. They subscribe to and persist the
wordWrapclient setting, so scrolling a virtualized thread no longer resets wrap after remount.Toggling wrap now writes the shared client preference (same key used elsewhere, e.g. diffs) instead of a one-shot seed from
getClientSettings().Reviewed by Cursor Bugbot for commit fcd943a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix code block word wrap persistence across virtualizer remounts
Replaces the one-time
readInitialWordWrapSettingsnapshot with a newuseWordWrapPreferencehook that reads and writes the sharedsettings.wordWrapvalue viaupdateClientSettings.MarkdownCodeBlockandMarkdownTablenow bind their wrap/expand state to the live, persisted preference instead of a local snapshot, so toggles survive remounts and stay in sync.updateClientSettingsin useSettings.ts to imperatively merge and persist aClientSettingsPatch.settings.wordWrapwill observe the change immediately.📊 Macroscope summarized fcd943a. 2 files reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues