Retire the obsolete classic settings renderer and duplicate save path
Nobody has claimed this yet.
- 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-50initializesSettings\Menuand has noSettings\Apiconstruction.src/Admin/Settings/Menu.php:53-65renders only the React mount point.assets/src/js/admin/main.js:4-9mountsSettingsApp, while lines11-51retain a separate handler for the oldperform-save-settingsbutton andperform-admin-settings-form.src/Admin/Settings/Api.php:46-105still 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
- Confirm public hooks, documented extension points, and the shipped admin page have no supported dependency on
Settings\Apior legacy DOM IDs. - Remove the unused
Settings\Apiclass and the second DOMContentLoaded/save listener fromassets/src/js/admin/main.js. - Delete obsolete CSS/selectors and translations only when they are exclusively tied to the retired markup.
- Keep the React AJAX payload, nonce handling, secret masking, and UI behavior unchanged.
- Add a focused smoke assertion that the settings page mounts one save control and submits one request.
- 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:jsnpm run buildcomposer testfor 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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