perf(settings): stop laying out every choice row when the filter drops rows - #2111
Conversation
…s rows Each row of the settings choice list was wrapped in animate:flip with a 0 ms duration. The flip moved nothing, but Svelte's animate still measured every row and forced a layout per row that left the list, so narrowing the filter took up to ~500 ms a keystroke with a few hundred choices. Fixes #2107
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughChoiceList removes row flip animation and no longer passes a flip duration to the drag-and-drop zone. The shared drag-and-drop options always set the flip duration to zero. A regression test checks row measurement when filtering removes choices. ChangesChoice List Reorder
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The filtering change avoids row measurements when choices are removed, while drag-and-drop still commits same- and cross-zone order changes. No actionable merge-blocking risk is evident. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The change removes redundant row animation and centralizes a zero-duration setting already used by existing callers. Filtering safeguards, reorder callbacks, and data ownership remain unchanged. No introduced or worsened security exposure was identified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. A rabbit watched the choices flow, Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 participant
Typing in Filter choices... no longer stalls when the filter drops rows. With 342 choices, a keystroke that removes rows took 390-550 ms and now takes about 40 ms.
Cause
Every row of the choice list was wrapped in
<div animate:flip={{ duration: flipDurationMs }}>, andflipDurationMsis 0, which the drag needs (the comment on it is now inbaseDndOptions). A 0 ms flip moves nothing. Svelte's animate runtime still runs for a keyed{#each}withanimate:: it measures every row, and for each row that leaves it writesposition: absoluteand reads the row's rect again. Each of those reads forces a layout of the whole list, so the cost grows with rows removed times rows shown.A CPU profile of
ca->cap->ca->cap->ca->caponmasterspent 1.5 s of 4.1 s ingetBoundingClientRectunder Svelte'sfix()andmeasure().Change
ChoiceList.sveltekeeps the row wrapper (the zone's direct child) withoutanimate:flip.baseDndOptionsno longer takesflipDurationMs; it was only passed by the choice list and is always 0.ChoiceList.crosszone.test.tsdrops its Web Animations stub, which existed foranimate:flip.Measured
Obsidian 1.13.7, isolated vault with 20,151 notes and 342 choices (22 at the root, 10 folders of 27 with nested folders). Time from the input event to the first painted frame after the list settles, two runs each:
ccacapcaptcaptuThe first letter still takes about 170-230 ms. That is mounting about 186 rows, each with its name rendered as Markdown, and is unchanged here.
Opening the tab with every folder expanded (331 rows) is unchanged (about 330-430 ms on both).
Drag still reorders: in the same vault, dragging the first root row down two places moved
Group 0from first to third, and the drop painted in 51 ms. Nothing animated before (0 ms flip) and nothing animates now, so the list looks the same.Tests
ChoiceView.test.ts: narrowing the filter from 20 rows to 10 doesn't callgetBoundingClientRect. Withanimate:flipback (and a Web Animations stub so jsdom can run it), it is called 40 times.pnpm run build-with-lint,pnpm run test(6600 passed),.agents/run-e2e(395 passed, 24 Templater-only skipped).Release / migration
None. Pre-existing (2.29.0 measures the same), not a 2.30.0 regression.
Fixes #2107
Note
Remove FLIP row animations from
ChoiceListto skip layout measurement on filteranimate:flipusage andflip-durationstate fromChoiceList, so filtered-out rows no longer triggergetBoundingClientRectmeasurements on every remaining row. Drag reordering now uses the shared zero-duration drag-zone configuration.flipDurationMsfrom thebaseDndOptionsoption type in dndReorder.ts, so all drag zones use a zero flip duration.Macroscope summarized 61f311c.
Summary by CodeRabbit