performwp / performwp/perform

Retire the obsolete classic settings renderer and duplicate save path

Open
#203 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area: admin-ui area: settings owner:codex priority: medium risk: backward-compatibility status: ready type: maintenance
Dominant language
PHP
Stars
5
Forks
2
PR merge metrics
No merged PRs in 30d

Description

Problem

The active settings page is React-only, but the bundle still ships a second DOM save handler and the PHP class that renders the former classic settings form. Plugin bootstrap constructs Settings\Menu, not Settings\Api; the legacy identifiers have no production renderer. Keeping both paths duplicates persistence assumptions, leaves dead markup/JS in the build, and increases the chance that a future change accidentally revives an incompatible save path.

Evidence

  • src/Plugin.php:48-50 initializes Settings\Menu and has no Settings\Api construction.
  • src/Admin/Settings/Menu.php:53-65 renders only the React mount point.
  • assets/src/js/admin/main.js:4-9 mounts SettingsApp, while lines 11-51 retain a separate handler for the old perform-save-settings button and perform-admin-settings-form.
  • src/Admin/Settings/Api.php:46-105 still renders that old form and controls, but repository search finds no production call site.
  • The React save path uses JSON/nonce payloads, while the legacy path uses FormData, creating two divergent client contracts for one endpoint.

Scope

Retire the unused classic settings rendering/save path after proving no supported extension or screen invokes it, leaving one React settings contract.

Implementation plan

  1. Confirm public hooks, documented extension points, and the shipped admin page have no supported dependency on Settings\Api or legacy DOM IDs.
  2. Remove the unused Settings\Api class and the second DOMContentLoaded/save listener from assets/src/js/admin/main.js.
  3. Delete obsolete CSS/selectors and translations only when they are exclusively tied to the retired markup.
  4. Keep the React AJAX payload, nonce handling, secret masking, and UI behavior unchanged.
  5. Add a focused smoke assertion that the settings page mounts one save control and submits one request.
  6. Update maintainer documentation if any internal reference still describes the former renderer.

Acceptance criteria

  • The shipped settings page has one rendering path and one client save contract.
  • No unused legacy settings DOM IDs or save listener remain in source or built admin assets.
  • Existing settings values, secrets, and AJAX behavior remain backward compatible.
  • Admin frontend lint/build and settings smoke coverage pass.

Validation

  • npm run lint:js
  • npm run build
  • composer test for settings payload/server behavior
  • Isolated WordPress admin proof: load settings, change one field, verify exactly one successful save request.

Non-goals and risk

  • Do not refactor the broader module lifecycle; issue #103 owns that work.
  • Do not remove documented public hooks or change stored option keys.
  • If a released extension directly instantiates the internal renderer, stop and obtain an owner compatibility decision before removal.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by checking the supported hooks, extension references, and shipped admin page for dependencies on src/Admin/Settings/Api.php or the legacy DOM IDs. Then inspect assets/src/js/admin/main.js, the related CSS and translations, and the settings smoke coverage before running npm run lint:js, npm run build, and composer test. Done means the React settings page retains its payload and behavior with one save control and no unused legacy renderer or listener.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, php, react
Domain
backend, build-system, frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.