Fix editor placement reset, delete controls and theme contrast

[alcatron]

Opening properties or a right-click menu while placing a node/link can leave the editor in crosshair mode, and delete confirmations use unclear wording. Reset placement consistently, restore the toolbar instructions, make Delete Node/Delete Link controls visibly red, and clarify that deleting a map node removes its connected map links while retaining Cacti devices and graphs.

Also clarify the raw Edit Configuration dialog, persist editor settings before exiting, use theme section headings consistently, and fix unreadable standalone dialog controls in legacy dark themes. The larger italic toolbar guidance remains in its original location.

Delete confirmations abandon both add_link and add_link2, along with other pending placement actions before switching back to normal map interaction, while retaining normal properties actions. Browser regressions cover node/link confirmations, Cancel and Close, cursor/help reset and preserved properties actions.

Validation: PHP 8.3 tests and all applicable Node/jsdom browser regressions pass in an isolated Cacti fixture. Syntax, whitespace, translation, manifest and changed-line coverage gates pass (100% of measured lines; existing controller/entry-point exemptions apply). The combined changes pass 292 PHP tests (5791 assertions) and 15 browser regressions. These revisions have not been deployed to the live installation.

This is one of five independently based develop PRs in the editor improvement batch. The branch includes updated develop with the merged icon picker and gallery resize. Shared editor files overlap, so later merges may still require reconciliation.

Unexpected link clicks during link placement now reset the editor mode, cursor and instruction text together.

[Copilot]

🟡 Changes recommended

The cursor selector cannot match the editor DOM, and new headings and cookie persistence lack required localization or regression coverage.

4 open findings

• Fix selector so pointer cursor applies to map hotspots · New
• Localize new section heading and regenerate POT · New
• Add regression test for editor settings cookie persistence · New
• Correct possessive pronoun from “it’s” to “its” · New

What changed in this PR

Improves editor placement resets, deletion clarity, settings persistence, and theme contrast.

Changes:

• Resets placement mode when dialogs or context menus open.
• Clarifies and styles destructive actions.
• Persists editor preferences and expands regression coverage.

File	Description
weathermap-cacti-plugin-mgmt.php	Loads plugin setup helpers.
weathermap-cacti-plugin-editor.php	Updates dialog text, headings, and live help.
lib/​editor.inc.php	Revises deletion and edit labels.
lib/​editor.actions.php	Persists editor settings cookie.
js/​editor.js	Resets placement state and styles delete actions.
css/​editor.css	Adds cursor, heading, deletion, and theme styles.
locales/​po/​cacti.pot	Updates translation strings.
tests/​Unit/​EditorJsStringsTest.php	Updates deletion-text assertions.
tests/​Browser/​EditorPickerLayeringTest.cjs	Loads new reset helpers.
tests/​Browser/​EditorContextMenuTest.cjs	Tests placement resets and confirmations.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The new deletion count logic bypasses locale-specific plural rules, and two HTML rows also violate required indentation.

1 open finding

• Add regression test for editor settings cookie persistence

3 resolved since last review

• Localize new section heading and regenerate POT
• Fix selector so pointer cursor applies to map hotspots
• Correct possessive pronoun from “it’s” to “its”

Previously missed (2)

In code that hasn't changed since last review

Use locale-aware pluralization for confirmation messages

js/​editor.js:769

The connected === 1 branch hard-codes English's two plural forms instead of using the locale's gettext plural rule. Languages with three or more forms will receive the same plural translation for counts such as 2 and 5, so the new confirmation becomes grammatically incorrect. Please either use a count-neutral translatable message or expose/select the locale-aware plural forms.

Fix table row indentation to match PHP style

weathermap-cacti-plugin-editor.php:906

Indent this table row to match the surrounding PHP/HTML block; PHP files in this repository require tab indentation.

This issue also appears on line 925 of the same file.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

The non-modal node confirmation permits changing the deletion target while the original warning remains open.

2 open findings

• Make node deletion confirmation modal to prevent target mutation · New
• Add regression test for editor settings cookie persistence

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The state, confirmation, styling, localization, and persistence changes are coherent and covered by focused regressions.

1 open finding

• Add regression test for editor settings cookie persistence

1 resolved since last review

• Make node deletion confirmation modal to prevent target mutation

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

An unexpected link click can clear the action while leaving the new crosshair placement state and action guidance active.

2 open findings

• Reset map mode after unexpected link click · New
• Add regression test for editor settings cookie persistence

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Cancelling Add Link by clicking a link leaves the new placement styling and help active.

2 open findings

• Reset map mode after unexpected link click
• Add regression test for editor settings cookie persistence

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Invalid link-placement clicks can leave the new placement class and action guidance stuck after the action is cancelled.

2 open findings

• Reset map mode after unexpected link click
• Add regression test for editor settings cookie persistence

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The implementation consistently addresses the described editor issues and includes focused regression coverage.

1 open finding

• Add regression test for editor settings cookie persistence

1 resolved since last review

• Reset map mode after unexpected link click

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The state-management, presentation, localization, and persistence changes are coherent and supported by focused regressions.

1 open finding

• Add regression test for editor settings cookie persistence

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The changes are consistent with the stated editor improvements and include focused regression coverage.

1 open finding

• Add regression test for editor settings cookie persistence

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The focused editor changes are internally consistent and covered by relevant unit and browser regressions.

1 open finding

• Add regression test for editor settings cookie persistence

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The editor behavior, styling, translations, persistence, and regression coverage are consistent with the stated purpose.

1 open finding

• Add regression test for editor settings cookie persistence

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The changes are coherent, appropriately tested, and no unresolved correctness issues were found.

0 open findings

1 resolved since last review

• Add regression test for editor settings cookie persistence

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Delete confirmations can still retain pending link-placement mode when opened from an existing properties dialog.

1 open finding

• Cancel pending link placement when resetting node properties mode · New

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Second-stage link placement can remain pending after either delete confirmation is closed.

0 open findings

1 resolved since last review

• Cancel pending link placement when resetting node properties mode

Previously missed (1)

In code that hasn't changed since last review

Include add_link2 in cancellation allowlist

js/​editor.js:774

The second phase of link placement uses add_link2, but this cancellation allowlist only includes add_link. Because Add Link leaves an open node-properties dialog usable, a user can select the first endpoint and then open this confirmation; Cancel/Close clears the cursor while leaving add_link2 pending, so the next node click unexpectedly submits the link. Include add_link2 among the placement actions that are cleared.

This issue also appears on line 862 of the same file.

🧠 Review effort: Balanced

[alcatron]

The additional add_link2 finding in the latest review summary is fixed in c9333a1. Both delete confirmation paths clear both link-placement stages, and mapmode consistently recognizes add_link2 as placement. The real browser regression covers both stages for node/link confirmations and both Cancel/Close exits, checking no pending link action, cursor class or placement guidance survives, while normal properties actions are retained.