Confirm a quick task in a modal, not window.confirm() - #1713
Merged
Conversation
The one-click Deploy/Capture/Multi-Cast buttons in the info card asked for confirmation through window.confirm(). It works, and it looks like nothing else in FOG: the browser dialog cannot be styled, ignores the dark theme, and prefixes the page URL, so the one place the app asks before wiping a machine is the one place that reads as though the site got something wrong. renderQuickTaskActions() now emits a modal beside the buttons, the same shape assocDelModal() uses -- which is what every other "are you sure" in this app already looks like. One modal per card, not one per button: the script fills its body from the clicked button's data-confirm, so two buttons cannot drift into two wordings of the same question. The text is still built server side and still translated. A .modal is position:fixed and display:none until shown, so it contributes nothing to the flex row it is emitted into. In fog.common.js the click handler now records which button opened the modal and the request moves to fire(), called from the modal's Create. `pending` is cleared before the request, so a second click during the hide animation has nothing left to commit, and the per-button in-flight lock is unchanged. Both handlers share the click.fogQuickTask namespace so the existing .off() still clears the pair on AJAX nav. The body is set with .text(), never .html() -- data-confirm carries an admin-supplied host or group name. Measured in both themes with the real stylesheets: the buttons stay at 4.69:1, the modal's Cancel at 5.92:1 light / 11.85:1 dark, and its Create (the modal-warning fill) at 5.14:1. All pass WCAG AA. FOG_BCACHE_VER 361 -> 362, since fog.common.js changed. tests/info-card-quick-tasks.test.php grows five checks covering the modal: that it is emitted, that it is the only one, that the footer is a dismiss plus a commit, that the body is a filled-in placeholder rather than static text, and that no window.confirm() is left in the handler. Each was proven by reintroducing the defect and watching it go red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JWJMQYE2br8E7Ehr55SJp2
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
1 participant
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.
Follow-up to #1712. The one-click Deploy/Capture/Multi-Cast buttons in the info card
worked, but asked for confirmation through
window.confirm().That dialog cannot be styled, ignores the dark theme, and prefixes the page URL — so
the one place FOG asks before wiping a machine is the one place that reads as though
the site got something wrong.
What changed
renderQuickTaskActions()now emits a modal beside the buttons, the same shapeassocDelModal()uses — which is what every other "are you sure" in this app alreadylooks like. One modal per card, not one per button: the script fills its body from
the clicked button's
data-confirm, so two buttons cannot drift into two wordings ofthe same question. The text is still built server side and still translated.
A
.modalisposition: fixedanddisplay: noneuntil shown, so it contributesnothing to the flex row it is emitted into.
In
fog.common.jsthe click handler records which button opened the modal and therequest moves to
fire(), called from the modal's Create.pendingis cleared beforethe request, so a second click during the hide animation has nothing left to commit,
and the per-button in-flight lock is unchanged. Both handlers share the
click.fogQuickTasknamespace so the existing.off()still clears the pair on AJAXnav. The body is set with
.text(), never.html()—data-confirmcarries anadmin-supplied host or group name.
Contrast
Measured in a browser against the real
adminlte4.min.css+fog-default-ui.min.css,both themes:
btn-secondary)btn-outline-secondary)modal-warningfill)All pass WCAG AA for body-sized text.
FOG_BCACHE_VER361 → 362, sincefog.common.jschanged.Tests
tests/info-card-quick-tasks.test.phpgrows five checks covering the modal: that it isemitted, that it is the only one, that the footer is a dismiss plus a commit, that the
body is a filled-in placeholder rather than static text, and that no
window.confirm()is left in the handler. Each was proven by reintroducing the defect and watching it go
red, then restored.
Both PHPStan passes clean, unscoped. Full suite: 334 passed, 1 failed —
certificate-table.test.php, which fails identically on an unmodified checkout of thebase commit and is green in CI.
No route changed, so
OpenAPI::document()is untouched.🤖 Generated with Claude Code
https://claude.ai/code/session_01JWJMQYE2br8E7Ehr55SJp2