Feat: Consistency in the modal window code for confirming the delete action.#4792
Feat: Consistency in the modal window code for confirming the delete action.#4792IgorA100 wants to merge 13 commits intoZoneMinder:masterfrom
Conversation
…_options_servers.php)
- Pass the "formName" argument when calling getDelConfirmModal() - Added a translation
- Removed the getDelConfirmModal() function (and its call) from report.js, options.js, reports.js, devices.js, +++controlcaps.js(?view=options&tab=control), and snapshots.js. This is because it's now a single function in skin.js, which is called when the "Delete" button is pressed, rather than during page initialization. - Slightly modified the local functions of manageDelConfirmModalBtns(). Removed the listener, so the code is executed immediately. - You can now call local functions from the global getDelConfirmModal() function (instead of just executing submitThisForm() ). Applies to report.js, reports.js, devices.js, controlcaps.js, snapshots.js - Changed the style of the "Delete" button on the Groups page - Added deletion confirmation to the Storage, Roles, and Groups pages - On the Event page, deletion permissions were broken. Deletes were allowed for those who did NOT have edit permissions, but deletions were blocked for those who did. ! - On the Report page, deletion didn't work and still doesn't work. This needs to be addressed, or maybe it's possible to delete reports only on the Reports page. (FIXED...) - On the Devices page, the "canEdit.Device" permissions check was being performed, but it should be performed like this: "canEdit.Devices" - The $action variable was not defined in \ajax\devices.php - Added translations. - Added a style for the disabled "Delete" button - Minor fixes
|
@connortechnology |
| 'ConfirmDeleteRole' => 'Are you sure you wish to delete the selected roles?', | ||
| 'ConfirmDeleteRoleTitle'=> 'Confirm Role Deletion', | ||
| 'ConfirmDeleteSnapshots'=> 'Are you sure you wish to delete the selected snapshots?', | ||
| 'ConfirmPassword' => 'Confirm Password', |
There was a problem hiding this comment.
I wonder if we ould get away with just one "Are you sure?"
There was a problem hiding this comment.
I don't know.
What are your suggestions, Isaac?
There was a problem hiding this comment.
Pull request overview
This PR centralizes and standardizes the “Delete” confirmation modal behavior across multiple classic-skin pages by moving modal-loading logic into skin.js, updating page-specific delete handlers to run on confirmation, and adding/adjusting related UI/permission behavior.
Changes:
- Introduces a global
getDelConfirmModal()inweb/skins/classic/js/skin.jsand updates multiple pages to invoke it on Delete click (instead of preloading at init). - Updates page-local
manageDelConfirmModalBtns()implementations to execute delete actions immediately after confirmation. - Adds delete confirmation support to additional Options tabs (Servers/Storage/Roles) and tweaks styling/translations.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| web/skins/classic/views/report.php | Makes report delete button non-submitting and adds data-report_id; adjusts early return/footer output. |
| web/skins/classic/views/js/snapshots.js | Switches snapshots deletion flow to use global modal loader and immediate confirm handler. |
| web/skins/classic/views/js/reports.js | Switches reports deletion flow to use global modal loader and immediate confirm handler. |
| web/skins/classic/views/js/report.js | Hooks single-report delete to modal confirmation and deletes via AJAX. |
| web/skins/classic/views/js/options.js | Routes Options delete actions (user/server/storage/role) through global modal loader. |
| web/skins/classic/views/js/groups.js | Adds modal confirmation for group deletion and updates delete-button enabling logic. |
| web/skins/classic/views/js/devices.js | Updates delete confirmation flow and fixes permission key to canEdit.Devices. |
| web/skins/classic/views/js/controlcaps.js | Updates delete confirmation flow to use global modal loader. |
| web/skins/classic/views/groups.php | Updates Groups delete button styling. |
| web/skins/classic/views/_options_storage.php | Converts Storage delete to modal-confirmed button action. |
| web/skins/classic/views/_options_servers.php | Converts Servers delete to modal-confirmed button action. |
| web/skins/classic/views/_options_roles.php | Converts Roles delete to modal-confirmed button action. |
| web/skins/classic/js/skin.js | Adds global getDelConfirmModal() implementation. |
| web/skins/classic/css/base/skin.css | Adds disabled styling for .btn-danger. |
| web/lang/ru_ru.php | Adds new delete-confirmation translations (servers/reports/storage/roles/snapshots). |
| web/lang/en_gb.php | Adds new delete-confirmation translations (servers/reports/storage/roles/snapshots). |
| web/includes/Report.php | Re-indents defaults array (formatting-only). |
| web/ajax/reports.php | Adjusts report delete permission check to use Events permissions. |
| web/ajax/devices.php | Defines $action and improves unrecognized-action error message. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ns() function is not found (skin.js)
As canEdit analysis has been added to reports in ZoneMinder@2630d55
…port deletion error occurs instead of updating a non-existent '#reportsTable' (report.js)
|
@connortechnology |
Similar to #4749