Accept confirm dialog input as soon as it opens

[AIC-BV]

The problem

Pressing Enter right after a backend confirm dialog opens (data-request-confirm, $.wn.confirm()) doesn't confirm it. The dialog closes and the request is silently dropped.

The bundled SweetAlert only adds the visible class 500ms after openModal(), which is longer than the 0.3s show animation. onButtonEvent only runs the callback when visible is set. Before that, a press falls through to the final else { closeModal(); }, so doneFunction is never called. For AJAX confirms, winter.alert.js then never resolves or rejects the ajaxConfirmMessage promise, and the request just hangs.

Enter counts here because the confirm button is focused when the dialog opens, so Enter is a normal button click.

Changes

• openModal() adds visible right away instead of after the 500ms timer, so Enter and clicks work during the show animation.
• onButtonEvent returns early when the dialog isn't visible, which now only happens while it's closing. A second press during the close fade used to call closeModal() a second time; now it's ignored. The modalIsVisible checks inside the handler were always true after that guard, so I removed them.
• winter-min.js is rebuilt with php artisan winter:util compile js. The other bundles came out with unrelated drift, so I left them out. Compiling from the unchanged source reproduces the committed winter-min.js exactly, so its whole diff comes from this change.

The delay didn't prevent accidental confirms in practice. The Enter that opens a dialog is handled on keydown before the dialog exists, a held Enter auto-repeats after about 500ms anyway, and the second click of a double-click lands on the overlay, not the OK button.

Testing

In the backend, calling $.wn.confirm() and pressing Enter 50ms after opening:

	callback	dialog
before	never called	closed
after	true	closed

A real Enter keypress confirms. Cancel calls the callback with false once, even with a second click during the close fade.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Prevented button clicks from triggering actions while the modal is not visible.
  • Made the modal visible immediately when it opens, rather than after a delay.

[coderabbitai]

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: cd520a8e-e9f1-41e0-87e2-71002c790de7

📥 Commits

Reviewing files that changed from the base of the PR and between 4ab9e5a and 9a3589f.

📒 Files selected for processing (2)

• modules/backend/assets/js/winter-min.js
• modules/backend/assets/vendor/sweet-alert/sweet-alert.js

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

SweetAlert now returns from its button event handler when the modal is not visible. The openModal() function adds the visible class immediately instead of after a 500 ms delay. The bundled asset contains the same behavioral changes and formatting-only edits.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 9a358

Early confirmation is enabled, and button events are rejected once closing starts. No concrete merge-blocking risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 9a358

Early button presses now reach the existing confirmation callback, while presses during closing are ignored. The review found no new request path or confirmed security issue. Risk remains low rather than minimal because authorization and all other confirmation callers were not covered.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The established affected path is backend browser confirmation and its pending AJAX request. The apparent high-fanout formatting range and unresolved widget name matches do not establish wider request reachability.

Security Findings and Attack Paths

• inferred — No introduced confirmation bypass was established: an accepted button event still supplies the callback result, and the AJAX path proceeds only on true. This does not establish the safety of uninspected server handlers.

Trust Boundaries and Controls

• observed — The visible-state check controls modal button handling; the existing confirmation callback controls resolution or rejection of the client-side AJAX confirmation promise.

Resilience and Maintainability Implications

• observed — Closing removes visible synchronously, so subsequent button events during the asynchronous fade cannot pass the new handler guard.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 118 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: allowing confirm dialog input immediately after the dialog opens.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mjauvin]

I tested this and it seems to work after two tries, not initially.

Something is not right.

[AIC-BV]

@mjauvin I think you were still running the old sweet-alert. "Fails the first time, works after that" is exactly how the unpatched code behaves.

In the old openModal(), the 500 ms setTimeout still fires after an early Enter has closed the dialog. It puts visible back on the hidden modal, so the next dialog opens with visible already set and Enter works. Only the first dialog after each page load fails. With this PR the first dialog confirms too; I checked both builds side by side.

A likely cause is the browser cache: winter-min.js is loaded with the core build as ?v=, and that doesn't change when you check out the branch. Could you try again after a hard reload (or with the cache disabled in DevTools)? To check which version is loaded, search the loaded winter-min.js for addClass(modal,'visible');},500). If that string is there, the old code is still loaded.

To test: right after a fresh page load, run this in the console and press Enter as soon as the dialog appears:

$.wn.confirm('Test?', function (ok) { console.log('callback:', ok) })

It should log callback: true.

[mjauvin]

I was not getting the right version of the assets earlier, this works as advertised.

Nice work @AIC-BV !

[AIC-BV]

[AIC-BV]

nice find