perf: windowed card rendering on large stacks + fix Tiptap mount race in Description#8140
Open
devshoa03 wants to merge 2 commits into
Open
perf: windowed card rendering on large stacks + fix Tiptap mount race in Description#8140devshoa03 wants to merge 2 commits into
devshoa03 wants to merge 2 commits into
Conversation
Stacks with many cards (observed: boards with 100+ cards per list) render every card's full component tree at once, producing thousands of DOM nodes. Any layout-invalidating interaction on such a board (e.g. opening the card detail sidebar) then has to lay out that entire subtree, which is a major contributor to card-open jank. Render only a growing prefix of `cardsByStack` (first 30, +30 as the user scrolls near the bottom of the stack, or immediately extended to the full list when a new card is added) once a stack exceeds 30 cards. Stacks at or below the threshold are unaffected. A true virtualized/recycling window (rendering an arbitrary middle slice) was intentionally avoided: vue-smooth-dnd resolves drop positions against the real DOM children of the drag container by index, so recycling would desync drop-index math from the underlying data array. A prefix window is always anchored at index 0, so the existing index-based drag-and-drop logic (payloadForCard/onDropCard) needs no changes and stays correct. Known limitation: a card cannot be dropped directly into the not-yet-rendered tail of a very large stack until that portion has been scrolled into view at least once. Signed-off-by: devhoa <devshoa@gmail.com>
Description is keyed by card.id and destroyed/remounted whenever the selected card changes. setupEditor() calls the async window.OCA.Text.createEditor(...), which can still be resolving when the user switches to another card and beforeDestroy() runs. When that happens the resolved editor is bound to an already-detached DOM element, and any later interaction with it (or with the stale `this.editor` reference) throws "the editor view is not available". Add an `isDestroyed` flag, set in beforeDestroy() and checked: - before starting editor creation, - before assigning the resolved editor to `this.editor` (destroying it immediately instead if the component was torn down while it was being created), - in every createEditor() callback (onLoaded/onUpdate/onFileInsert), and - everywhere else `this.editor` is used outside of the lifecycle methods. Also defer the createEditor() call by one tick ($nextTick) so editor setup doesn't compete with the card sidebar/modal's own open transition, and reset `this.editor` to null in destroyEditor(). Known limitation: this narrows but does not fully close the race. Vue 2 does not await an async beforeDestroy() hook before proceeding with teardown/remount, so in principle isDestroyed could still be set after a check but before the next line runs in the same microtask sequence. This is a meaningful reduction of the failure window, not a complete fix. Signed-off-by: devhoa <devshoa@gmail.com>
devshoa03
force-pushed
the
perf/stack-windowing-description-race-fix
branch
from
23d0e69 to
eb33751
Compare
6 tasks
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
2 participants
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.
Summary
On boards with many cards per list, clicking a card to open the detail
sidebar is very slow. We profiled this on Deck 1.17.4 and measured an
INP (Interaction to Next Paint) of 2,649ms on click, with ~10.7s
of cumulative forced-reflow time attributable to that single click.
The root cause is layout thrashing spread across a few independent
problems in the card-open path. This PR fixes two of them:
1.
src/components/board/Stack.vue— unbounded card rendering per stackEvery card in a stack is rendered in full at once, regardless of
column length. On a column with hundreds of cards this produces
thousands of DOM nodes (one board we tested against hit 4,942
elements in a single column), so any layout-invalidating interaction —
such as opening the card sidebar — forces the browser to lay out that
entire subtree.
We considered a standard virtual-scroll/recycling approach, but Deck
uses
vue-smooth-dndfor drag-and-drop, which resolves drop positionsagainst the real DOM children of the drag container, by index.
Recycling an arbitrary middle slice of the list would desync those
indices from the underlying card array and break drag-and-drop.
Instead, this PR renders a contiguous prefix of the card list
(the first 30 cards), growing it by 30 as the user scrolls near the
bottom of the stack (and immediately expanding to the full list when a
new card is added). Because the rendered set is always a prefix
starting at index 0, the existing index-based drag-and-drop math
(
payloadForCard/onDropCard) is untouched and stays correct.Stacks at or below the 30-card threshold render exactly as before.
Known, intentional limitation: a card cannot be dragged directly
into the not-yet-rendered tail of a very large stack until that part
of the list has been scrolled into view at least once. We consider
this an acceptable trade-off given it preserves 100% of the existing
drag-and-drop correctness, but flagging it explicitly for review.
2.
src/components/card/Description.vue— Tiptap/OCA.Textmount raceDescriptionis keyed bycard.id, so Vue destroys and remounts itevery time the selected card changes.
setupEditor()calls the asyncwindow.OCA.Text.createEditor(...); if the user switches cardsquickly, that promise can resolve after
beforeDestroy()has alreadyrun, binding a live Tiptap/ProseMirror editor to an already-detached
DOM element. That editor is then never destroyed, and interacting with
it (or with the stale
this.editorreference from the old instance)throws
"the editor view is not available".This PR adds an
isDestroyedflag (set inbeforeDestroy()) that ischecked before starting editor creation, before assigning the resolved
editor to the instance (destroying it immediately instead, if the
component was torn down in the meantime), inside every
createEditor()callback (onLoaded/onUpdate/onFileInsert),and at the other call sites that touch
this.editor. It also defersthe
createEditor()call by one$nextTick()so editor setup doesn'tcompete with the sidebar/modal's own open transition, and resets
this.editortonullafter teardown.Known, intentional limitation: this narrows but does not fully
eliminate the race. Vue 2 does not await an async
beforeDestroy()hook before proceeding, so there remains, in principle, a narrower
window in the same microtask sequence. This is a meaningful reduction
of the failure window, not a complete fix.
What's intentionally not in this PR
A third contributor to the same click-lag problem is in
src/components/cards/CardItem.vue, where a<transition-group>around the sidebar/card list causes additional forced reflow, plus a
full-list re-render on modal open. #8112 already fixes exactly this,
so it is deliberately excluded here to avoid duplicating that
open PR. We are not requesting any changes to #8112 or commenting on
it; it's referenced here purely for context on the full picture.
Measurement, honestly stated
We measured before/after INP and forced-reflow time on a production
instance with all three fixes applied together (this PR's two
fixes + the CardItem.vue fix from #8112):
We do not have isolated measurements for this PR's two fixes alone
(Stack.vue windowing + Description.vue race guard), so we are not
claiming that combined result for this PR by itself. Both changes are
independently justified on their own merits (bounding DOM size, and
removing a real error condition), but for the full measured
improvement we'd recommend maintainers consider merging this PR
together with #8112.
TODO
Stack.vueandDescription.vueare touched (nooverlap with fix: improve card open performance on boards with many cards #8112's
CardItem.vuechanges)Checklist