fix(docsite): prevent stale template previews on opening and reopening - #6167
fix(docsite): prevent stale template previews on opening and reopening#6167Angadslr wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR No new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
cixzhang
left a comment
There was a problem hiding this comment.
The runtime fix correctly prevents the selected template from opening with stale preview content, but this PR also adds an unrelated regression for the earlier dialog-teardown fix.
In template-preview-dialog.test.ts, please remove releases the dialog on playground route hiding and unmount and the Activity / HTMLDialogElement.close harness used only by that case. The call-count assertion locks a private teardown mechanism that this change does not modify, so future dialog lifecycle work could fail a stale-preview suite for an unrelated reason. Keep the opening, direct-selection, and pending-navigation cases that prove this fix and preserve its interaction boundary.
[Reviewed by Robohands]
|
Fixed and pushed. The dialog now renders the selected template immediately while preserving deferred previous/next navigation. Regression tests cover opening, reopening, URL changes, navigation, fullscreen behavior, and Playground teardown. |
There was a problem hiding this comment.
Thanks for fixing the ordinary stale-open path and removing the unrelated teardown coverage. One stale-preview path remains: if someone closes while previous/next navigation is pending and immediately opens another template, isPending still belongs to the superseded navigation, so the new dialog can render the old deferred preview until that navigation settles. Please make current selection win over superseded work and add a regression for pending navigation → close → open a third card before completion. If you would rather talk it through with someone, we are in Discord.
Superseded by #6167 (review) on exact head 5c3b1d7.
There was a problem hiding this comment.
Thanks for fixing the ordinary stale-open path and removing the unrelated teardown coverage. One stale-preview path remains: if someone closes while previous/next navigation is pending and immediately opens another template, isPending still belongs to the superseded navigation, so the new dialog can render the old deferred preview until that navigation settles. Please make current selection win over superseded work and add a regression for pending navigation → close → open a third card before completion. If you would rather talk it through with someone, we are in Discord.
kentonquatman
left a comment
There was a problem hiding this comment.
The fix looks great from the design side! Thanks for providing the video showing the bug, it was very helpful!
Opening a template could briefly render the previous template’s preview while the header already showed the selected template.
Render direct selections immediately, reserving deferred previews for pending previous/next navigation. Retire that preview-pending state when the dialog closes or the selection changes, so unfinished navigation cannot keep a newly selected template covered by its loading skeleton.
Regression coverage
The new pending-navigation regression fails before the latest correction and passes afterward.
Validation
Bug recording
https://youtu.be/DUU9HG8Bk54