Skip to content

fix: close What's New popup by calling modal.hide()#3356

Open
jakeefr wants to merge 1 commit into
BloopAI:mainfrom
jakeefr:fix/release-notes-close
Open

fix: close What's New popup by calling modal.hide()#3356
jakeefr wants to merge 1 commit into
BloopAI:mainfrom
jakeefr:fix/release-notes-close

Conversation

@jakeefr

@jakeefr jakeefr commented Apr 15, 2026

Copy link
Copy Markdown

Fixes #3348. Fixed the What's New popup not being closeable — the onOpenChange handler called modal.resolve() but never modal.hide(), so the dialog promise resolved but the modal stayed visible. Every other dialog in the codebase calls both.

I maintain PRISM, a post-session diagnostics tool for Claude Code — CLAUDE.md adherence analysis and session health scoring. Vibe-kanban users running multi-agent workflows are exactly the audience who'd benefit from session health diagnostics.


Note

Low Risk
Low risk UI behavior change limited to the Release Notes dialog close handler; it now hides the modal in addition to resolving its promise.

Overview
Fixes the "What�s New" / release notes dialog so closing it actually dismisses the modal: onOpenChange now calls both modal.resolve() and modal.hide() when the dialog is closed.

Reviewed by Cursor Bugbot for commit 1dc304a. Bugbot is set up for automated code reviews on this repo. Configure here.

The onOpenChange handler called modal.resolve() to settle the promise
but never called modal.hide(), so the dialog stayed visible. Every
other dialog in the codebase calls both — this one was missing hide().

Fixes BloopAI#3348

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1dc304a. Configure here.

modal.resolve();
modal.hide();
}
}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing modal.remove() call unlike all other dialogs

Medium Severity

Every other dialog in the codebase (SettingsDialog, KeyboardShortcutsDialog, WorkspacesGuideDialog, OAuthDialog) calls modal.hide(), modal.resolve(), and modal.remove() when closing. This handler omits modal.remove(), which means the component remains mounted in the React tree after dismissal, preserving stale state and preventing cleanup. The call order also differs — other dialogs call hide() before resolve(), but here resolve() is called first.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1dc304a. Configure here.

@Juanlucasbg

Copy link
Copy Markdown

Aggressive review summary — PR #3356

7-line fix to packages/web-core/src/shared/dialogs/global/ReleaseNotesDialog.tsx. Adds modal.hide() alongside modal.resolve() in the dialog's onOpenChange handler. Verdict: clean — recommend merge.

Why the fix is correct

NiceModal pattern: modal.resolve() resolves the awaiting promise, but modal.hide() is what flips modal.visible = false. Pre-PR, the dialog wires open={modal.visible} (line 43) and only calls resolve() on close — visible never goes false, so the Dialog's open prop stays true and the dialog re-renders open. The "What's New" popup not closing is the symptom.

Repo-wide convention check

DeleteConfigurationDialog.tsx:37-38  resolve() + hide()
CreateConfigurationDialog.tsx:72-77  resolve() + hide()
SettingsDialog.tsx:374-375           hide() + resolve()
PrCommentsDialog.tsx:79-80           resolve() + hide()
ResolveConflictsDialog.tsx:71-74     resolve() + hide()
ReposSettingsSection.tsx:82-83       resolve() + hide()

The PR follows the dominant resolve() → hide() ordering. ReleaseNotesDialog was the only dialog in the repo missing hide() — that is why closing it didn't close it.

Findings

  • Structural: 0 HIGH, 0 MED, 0 LOW. Convergence on the existing convention.
  • Adversarial: 0 HIGH. No double-resolve risk: modal.resolve() is idempotent in NiceModal (resolves the same promise once); subsequent onOpenChange events are guarded because modal.visible will already be false.
  • Security: N/A.
  • Conventions: PASS. fix: prefix matches repo style.

Verdict

Approve.

— Reviewed by automated single-pass review (convention-conformance triage; full 4-tool battery skipped — diff is 7 lines and exactly mirrors the repo-wide pattern).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: cant close the "What's New" popup

2 participants