Migrate Settings screen to React (REST-driven, legacy fallback)#1897
Open
arifulhoque7 wants to merge 18 commits into
Open
Migrate Settings screen to React (REST-driven, legacy fallback)#1897arifulhoque7 wants to merge 18 commits into
arifulhoque7 wants to merge 18 commits into
Conversation
Replace the legacy WeDevs_Settings_API screen with a React app served entirely over `GET/POST wpuf/v1/settings`, gated by `current_user_can( wpuf_admin_role() )`. - Schema bridge: renders whatever `wpuf_settings_sections/fields` produce, so free + Pro + module fields appear with no JS changes; storage parity preserved (same option keys, value shapes, sanitization). - Redesigned IA (tabs + sub-tabs), Figma-synced cards/logos/icons, tab state in the URL, dynamic merge-tag hints under email bodies. - Custom React fields for callbacks/own-option settings: General provider cards, payment/SMS/social provider cards, file uploader, email role-template repeater, profile-form-roles, tax country/state + rates, reactive AI config (test connection + fetch models). - Extensibility: `wpuf_settings_rest_data` filter + `wpuf_settings_saved` action with an `extra` side-channel for own-option settings; field-name dedupe so Pro's functional field wins over the free preview. - Security: number stays scalar-form on no-op save (byte parity), AI provider allowlisted, n8n secret fields no longer null-crash. Build outputs (assets/js/settings-react.*, assets/css/settings-react.css) are git-ignored — produced by `npm run build:settings-react`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Legacy fallback: a `wpuf_settings_ui_mode` option (+ `?wpuf_settings_ui=`
emergency override, `WPUF_LEGACY_SETTINGS` constant, `wpuf_use_legacy_settings`
filter) renders the classic WeDevs_Settings_API screen instead of React. Both
screens use the same wpuf_* options, so they stay in sync; nonce-protected
switch links both ways. React bundle is skipped in legacy mode.
- Header title is Pro-aware ("WP User Frontend Pro" when Pro is active).
- Unsaved Changes modal uses the Figma warning badge icon.
- Tax rate rows: fixed-width rate input with aligned `%` suffix (was forced
full-width, squishing the country/state selects); spinner arrows hidden.
- Removed the "moved to User Center" notice on Frontend Posting.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The sub-tab reset effect wiped the URL-restored `sub` on init: on mount `prevTab` became the default empty tab, then the URL-restore set the real tab, which the effect read as a change and cleared `activeSub`. Drop that effect and reset the sub-tab only on an actual user tab switch (handleSelectTab / handleDiscard), so `?tab=…&sub=…` survives a refresh / shared link. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- depends_on conditional rendering in the React settings screen: string form (turnstile keys on enable_turnstile), string+value (n8n auth fields per authentication_type), and object/AND form (n8n JWT passphrase vs PEM). Fields show/hide reactively as the controlling value changes. - Email "Select Roles" routes by `_default_roles` name suffix to the multiselect (field is a legacy callback again). - URL sub-tab restore now validates `?sub=` belongs to the restored tab — a stale/hand-edited value no longer shows a blank panel. - Drop the unused `gemini` provider from the AI key loops (was writing an empty `gemini_api_key` on every save). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- AI Temperature didn't persist: AISettings rendered NumberField without min/max/step, so it defaulted to step=1 and the browser rejected decimals (0.7/0.5) — value never updated. Pass the 0–1 range + step 0.1 + default 0.7 + the description help text. - File field (Set Invoice Logo / email Header Image): the input used SETTING_CLASS_NAMES (min-w-full), forcing 100% width in its flex row and pushing the "Select" button outside the card. Switch to a flex-shrink input (min-w-0 flex-1). Same class of overflow as the tax rate input — those are the only two inputs placed in a flex row with a sibling; all other fields render block-level and fit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Search now spans ALL tabs (not just the active one): typing a term renders matching sections from every tab under a "Search results" heading; clicking a nav tab clears the search and jumps there. - Provider-aware: searching a provider name (e.g. "facebook") shows that provider's full credential fields (App ID/Secret/Redirect), auto-selecting it in the provider cards — previously only the label-matched field showed. - Empty state: "No settings found" when a search has no matches. - Search box: clear (X) button. - Loading: animated skeleton (nav + field placeholders) instead of bare text. - Save feedback: transient "Saved ✓" confirmation in the footer on success. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- searchHasResults (settings.jsx) now matches provider labels/keys, mirroring SettingsSection/ProviderTabs. Searching a provider name that isn't in any field label — e.g. "facebook" (fields are fb_*) or "vonage" (nexmo_*) — used to short-circuit into "No settings found" even though the provider card + fields would render. PROVIDER_SECTIONS is now exported and shared so the two matchers can't diverge. - During a provider search, hide standalone non-provider fields (the global "Enable Social Login" toggle) that don't match the term, so they don't appear as noise above the matched provider's card. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Save "Saved ✓" now driven by the save() thunk's explicit boolean result (handleSave) instead of inferring from isSaving/error transitions (M2). - AI Test Connection: a resolved response is success, real failures land in catch — removed the dead success-false branch (M3). - AI Fetch latest models: Google's refresh reads the SAVED key, so a freshly typed unsaved key now surfaces "Save your Google API key first, then fetch" instead of silently doing nothing (M4). - depends_on: object/AND and single-value forms now share one valueMatches helper, so a required "on"/truthy controller value matches consistently in both (L6). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the easy-to-miss inline red banner with a centered MessageModal (icon badge + title + message + OK, Figma modal style). Save and load failures now show prominently and dismiss via OK / X / Esc, clearing the error state. Reusable for error/warning/success tones. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make the settings area easier to work with — no storage/DB-key changes: - WP_DEBUG-only guard (wpuf_settings_legacy_compat_check) logs a warning for any settings field whose type the legacy WeDevs_Settings_API can't render and that has no callback — catches the exact "classic screen breaks" regression early. - docs/settings-react.md: how the schema → REST → React flow works, how to add a setting (one field, both screens), supported types, the special-renderer escape hatches, depends_on, the legacy fallback, and the storage-parity rule. - FieldRenderer: flatten the deeply-nested component-selection ternary into a readable if/else chain (no behavior change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Behavior-preserving readability fixes from a settings-wide audit. Picked the clear, low-risk wins; skipped the churny/abstraction-heavy ones. - Shared `stripTags` util (components/settings/utils.js) replaces 4 copied definitions + 2 inline regexes (one copy in AISettings silently returned the input on non-strings — now consistent). - Shared `ModalShell` (overlay + card + close button + Escape) — UnsavedChanges and MessageModal had this byte-identical; both now render content inside it. - PHP Settings.php: extract `sanitize_number()` (the regex/scalar-preserve algorithm) and `is_pro_preview_section()` (the inline pro-badge regex) out of the long `sanitize_value`/`get_items` methods. - MultiSelectChips: flatten the summary nested ternary to if/else + name the magic `3` (MAX_NAMES_IN_SUMMARY). - GeneralSettings: declare `byName` before `fieldIsPro` (was a forward ref). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The readability refactor removed AISettings' local stripTags definition but didn't add the shared import, so the AI section threw "stripTags is not defined". Import it from ../../utils. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughIntroduces a React-based settings screen with a new REST endpoint, store, UI shell, field renderers, build setup, docs, and Playwright coverage. It also adds legacy/React mode switching and generated-asset ignore rules. ChangesReact Settings Screen
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add Playwright coverage (Page Object Model, matching existing conventions) for WPUF > Settings: - pages/settingsReact.ts — SettingsReactPage POM (nav/sub-tabs, field by label, search, save/dirty, unsaved-changes modal, legacy fallback). - pages/selectors.ts — settingsReact selector block (text/role/structure based). - tests/settingsReactTest.spec.ts — @Test_SR0001–0023: mount/skeleton, tabs + sub-tabs, header Pro/Free, URL tab+sub persistence (+ invalid sub), dirty state, save confirmation, save round-trip & no-op parity, unsaved-changes modal, global + provider-aware search, no-results, clear button, search reset on tab click, legacy fallback (Classic view + ?wpuf_settings_ui= override). - Registered in playwright.parallel-two.config.ts (alongside the other settings suites). tsc clean; Playwright lists all cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
includes/functions/settings-react.php-452-456 (1)
452-456: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix PHPCS array formatting to unblock CI.
The
$supportedarray packs multiple items per line, which fails the PHPCS check ("Each item in a multi-line array must be on a new line"). Put one item per line.🔧 Proposed fix
- $supported = [ - 'text', 'hidden', 'url', 'number', 'checkbox', 'multicheck', 'gateway_selector', - 'radio', 'radio_inline', 'select', 'textarea', 'html', 'wysiwyg', 'file', 'password', - 'color', 'toggle', - ]; + $supported = [ + 'text', + 'hidden', + 'url', + 'number', + 'checkbox', + 'multicheck', + 'gateway_selector', + 'radio', + 'radio_inline', + 'select', + 'textarea', + 'html', + 'wysiwyg', + 'file', + 'password', + 'color', + 'toggle', + ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@includes/functions/settings-react.php` around lines 452 - 456, The $supported array in settings-react.php is formatted in a way that violates PHPCS multi-line array rules. Update the array declaration so each value in $supported is placed on its own line, keeping the existing contents and order intact, to satisfy the "Each item in a multi-line array must be on a new line" check.Source: Pipeline failures
src/js/components/settings/kit/fields/NumberField.jsx-27-27 (1)
27-27: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDefault value of
0is silently dropped.
field.default || ''treats a numeric0default as falsy, so a field whose schema default is0renders empty instead of0. Use an explicit nullish check.🐛 Proposed fix
- value={ value !== undefined && value !== null ? value : ( field.default || '' ) } + value={ value !== undefined && value !== null + ? value + : ( field.default !== undefined && field.default !== null ? field.default : '' ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/fields/NumberField.jsx` at line 27, The NumberField value fallback is dropping valid default values of 0 because the default is read with a falsy check. Update the fallback logic in NumberField.jsx so the value uses a nullish check for field.default instead of an || fallback, keeping 0 intact while still falling back to an empty string only when default is undefined or null.src/js/components/settings/kit/fields/ProfileFormRoles.jsx-37-37 (1)
37-37: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBlock edits for Pro-only mapping when Pro is inactive.
Line 37 still writes role mappings even when
isProis false (while Line 61 says this is Pro-only), which creates misleading editable UI state.Proposed fix
- const setRole = ( role, formId ) => setExtraValue( KEY, { ...data, map: { ...map, [ role ]: formId } } ); + const setRole = ( role, formId ) => { + if ( ! isPro ) { + return; + } + setExtraValue( KEY, { ...data, map: { ...map, [ role ]: formId } } ); + };Also applies to: 45-63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/fields/ProfileFormRoles.jsx` at line 37, The role-mapping logic in ProfileFormRoles.jsx still updates and exposes Pro-only mappings even when isPro is false, which leaves the UI in an editable state it should not have. Update the setRole flow and the related render logic in ProfileFormRoles to gate all mapping writes and edit controls behind isPro, and ensure the non-Pro branch is read-only or hidden for the affected role mapping UI. Make sure the changes cover the setRole function and the conditional UI paths around the Pro-only mapping section so no role map state is mutated when Pro is inactive.src/js/components/settings/kit/fields/MultiSelectChips.jsx-85-87 (1)
85-87: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssociate the field label with the dropdown trigger.
Line 86 renders a
<label>withouthtmlFor, and the trigger at Line 92 has noid, so screen readers won’t correctly bind them.Proposed fix
- <label className="wpuf-text-sm wpuf-text-gray-700 wpuf-my-2">{ field.label }</label> + <label htmlFor={ `${ name }_trigger` } className="wpuf-text-sm wpuf-text-gray-700 wpuf-my-2">{ field.label }</label> ... <button + id={ `${ name }_trigger` } type="button"Also applies to: 92-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/fields/MultiSelectChips.jsx` around lines 85 - 87, The field label in MultiSelectChips.jsx is not associated with the dropdown trigger, so update the label/trigger pair in the component’s render logic to use a matching identifier. Add an id to the dropdown trigger element and pass the same value to the label’s htmlFor (or equivalent accessible association) so field.label correctly binds to the interactive control rendered around the trigger and menu.src/js/components/settings/kit/fields/SelectDropdown.jsx-137-139 (1)
137-139: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent Space key from scrolling while selecting an option.
At Line 137, Space triggers selection but doesn’t call
preventDefault(), so keyboard selection may also scroll the page.Proposed fix
onKeyDown={ ( e ) => { if ( e.key === 'Enter' || e.key === ' ' ) { + e.preventDefault(); select( key ); } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/fields/SelectDropdown.jsx` around lines 137 - 139, The keyboard handler in SelectDropdown.jsx selects an option on Enter and Space, but the Space branch does not stop the browser’s default scrolling behavior. Update the keydown logic in the option selection handler so that when e.key is ' ' (or Space), it calls preventDefault() before invoking select(key), while keeping the Enter behavior unchanged.src/js/components/settings/kit/fields/MultiSelectChips.jsx-142-145 (1)
142-145: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrevent page scroll on keyboard selection in options list.
At Line 143, pressing Space selects an item but can also trigger page scroll because default behavior isn’t prevented.
Proposed fix
onKeyDown={ ( e ) => { if ( e.key === 'Enter' || e.key === ' ' ) { + e.preventDefault(); handleSelect( key ); } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/fields/MultiSelectChips.jsx` around lines 142 - 145, The keyboard handler in MultiSelectChips.jsx allows Space to select an option but doesn’t stop the browser’s default scroll behavior. Update the onKeyDown logic for the options list item so that, in the Enter/Space path, the event default is prevented before calling handleSelect(key). Keep the fix localized to the existing onKeyDown callback around the option selection logic.src/js/components/settings/kit/fields/WysiwygField.jsx-23-29 (1)
23-29: 🎯 Functional Correctness | 🟡 MinorSync the WYSIWYG editor on reset
TextEditorinitializes TinyMCE fromdefaultValueonce and never reapplies latervaluechanges, soDiscardcan leave the visible editor on stale content. Force a remount when settings reset, or makeTextEditorupdate the editor whenvaluechanges.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/fields/WysiwygField.jsx` around lines 23 - 29, The WYSIWYG editor in WysiwygField.jsx is only initialized once, so later value changes from a reset are not reflected in the visible TinyMCE content. Update the integration around TextEditor so it re-syncs when the field value changes, either by forcing a remount on reset or by teaching TextEditor to react to value updates after initialization. Use the TextEditor usage in WysiwygField and its value/defaultValue handling to locate the fix.src/js/components/settings/Header.jsx-71-72 (1)
71-72: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove trailing whitespace from translatable string.
'Support 'should be'Support'to avoid translation-key mismatch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/Header.jsx` around lines 71 - 72, The translatable label in the Header component includes a trailing space, which can cause a translation-key mismatch. Update the string passed to the translation helper in the settings Header JSX so it uses the exact text without extra whitespace, and keep the surrounding spacing handled by layout markup instead of the translation string.src/js/components/settings/ErrorBoundary.jsx-27-29 (1)
27-29: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid rendering
"undefined"in the error notice.Use a fallback when
error.messageis absent.Suggested fix
- { String( this.state.error && this.state.error.message ) } + { String( ( this.state.error && this.state.error.message ) || __( 'Unknown error.', 'wp-user-frontend' ) ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/ErrorBoundary.jsx` around lines 27 - 29, The error notice in ErrorBoundary currently renders String(this.state.error && this.state.error.message), which can display "undefined" when the error has no message. Update the ErrorBoundary render path to use a safe fallback when this.state.error.message is missing, so the user sees a meaningful message instead of "undefined"; use the ErrorBoundary class and its this.state.error access to locate the change.src/js/stores-react/settings/actions.js-119-121 (1)
119-121: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard non-Error throws in
save()catch.Direct
e.messageaccess can throw for nullish rejection values and leave save-state cleanup incomplete.Suggested fix
- } catch ( e ) { - dispatch.setError( e.message || __( 'Failed to save settings.', 'wp-user-frontend' ) ); + } catch ( e ) { + dispatch.setError( ( e && e.message ) || __( 'Failed to save settings.', 'wp-user-frontend' ) ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/stores-react/settings/actions.js` around lines 119 - 121, The save() catch block in actions.js directly reads e.message, which can itself fail when the rejection value is nullish or not an Error. Update the catch handling to safely derive the message from the thrown value in save(), using a guarded fallback for non-Error throws before calling dispatch.setError, so cleanup/error handling can complete reliably.src/js/components/settings/kit/SettingsField.jsx-13-13 (1)
13-13: 🎯 Functional Correctness | 🟡 MinorPrefix the final utility with
wpuf-
dropdownends with!leading-6, but this Tailwind config usesprefix: 'wpuf-', so the line-height class should be!wpuf-leading-6.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/SettingsField.jsx` at line 13, The dropdown utility string in SettingsField should use the Tailwind prefix consistently, since this config is using prefix: 'wpuf-'. Update the final line-height utility in the dropdown class list from the unprefixed variant to the prefixed one in the SettingsField configuration so it matches the rest of the wpuf- utilities.src/js/components/settings/SettingsNav.jsx-11-33 (1)
11-33: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd an accessible name to the search input.
The input relies solely on
placeholderfor its name; placeholders aren't exposed as accessible labels to assistive tech. Add anaria-label.♿ Proposed fix
<input type="text" value={ search } placeholder={ __( 'Search settings…', 'wp-user-frontend' ) } + aria-label={ __( 'Search settings', 'wp-user-frontend' ) } onChange={ ( e ) => onSearch( e.target.value ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/SettingsNav.jsx` around lines 11 - 33, The search field in SettingsNav lacks an accessible name because it only uses the placeholder, so update the input in SettingsNav.jsx to include an explicit aria-label for screen readers. Keep the existing placeholder for visual guidance, and ensure the new label clearly describes the field’s purpose (searching settings).src/js/components/settings/kit/Accordion.jsx-15-36 (1)
15-36: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winExpose the open/closed state to assistive tech.
The toggle button communicates state only visually (chevron rotation). Add
aria-expanded={ open }(and ideallyaria-controlsreferencing the panel) so screen-reader users know the section is collapsible and its current state.♿ Proposed fix
<button type="button" + aria-expanded={ open } onClick={ () => setOpen( ( v ) => ! v ) } className="wpuf-flex wpuf-w-full wpuf-items-center wpuf-gap-3 wpuf-py-4 wpuf-text-left" >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/Accordion.jsx` around lines 15 - 36, The Accordion toggle in the JSX component only shows its open state visually; update the button in Accordion to expose state to assistive tech by adding aria-expanded bound to open and, if possible, aria-controls tied to the content panel. Use the existing button click handler and open state, and ensure the panel rendered when open has a matching id so screen readers can identify the relationship.src/js/settings.jsx-99-122 (1)
99-122: 🚀 Performance & Scalability | 🟡 MinorAvoid the redundant settings fetch on mount.
useSelect()already readsstore.getSections(), which triggers the resolver whensectionsis empty; the extraloadSettings()effect issues a secondwpuf/v1/settingsrequest on first render. Keep only one load path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/settings.jsx` around lines 99 - 122, The settings screen is fetching data twice on mount because the initial useEffect in settings.jsx calls loadSettings() even though useSelect() already reads store.getSections() and triggers the resolver when needed. Remove the redundant loadSettings() effect and keep the URL-restoration useEffect that depends on ia, setActiveTab, and didInit so the first render only uses one settings load path.
🧹 Nitpick comments (8)
src/css/settings.css (1)
149-151: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake tooltip visibility keyboard-accessible (not hover-only).
Tooltips are shown only on
:hover; add keyboard focus support so non-mouse users can access the same help text.Suggested fix
.wpuf-tooltip[data-tip]:hover::before, -.wpuf-tooltip[data-tip]:hover::after { +.wpuf-tooltip[data-tip]:hover::after, +.wpuf-tooltip[data-tip]:focus-within::before, +.wpuf-tooltip[data-tip]:focus-within::after { opacity: 1; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/css/settings.css` around lines 149 - 151, Tooltip visibility is currently triggered only by hover on the wpuf-tooltip[data-tip] selector, so keyboard users cannot access the help text. Update the tooltip state rules for the same selector to also apply on keyboard focus, using the existing tooltip styling block so the focus behavior matches hover behavior. Keep the change scoped to the tooltip selectors in the CSS and ensure the pseudo-elements shown by wpuf-tooltip[data-tip] are revealed for focus as well as hover.src/js/components/settings/kit/ProviderTabs.jsx (1)
59-99: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
activecan become stale when the provider list changes.
activeis seeded once at mount fromproviders[0]. When not searching, ifproviderslater changes (a provider gains/loses fields and drops out of thehasFieldsfilter),activeKeymay point to a provider no longer present, leavingactiveFieldsempty until the user clicks another card. Consider falling back to the first available provider whenactiveisn't in the current list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/ProviderTabs.jsx` around lines 59 - 99, The active provider selection in ProviderTabs can become stale when the filtered providers list changes. Update the active state logic around useState, activeKey, and activeFields so that when providers no longer contains the current active key, it falls back to providers[0].key (or clears safely) instead of keeping a missing selection. Use the existing identifiers providers, active, setActive, activeKey, and activeFields to ensure the displayed fields always match a provider still present in the current list.src/js/components/settings/kit/fields/HtmlField.jsx (1)
6-18: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
dangerouslySetInnerHTMLsource is trusted schema, but confirm no user-derived content flows in.
htmlis derived fromfield.desc/default/html, which come from the PHP settings schema (wpuf_settings_fields()) and are plugin-defined, so the static-analysis XSS hint is largely a false positive here. The risk only materializes if any field description is ever populated from stored/user input. If that's possible, sanitize (e.g. DOMPurify) before injecting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/fields/HtmlField.jsx` around lines 6 - 18, The HtmlField component currently injects content via dangerouslySetInnerHTML using html from field.desc, field.default, and field.html, so confirm these values are always plugin-defined and never populated from stored or user input. If any of those sources can become user-derived, sanitize the content before rendering by passing it through a safe HTML sanitizer in HtmlField.jsx before the JSX return.Source: Linters/SAST tools
src/js/components/settings/kit/fields/TaxSettings.jsx (1)
69-110: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueIndex keys are lower risk here, but still prefer stable ids.
Unlike
RoleEmailTemplates, the rate rows render only controlled inputs (SelectDropdown, number input), so removing a middle row won't corrupt internal child state. The index-key reconciliation is therefore acceptable, but a stable per-row id would avoid unnecessary remounts and keep parity with the fix suggested inRoleEmailTemplates.jsx.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/fields/TaxSettings.jsx` around lines 69 - 110, The rate rows in TaxSettings.jsx still use the map index as the React key, which can cause unnecessary remounts when rows are added or removed. Update the rates.map render in TaxSettings to use a stable per-row identifier instead of i, and make sure the row data managed by update/remove carries that id consistently so each SelectDropdown and number input keeps a stable identity across reorders.Source: Linters/SAST tools
src/js/components/settings/kit/fields/TextField.jsx (1)
45-50: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
long_helprendered as raw HTML.
field.long_helpis injected viadangerouslySetInnerHTML. This is sourced from the server settings schema (admin-defined), so risk is low, but the same pattern repeats inTextareaFieldandWysiwygField. If any of these strings can ever carry merge-tag/user-influenced content, sanitize once in a shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/fields/TextField.jsx` around lines 45 - 50, The long_help rendering in TextField uses dangerouslySetInnerHTML, and the same raw-HTML pattern also exists in TextareaField and WysiwygField. Add a shared sanitizing helper for long_help before rendering, and update the fields to use the sanitized output instead of injecting the server-provided string directly. Keep the fix centralized so all three components use the same safe rendering path.Source: Linters/SAST tools
src/js/components/settings/kit/TextEditor.jsx (1)
50-57: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUncontrolled fallback textarea won't reflect external value changes (e.g. Discard).
Because the fallback uses
defaultValue, a later programmatic value change (such as a Discard restoringsavedValues) won't update the visible content unless the component remounts. If discard-while-editing is a supported flow for WYSIWYG fields, consider keying the field or re-syncing. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/kit/TextEditor.jsx` around lines 50 - 57, The fallback textarea in TextEditor is uncontrolled because it uses defaultValue, so programmatic updates like Discard won’t refresh the visible content. Update the TextEditor component to keep the textarea in sync with external value changes, either by making the fallback controlled via value or by adding a remount/sync mechanism keyed off the incoming value; use the TextEditor and onChange logic to locate the fix.src/js/stores-react/settings/index.js (1)
8-13: 📐 Maintainability & Code Quality | 🔵 TrivialUse
createReduxStoreandregisterinstead ofregisterStore
@wordpress/dataresolves to 9.28.0 here, whereregisterStoreis deprecated. Switch toconst store = createReduxStore( STORE_NAME, { ... } ); register( store );to match the current store API.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/stores-react/settings/index.js` around lines 8 - 13, The settings store setup still uses the deprecated registerStore API. In the store initialization around STORE_NAME, replace registerStore with createReduxStore to build the store object, then call register on that store from `@wordpress/data`. Keep the existing reducer, actions, selectors, and resolvers wired through the new createReduxStore call so the exported store setup matches the current API.src/js/components/settings/ModalShell.jsx (1)
12-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffModal lacks focus trapping and focus restoration.
Tab can move focus to background content behind the overlay, and focus isn't returned to the trigger element on close — both are accessibility gaps for a
role="dialog"aria-modal="true"element. Escape/initial focus are handled, but consider capturing the active element on open and restoring it on cleanup, plus trapping Tab withincardRef.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/js/components/settings/ModalShell.jsx` around lines 12 - 27, The ModalShell dialog setup currently handles Escape and initial focus but does not trap keyboard focus or restore it on close. Update the useEffect in ModalShell.jsx to capture the active element when the dialog opens, keep Tab/Shift+Tab cycling within cardRef, and restore focus to that stored element in the cleanup. Use the existing onClose, handleKeyDown, and cardRef logic as the anchor points for the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@includes/Api/Settings.php`:
- Around line 340-374: sanitize_value() is over-unslashing REST input, which can
strip legitimate backslashes from saved settings. In includes/Api/Settings.php,
update the sanitize_value method so the REST-provided values are sanitized
directly without wp_unslash() in the type-specific branches and default
fallback; keep the existing field-type handling, sanitize_callback path, and
helper methods like sanitize_number unchanged.
In `@src/css/settings.css`:
- Around line 71-84: Restore a visible keyboard focus state for the standard
controls in the `#wpuf-settings-root` focus rule by revising the
input/textarea/select styling in settings.css. The current selector group
(`#wpuf-settings-root` input:focus, input[type="text"]:focus, textarea:focus,
select:focus) removes the native/WP focus cues with outline and box-shadow
overrides while using a low-contrast border, so update this rule to keep or add
a clearly visible focus indicator (for example a stronger outline or box-shadow)
instead of suppressing it.
In `@src/js/components/settings/FieldRenderer.jsx`:
- Around line 65-97: The default branch in pickByCallback is too aggressive
because it forces every unknown callback to ProNote, which blocks editable
callback-backed fields from falling back to the normal type-based renderer.
Update pickByCallback so only the known non-editable entries from
NON_EDITABLE_CALLBACKS use a read-only note, while unknown callbacks can still
pass through to the existing field-type resolution in FieldRenderer instead of
always rendering ProNote. Make sure the special cases like
wpuf_settings_multiselect, wpuf_ai_api_key_field, wpuf_ai_temperature_field,
wpuf_descriptive_text, wpuf_settings_password_preview, and
wpuf_role_email_templates still map correctly.
In `@src/js/components/settings/kit/fields/RadioCardsField.jsx`:
- Around line 35-40: The icon handling in iconOf and the renders that use
dangerouslySetInnerHTML are passing through arbitrary opt.icon HTML, which can
allow admin-side XSS. Update RadioCardsField so opt.icon is treated as untrusted
input: either restrict it to a safe identifier/whitelisted SVG source or
sanitize it before it reaches the places that render the icon. Make sure the
values used in the icon render paths are derived from trusted mappings like
GATEWAY_SVG rather than raw schema/filter-provided HTML.
- Around line 47-52: Normalize field.default into an array before assigning
selected in RadioCardsField so selected is never an object when the field uses
an associative default. Update the selected initialization in RadioCardsField to
convert any object default to its keys, matching the existing object-handling
path for value, so later uses like selected.includes remain safe. Keep the fix
localized to the selected derivation logic and verify the single-choice branch
still returns an array.
In `@src/js/components/settings/kit/fields/RoleEmailTemplates.jsx`:
- Around line 42-75: The row rendering in RoleEmailTemplates.jsx is using the
array index as the React key, which causes WysiwygField/TextEditor instances to
be reused with shifted data after removals. Update the rows managed by the
add/update/remove flow to carry a stable unique id per row and use that id for
the key in the rows.map render, while keeping the existing name props aligned to
the same row identity.
In `@src/js/components/settings/kit/fields/TextField.jsx`:
- Line 35: The TextField value fallback is treating an intentionally empty
string as missing, so a cleared field snaps back to field.default and cannot
stay empty. Update the value handling in TextField to mirror TextareaField by
checking only for null/undefined instead of using the truthy chain, so value={
value || field.default || '' } preserves ''. Use the TextField component’s value
prop logic and field.default as the key symbols to fix.
In `@src/js/components/settings/kit/GeneralSettings.jsx`:
- Around line 66-82: The enable toggle in GeneralSettings is implemented with a
span acting as a checkbox, but it is not keyboard accessible. Update the span
used for the Turnstile/Cloudflare toggle to be focusable and operable via
keyboard by adding the appropriate tab index and keyboard handler, and make it
trigger the same toggle logic as onClick when activated with Enter or Space. Use
the existing checked, onToggle, and Check behavior as the anchor points so the
control remains consistent with the payment gateway card fix.
In `@src/js/components/settings/kit/PaymentGateways.jsx`:
- Around line 124-140: The gateway enable control in PaymentGateways is only
clickable with a mouse and cannot be used from the keyboard. Update the corner
span with role="checkbox" so it is focusable and operable via Enter/Space by
adding keyboard handling and a tab stop, and keep it wired to toggleEnable(gid)
with the current isEnabled state reflected correctly. Ensure the span’s
accessible interaction matches the parent gateway item’s behavior so
keyboard-only users can enable or disable a gateway.
In `@src/js/stores-react/settings/resolvers.js`:
- Around line 10-14: The getSections resolver is incorrectly treating its
argument as store state, which can be undefined and make state.sections throw.
Update getSections to either load settings unconditionally, or move the sections
existence check into the selector that calls it; use the getSections symbol to
locate the resolver and avoid dereferencing state inside the resolver.
---
Minor comments:
In `@includes/functions/settings-react.php`:
- Around line 452-456: The $supported array in settings-react.php is formatted
in a way that violates PHPCS multi-line array rules. Update the array
declaration so each value in $supported is placed on its own line, keeping the
existing contents and order intact, to satisfy the "Each item in a multi-line
array must be on a new line" check.
In `@src/js/components/settings/ErrorBoundary.jsx`:
- Around line 27-29: The error notice in ErrorBoundary currently renders
String(this.state.error && this.state.error.message), which can display
"undefined" when the error has no message. Update the ErrorBoundary render path
to use a safe fallback when this.state.error.message is missing, so the user
sees a meaningful message instead of "undefined"; use the ErrorBoundary class
and its this.state.error access to locate the change.
In `@src/js/components/settings/Header.jsx`:
- Around line 71-72: The translatable label in the Header component includes a
trailing space, which can cause a translation-key mismatch. Update the string
passed to the translation helper in the settings Header JSX so it uses the exact
text without extra whitespace, and keep the surrounding spacing handled by
layout markup instead of the translation string.
In `@src/js/components/settings/kit/Accordion.jsx`:
- Around line 15-36: The Accordion toggle in the JSX component only shows its
open state visually; update the button in Accordion to expose state to assistive
tech by adding aria-expanded bound to open and, if possible, aria-controls tied
to the content panel. Use the existing button click handler and open state, and
ensure the panel rendered when open has a matching id so screen readers can
identify the relationship.
In `@src/js/components/settings/kit/fields/MultiSelectChips.jsx`:
- Around line 85-87: The field label in MultiSelectChips.jsx is not associated
with the dropdown trigger, so update the label/trigger pair in the component’s
render logic to use a matching identifier. Add an id to the dropdown trigger
element and pass the same value to the label’s htmlFor (or equivalent accessible
association) so field.label correctly binds to the interactive control rendered
around the trigger and menu.
- Around line 142-145: The keyboard handler in MultiSelectChips.jsx allows Space
to select an option but doesn’t stop the browser’s default scroll behavior.
Update the onKeyDown logic for the options list item so that, in the Enter/Space
path, the event default is prevented before calling handleSelect(key). Keep the
fix localized to the existing onKeyDown callback around the option selection
logic.
In `@src/js/components/settings/kit/fields/NumberField.jsx`:
- Line 27: The NumberField value fallback is dropping valid default values of 0
because the default is read with a falsy check. Update the fallback logic in
NumberField.jsx so the value uses a nullish check for field.default instead of
an || fallback, keeping 0 intact while still falling back to an empty string
only when default is undefined or null.
In `@src/js/components/settings/kit/fields/ProfileFormRoles.jsx`:
- Line 37: The role-mapping logic in ProfileFormRoles.jsx still updates and
exposes Pro-only mappings even when isPro is false, which leaves the UI in an
editable state it should not have. Update the setRole flow and the related
render logic in ProfileFormRoles to gate all mapping writes and edit controls
behind isPro, and ensure the non-Pro branch is read-only or hidden for the
affected role mapping UI. Make sure the changes cover the setRole function and
the conditional UI paths around the Pro-only mapping section so no role map
state is mutated when Pro is inactive.
In `@src/js/components/settings/kit/fields/SelectDropdown.jsx`:
- Around line 137-139: The keyboard handler in SelectDropdown.jsx selects an
option on Enter and Space, but the Space branch does not stop the browser’s
default scrolling behavior. Update the keydown logic in the option selection
handler so that when e.key is ' ' (or Space), it calls preventDefault() before
invoking select(key), while keeping the Enter behavior unchanged.
In `@src/js/components/settings/kit/fields/WysiwygField.jsx`:
- Around line 23-29: The WYSIWYG editor in WysiwygField.jsx is only initialized
once, so later value changes from a reset are not reflected in the visible
TinyMCE content. Update the integration around TextEditor so it re-syncs when
the field value changes, either by forcing a remount on reset or by teaching
TextEditor to react to value updates after initialization. Use the TextEditor
usage in WysiwygField and its value/defaultValue handling to locate the fix.
In `@src/js/components/settings/kit/SettingsField.jsx`:
- Line 13: The dropdown utility string in SettingsField should use the Tailwind
prefix consistently, since this config is using prefix: 'wpuf-'. Update the
final line-height utility in the dropdown class list from the unprefixed variant
to the prefixed one in the SettingsField configuration so it matches the rest of
the wpuf- utilities.
In `@src/js/components/settings/SettingsNav.jsx`:
- Around line 11-33: The search field in SettingsNav lacks an accessible name
because it only uses the placeholder, so update the input in SettingsNav.jsx to
include an explicit aria-label for screen readers. Keep the existing placeholder
for visual guidance, and ensure the new label clearly describes the field’s
purpose (searching settings).
In `@src/js/settings.jsx`:
- Around line 99-122: The settings screen is fetching data twice on mount
because the initial useEffect in settings.jsx calls loadSettings() even though
useSelect() already reads store.getSections() and triggers the resolver when
needed. Remove the redundant loadSettings() effect and keep the URL-restoration
useEffect that depends on ia, setActiveTab, and didInit so the first render only
uses one settings load path.
In `@src/js/stores-react/settings/actions.js`:
- Around line 119-121: The save() catch block in actions.js directly reads
e.message, which can itself fail when the rejection value is nullish or not an
Error. Update the catch handling to safely derive the message from the thrown
value in save(), using a guarded fallback for non-Error throws before calling
dispatch.setError, so cleanup/error handling can complete reliably.
---
Nitpick comments:
In `@src/css/settings.css`:
- Around line 149-151: Tooltip visibility is currently triggered only by hover
on the wpuf-tooltip[data-tip] selector, so keyboard users cannot access the help
text. Update the tooltip state rules for the same selector to also apply on
keyboard focus, using the existing tooltip styling block so the focus behavior
matches hover behavior. Keep the change scoped to the tooltip selectors in the
CSS and ensure the pseudo-elements shown by wpuf-tooltip[data-tip] are revealed
for focus as well as hover.
In `@src/js/components/settings/kit/fields/HtmlField.jsx`:
- Around line 6-18: The HtmlField component currently injects content via
dangerouslySetInnerHTML using html from field.desc, field.default, and
field.html, so confirm these values are always plugin-defined and never
populated from stored or user input. If any of those sources can become
user-derived, sanitize the content before rendering by passing it through a safe
HTML sanitizer in HtmlField.jsx before the JSX return.
In `@src/js/components/settings/kit/fields/TaxSettings.jsx`:
- Around line 69-110: The rate rows in TaxSettings.jsx still use the map index
as the React key, which can cause unnecessary remounts when rows are added or
removed. Update the rates.map render in TaxSettings to use a stable per-row
identifier instead of i, and make sure the row data managed by update/remove
carries that id consistently so each SelectDropdown and number input keeps a
stable identity across reorders.
In `@src/js/components/settings/kit/fields/TextField.jsx`:
- Around line 45-50: The long_help rendering in TextField uses
dangerouslySetInnerHTML, and the same raw-HTML pattern also exists in
TextareaField and WysiwygField. Add a shared sanitizing helper for long_help
before rendering, and update the fields to use the sanitized output instead of
injecting the server-provided string directly. Keep the fix centralized so all
three components use the same safe rendering path.
In `@src/js/components/settings/kit/ProviderTabs.jsx`:
- Around line 59-99: The active provider selection in ProviderTabs can become
stale when the filtered providers list changes. Update the active state logic
around useState, activeKey, and activeFields so that when providers no longer
contains the current active key, it falls back to providers[0].key (or clears
safely) instead of keeping a missing selection. Use the existing identifiers
providers, active, setActive, activeKey, and activeFields to ensure the
displayed fields always match a provider still present in the current list.
In `@src/js/components/settings/kit/TextEditor.jsx`:
- Around line 50-57: The fallback textarea in TextEditor is uncontrolled because
it uses defaultValue, so programmatic updates like Discard won’t refresh the
visible content. Update the TextEditor component to keep the textarea in sync
with external value changes, either by making the fallback controlled via value
or by adding a remount/sync mechanism keyed off the incoming value; use the
TextEditor and onChange logic to locate the fix.
In `@src/js/components/settings/ModalShell.jsx`:
- Around line 12-27: The ModalShell dialog setup currently handles Escape and
initial focus but does not trap keyboard focus or restore it on close. Update
the useEffect in ModalShell.jsx to capture the active element when the dialog
opens, keep Tab/Shift+Tab cycling within cardRef, and restore focus to that
stored element in the cleanup. Use the existing onClose, handleKeyDown, and
cardRef logic as the anchor points for the fix.
In `@src/js/stores-react/settings/index.js`:
- Around line 8-13: The settings store setup still uses the deprecated
registerStore API. In the store initialization around STORE_NAME, replace
registerStore with createReduxStore to build the store object, then call
register on that store from `@wordpress/data`. Keep the existing reducer, actions,
selectors, and resolvers wired through the new createReduxStore call so the
exported store setup matches the current API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5941353f-0eb5-4119-9416-3b947466ce8c
⛔ Files ignored due to path filters (17)
assets/images/cloudflare.pngis excluded by!**/*.pngassets/images/email/activated.svgis excluded by!**/*.svgassets/images/email/approved-post.svgis excluded by!**/*.svgassets/images/email/approved.svgis excluded by!**/*.svgassets/images/email/denied.svgis excluded by!**/*.svgassets/images/email/guest.svgis excluded by!**/*.svgassets/images/email/pending.svgis excluded by!**/*.svgassets/images/email/subscription.svgis excluded by!**/*.svgassets/images/gateways/clickatell.pngis excluded by!**/*.pngassets/images/gateways/plivo.pngis excluded by!**/*.pngassets/images/gateways/twilio.pngis excluded by!**/*.pngassets/images/gateways/vonage.pngis excluded by!**/*.pngassets/images/google-maps.pngis excluded by!**/*.pngassets/images/ipstack.pngis excluded by!**/*.pngassets/images/modal/unsaved-changes.svgis excluded by!**/*.svgassets/images/recaptcha.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (61)
.gitignoredocs/settings-react.mdincludes/API.phpincludes/Admin/Admin_Settings.phpincludes/Admin/Menu.phpincludes/Api/Settings.phpincludes/functions/settings-react.phppackage.jsonsrc/css/settings.csssrc/js/api/settings.jssrc/js/components/settings/ErrorBoundary.jsxsrc/js/components/settings/FieldRenderer.jsxsrc/js/components/settings/Header.jsxsrc/js/components/settings/MessageModal.jsxsrc/js/components/settings/ModalShell.jsxsrc/js/components/settings/SettingsNav.jsxsrc/js/components/settings/SettingsSection.jsxsrc/js/components/settings/UnsavedChanges.jsxsrc/js/components/settings/kit/Accordion.jsxsrc/js/components/settings/kit/GeneralSettings.jsxsrc/js/components/settings/kit/PaymentGateways.jsxsrc/js/components/settings/kit/ProBadge.jsxsrc/js/components/settings/kit/ProPreviewWrapper.jsxsrc/js/components/settings/kit/ProviderTabs.jsxsrc/js/components/settings/kit/SettingsField.jsxsrc/js/components/settings/kit/TextEditor.jsxsrc/js/components/settings/kit/email-icons.jsxsrc/js/components/settings/kit/fields/AISettings.jsxsrc/js/components/settings/kit/fields/CheckboxField.jsxsrc/js/components/settings/kit/fields/CheckboxGroupField.jsxsrc/js/components/settings/kit/fields/ColorPickerField.jsxsrc/js/components/settings/kit/fields/FileField.jsxsrc/js/components/settings/kit/fields/HelpTextIcon.jsxsrc/js/components/settings/kit/fields/HtmlField.jsxsrc/js/components/settings/kit/fields/MultiSelectChips.jsxsrc/js/components/settings/kit/fields/MultiSelectField.jsxsrc/js/components/settings/kit/fields/NumberField.jsxsrc/js/components/settings/kit/fields/PicRadioField.jsxsrc/js/components/settings/kit/fields/ProNote.jsxsrc/js/components/settings/kit/fields/ProfileFormRoles.jsxsrc/js/components/settings/kit/fields/RadioCardsField.jsxsrc/js/components/settings/kit/fields/RadioField.jsxsrc/js/components/settings/kit/fields/RoleEmailTemplates.jsxsrc/js/components/settings/kit/fields/SelectDropdown.jsxsrc/js/components/settings/kit/fields/SelectField.jsxsrc/js/components/settings/kit/fields/TaxSettings.jsxsrc/js/components/settings/kit/fields/TextField.jsxsrc/js/components/settings/kit/fields/TextareaField.jsxsrc/js/components/settings/kit/fields/ToggleField.jsxsrc/js/components/settings/kit/fields/WysiwygField.jsxsrc/js/components/settings/nav-icons.jsxsrc/js/components/settings/utils.jssrc/js/settings.jsxsrc/js/stores-react/settings/actions.jssrc/js/stores-react/settings/constants.jssrc/js/stores-react/settings/index.jssrc/js/stores-react/settings/reducer.jssrc/js/stores-react/settings/resolvers.jssrc/js/stores-react/settings/selectors.jstailwind.settings.config.jswebpack.config.js
Comment on lines
+340
to
+374
| protected function sanitize_value( $value, $field ) { | ||
| if ( isset( $field['sanitize_callback'] ) && is_callable( $field['sanitize_callback'] ) ) { | ||
| return call_user_func( $field['sanitize_callback'], $value ); | ||
| } | ||
|
|
||
| $type = isset( $field['type'] ) ? $field['type'] : 'text'; | ||
|
|
||
| switch ( $type ) { | ||
| case 'checkbox': | ||
| case 'multicheck': | ||
| return is_array( $value ) ? array_map( 'sanitize_text_field', wp_unslash( $value ) ) : sanitize_text_field( wp_unslash( $value ) ); | ||
|
|
||
| case 'multiselect': | ||
| return is_array( $value ) ? array_map( 'sanitize_text_field', wp_unslash( $value ) ) : []; | ||
|
|
||
| case 'textarea': | ||
| return sanitize_textarea_field( wp_unslash( $value ) ); | ||
|
|
||
| case 'wysiwyg': | ||
| case 'html': | ||
| return wp_kses_post( wp_unslash( $value ) ); | ||
|
|
||
| case 'url': | ||
| return esc_url_raw( wp_unslash( $value ) ); | ||
|
|
||
| case 'number': | ||
| return $this->sanitize_number( $value ); | ||
|
|
||
| default: | ||
| if ( is_array( $value ) ) { | ||
| return map_deep( wp_unslash( $value ), 'sanitize_text_field' ); | ||
| } | ||
|
|
||
| return sanitize_text_field( wp_unslash( $value ) ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Are WordPress REST API request parameters (WP_REST_Request::get_param) slashed by wp_magic_quotes, and should wp_unslash be applied to them?
💡 Result:
Yes. WordPress REST API request parameters retrieved via WP_REST_Request::get_param are expected to be unslashed by the REST server layer before your endpoint callback runs; therefore you generally should not apply wp_unslash again to values returned by get_param. The WP REST server explicitly unslashes the superglobals when building the WP_REST_Request object. In particular, it sets query parameters using wp_unslash($_GET) and body params using wp_unslash($_POST) (and also unslashes $_SERVER for headers) before dispatching to callbacks [1]. Since WP_REST_Request::get_param simply returns values from the already-populated internal $this->params array [2], the values you get are those post-unslash inputs, not the raw slashed superglobals. Also, WordPress core documentation/notes from the REST API “slashed data” work describe that data passed through the URL/query string is slashed in general PHP superglobal handling, and that the REST API infrastructure was changed so callbacks receive consistently unslashed data regardless of source (query string, form-encoded body, JSON body, etc.) [3]. So, should you apply wp_unslash to parameters you obtain via get_param? - No, not if you are getting the value via $request->get_param('key') inside a REST endpoint callback; the REST server already unslashes those inputs before callbacks run [1][2]. - Do apply wp_unslash only if you are directly reading from PHP superglobals (e.g., $_GET, $_POST) yourself inside your callback or elsewhere; REST-callback parameters are not expected to still be slashed [1][3].
Citations:
- 1: https://developer.wordpress.org/reference/classes/wp_rest_server/serve_request/
- 2: https://developer.wordpress.org/reference/classes/wp_rest_request/
- 3: https://make.wordpress.org/core/2016/04/06/rest-api-slashed-data-in-wordpress-4-4-and-4-5/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby context.
git ls-files includes/Api/Settings.php
wc -l includes/Api/Settings.php
sed -n '300,420p' includes/Api/Settings.phpRepository: weDevsOfficial/wp-user-frontend
Length of output: 4109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant context around the sanitize_value method.
git ls-files includes/Api/Settings.php
wc -l includes/Api/Settings.php
sed -n '320,390p' includes/Api/Settings.phpRepository: weDevsOfficial/wp-user-frontend
Length of output: 2530
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant context around the sanitize_value method.
git ls-files includes/Api/Settings.php
wc -l includes/Api/Settings.php
sed -n '320,390p' includes/Api/Settings.phpRepository: weDevsOfficial/wp-user-frontend
Length of output: 2530
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how sanitize_value is called and where its input comes from.
ast-grep outline includes/Api/Settings.php --view expanded
printf '\n--- references ---\n'
rg -n "sanitize_value\(|get_param\(|WP_REST_Request|register_rest_route|request" includes/Api/Settings.phpRepository: weDevsOfficial/wp-user-frontend
Length of output: 970
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how sanitize_value is called and where its input comes from.
ast-grep outline includes/Api/Settings.php --view expanded
printf '\n--- references ---\n'
rg -n "sanitize_value\(|get_param\(|WP_REST_Request|register_rest_route|request" includes/Api/Settings.phpRepository: weDevsOfficial/wp-user-frontend
Length of output: 970
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how sanitize_value is called and where its input comes from.
ast-grep outline includes/Api/Settings.php --view expanded
printf '\n--- references ---\n'
rg -n "sanitize_value\(|get_param\(|WP_REST_Request|register_rest_route|request" includes/Api/Settings.phpRepository: weDevsOfficial/wp-user-frontend
Length of output: 970
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how sanitize_value is called and where its input comes from.
ast-grep outline includes/Api/Settings.php --view expanded
printf '\n--- references ---\n'
rg -n "sanitize_value\(|get_param\(|WP_REST_Request|register_rest_route|request" includes/Api/Settings.phpRepository: weDevsOfficial/wp-user-frontend
Length of output: 970
Drop wp_unslash() from REST sanitization
sanitize_value() only receives values from WP_REST_Request::get_param(), so the extra wp_unslash() calls here are redundant and will strip literal backslashes from stored settings. Remove them in the fallback sanitizers to preserve user input. includes/Api/Settings.php:340-374
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@includes/Api/Settings.php` around lines 340 - 374, sanitize_value() is
over-unslashing REST input, which can strip legitimate backslashes from saved
settings. In includes/Api/Settings.php, update the sanitize_value method so the
REST-provided values are sanitized directly without wp_unslash() in the
type-specific branches and default fallback; keep the existing field-type
handling, sanitize_callback path, and helper methods like sanitize_number
unchanged.
Comment on lines
+71
to
+84
| #wpuf-settings-root input:focus, | ||
| #wpuf-settings-root input[type="text"]:focus, | ||
| #wpuf-settings-root input[type="search"]:focus, | ||
| #wpuf-settings-root input[type="email"]:focus, | ||
| #wpuf-settings-root input[type="url"]:focus, | ||
| #wpuf-settings-root input[type="number"]:focus, | ||
| #wpuf-settings-root input[type="password"]:focus, | ||
| #wpuf-settings-root textarea:focus, | ||
| #wpuf-settings-root select:focus { | ||
| border-color: #d1d5db !important; | ||
| box-shadow: none !important; | ||
| outline: 2px solid transparent !important; | ||
| outline-offset: 0 !important; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore a clearly visible focus indicator on standard controls.
Line 80 sets the focus border to #d1d5db while Lines 81-83 remove native/WP indicators, so keyboard focus can become effectively invisible on many fields.
Suggested fix
`#wpuf-settings-root` input:focus,
`#wpuf-settings-root` input[type="text"]:focus,
`#wpuf-settings-root` input[type="search"]:focus,
`#wpuf-settings-root` input[type="email"]:focus,
`#wpuf-settings-root` input[type="url"]:focus,
`#wpuf-settings-root` input[type="number"]:focus,
`#wpuf-settings-root` input[type="password"]:focus,
`#wpuf-settings-root` textarea:focus,
`#wpuf-settings-root` select:focus {
- border-color: `#d1d5db` !important;
- box-shadow: none !important;
+ border-color: `#3b82f6` !important;
+ box-shadow: 0 0 0 2px rgba(59, 130, 246, 0.35) !important;
outline: 2px solid transparent !important;
outline-offset: 0 !important;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #wpuf-settings-root input:focus, | |
| #wpuf-settings-root input[type="text"]:focus, | |
| #wpuf-settings-root input[type="search"]:focus, | |
| #wpuf-settings-root input[type="email"]:focus, | |
| #wpuf-settings-root input[type="url"]:focus, | |
| #wpuf-settings-root input[type="number"]:focus, | |
| #wpuf-settings-root input[type="password"]:focus, | |
| #wpuf-settings-root textarea:focus, | |
| #wpuf-settings-root select:focus { | |
| border-color: #d1d5db !important; | |
| box-shadow: none !important; | |
| outline: 2px solid transparent !important; | |
| outline-offset: 0 !important; | |
| } | |
| `#wpuf-settings-root` input:focus, | |
| `#wpuf-settings-root` input[type="text"]:focus, | |
| `#wpuf-settings-root` input[type="search"]:focus, | |
| `#wpuf-settings-root` input[type="email"]:focus, | |
| `#wpuf-settings-root` input[type="url"]:focus, | |
| `#wpuf-settings-root` input[type="number"]:focus, | |
| `#wpuf-settings-root` input[type="password"]:focus, | |
| `#wpuf-settings-root` textarea:focus, | |
| `#wpuf-settings-root` select:focus { | |
| border-color: `#3b82f6` !important; | |
| box-shadow: 0 0 0 2px rgba(59, 130, 246, 0.35) !important; | |
| outline: 2px solid transparent !important; | |
| outline-offset: 0 !important; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/css/settings.css` around lines 71 - 84, Restore a visible keyboard focus
state for the standard controls in the `#wpuf-settings-root` focus rule by
revising the input/textarea/select styling in settings.css. The current selector
group (`#wpuf-settings-root` input:focus, input[type="text"]:focus,
textarea:focus, select:focus) removes the native/WP focus cues with outline and
box-shadow overrides while using a low-contrast border, so update this rule to
keep or add a clearly visible focus indicator (for example a stronger outline or
box-shadow) instead of suppressing it.
Comment on lines
+35
to
+40
| const iconOf = ( opt, key ) => { | ||
| if ( opt && typeof opt === 'object' && typeof opt.icon === 'string' && opt.icon ) { | ||
| return opt.icon; | ||
| } | ||
| return GATEWAY_SVG[ key ] || ''; | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent raw HTML injection from opt.icon values.
Line 36 accepts arbitrary icon strings, and Lines 107 and 144 inject them with dangerouslySetInnerHTML. If a schema/filter supplies untrusted icon HTML, this becomes admin-side XSS.
Suggested fix
const iconOf = ( opt, key ) => {
- if ( opt && typeof opt === 'object' && typeof opt.icon === 'string' && opt.icon ) {
- return opt.icon;
+ if (
+ opt &&
+ typeof opt === 'object' &&
+ typeof opt.icon === 'string' &&
+ /^https?:\/\//i.test( opt.icon )
+ ) {
+ return opt.icon;
}
return GATEWAY_SVG[ key ] || '';
};Also applies to: 103-107, 138-145
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/js/components/settings/kit/fields/RadioCardsField.jsx` around lines 35 -
40, The icon handling in iconOf and the renders that use dangerouslySetInnerHTML
are passing through arbitrary opt.icon HTML, which can allow admin-side XSS.
Update RadioCardsField so opt.icon is treated as untrusted input: either
restrict it to a safe identifier/whitelisted SVG source or sanitize it before it
reaches the places that render the icon. Make sure the values used in the icon
render paths are derived from trusted mappings like GATEWAY_SVG rather than raw
schema/filter-provided HTML.
Source: Linters/SAST tools
Comment on lines
+47
to
+52
| const selected = single | ||
| ? ( value ? [ value ] : ( field.default ? [ field.default ] : [] ) ) | ||
| : Array.isArray( value ) | ||
| ? value | ||
| : ( value && typeof value === 'object' ? Object.keys( value ) : ( field.default || [] ) ); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Normalize field.default before using array operations.
Line 51 can set selected to an object when field.default is associative. Line 86 then calls selected.includes, which will throw at runtime.
Suggested fix
export default function RadioCardsField( { field, name, value, onChange, single = false } ) {
const options = field.options || {};
+ const defaultSelected = Array.isArray( field.default )
+ ? field.default
+ : ( field.default && typeof field.default === 'object' ? Object.keys( field.default ) : [] );
// Single-select stores a string; multi-select stores the legacy associative
// shape { slug: slug }. Derive the selected key list for either case.
const selected = single
? ( value ? [ value ] : ( field.default ? [ field.default ] : [] ) )
: Array.isArray( value )
? value
- : ( value && typeof value === 'object' ? Object.keys( value ) : ( field.default || [] ) );
+ : ( value && typeof value === 'object' ? Object.keys( value ) : defaultSelected );Also applies to: 86-86
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/js/components/settings/kit/fields/RadioCardsField.jsx` around lines 47 -
52, Normalize field.default into an array before assigning selected in
RadioCardsField so selected is never an object when the field uses an
associative default. Update the selected initialization in RadioCardsField to
convert any object default to its keys, matching the existing object-handling
path for value, so later uses like selected.includes remain safe. Keep the fix
localized to the selected derivation logic and verify the single-choice branch
still returns an array.
Comment on lines
+42
to
+75
| { rows.map( ( row, i ) => ( | ||
| <div key={ i } className="wpuf-mb-3 wpuf-rounded-lg wpuf-border wpuf-border-gray-200 wpuf-p-4"> | ||
| <div className="wpuf-mb-2 wpuf-flex wpuf-items-center wpuf-justify-between"> | ||
| <span className="wpuf-text-sm wpuf-font-semibold wpuf-text-gray-900"> | ||
| { __( 'Role-specific Template', 'wp-user-frontend' ) } #{ i + 1 } | ||
| </span> | ||
| <button | ||
| type="button" | ||
| onClick={ () => remove( i ) } | ||
| className="wpuf-text-sm wpuf-font-medium wpuf-text-red-600 hover:wpuf-text-red-700" | ||
| > | ||
| { __( 'Remove', 'wp-user-frontend' ) } | ||
| </button> | ||
| </div> | ||
| <MultiSelectChips | ||
| field={ { label: __( 'Roles', 'wp-user-frontend' ), options: roles } } | ||
| name={ `roles_${ i }` } | ||
| value={ row.roles || [] } | ||
| onChange={ ( n, val ) => update( i, { roles: val } ) } | ||
| /> | ||
| <TextField | ||
| field={ { label: __( 'Subject', 'wp-user-frontend' ) } } | ||
| name={ `subject_${ i }` } | ||
| value={ row.subject || '' } | ||
| onChange={ ( n, val ) => update( i, { subject: val } ) } | ||
| /> | ||
| <WysiwygField | ||
| field={ { label: __( 'Body', 'wp-user-frontend' ) } } | ||
| name={ `wpuf_role_tpl_${ emailType }_${ i }` } | ||
| value={ row.body || '' } | ||
| onChange={ ( n, val ) => update( i, { body: val } ) } | ||
| /> | ||
| </div> | ||
| ) ) } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use stable row keys, not the array index.
Each row renders a WysiwygField whose TextEditor mounts a TinyMCE instance keyed by name (wpuf_role_tpl_${emailType}_${i}) and initializes from defaultValue only once. When a non-last row is removed (Line 38), the trailing rows reindex while React—using key={i}—reuses the existing component instances. The instances then receive a different name/editor id and shifted value/subject/roles, leading to editor/content mismatch and orphaned TinyMCE instances. Give each row a stable id at creation and key on it.
♻️ Suggested approach
- const add = () => commit( [ ...rows, { roles: [], subject: '', body: '' } ] );
+ const add = () => commit( [ ...rows, { id: `tpl_${ Date.now() }_${ rows.length }`, roles: [], subject: '', body: '' } ] );- <div key={ i } className="wpuf-mb-3 wpuf-rounded-lg wpuf-border wpuf-border-gray-200 wpuf-p-4">
+ <div key={ row.id || i } className="wpuf-mb-3 wpuf-rounded-lg wpuf-border wpuf-border-gray-200 wpuf-p-4">🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 43-54: A list component should have a key to prevent re-rendering
Context:
{ __( 'Role-specific Template', 'wp-user-frontend' ) } #{ i + 1 }
<button
type="button"
onClick={ () => remove( i ) }
className="wpuf-text-sm wpuf-font-medium wpuf-text-red-600 hover:wpuf-text-red-700"
>
{ __( 'Remove', 'wp-user-frontend' ) }
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 44-46: A list component should have a key to prevent re-rendering
Context:
{ __( 'Role-specific Template', 'wp-user-frontend' ) } #{ i + 1 }
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 47-53: A list component should have a key to prevent re-rendering
Context: <button
type="button"
onClick={ () => remove( i ) }
className="wpuf-text-sm wpuf-font-medium wpuf-text-red-600 hover:wpuf-text-red-700"
>
{ __( 'Remove', 'wp-user-frontend' ) }
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 55-60: A list component should have a key to prevent re-rendering
Context: <MultiSelectChips
field={ { label: __( 'Roles', 'wp-user-frontend' ), options: roles } }
name={ roles_${ i } }
value={ row.roles || [] }
onChange={ ( n, val ) => update( i, { roles: val } ) }
/>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 61-66: A list component should have a key to prevent re-rendering
Context: <TextField
field={ { label: __( 'Subject', 'wp-user-frontend' ) } }
name={ subject_${ i } }
value={ row.subject || '' }
onChange={ ( n, val ) => update( i, { subject: val } ) }
/>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 67-72: A list component should have a key to prevent re-rendering
Context: <WysiwygField
field={ { label: __( 'Body', 'wp-user-frontend' ) } }
name={ wpuf_role_tpl_${ emailType }_${ i } }
value={ row.body || '' }
onChange={ ( n, val ) => update( i, { body: val } ) }
/>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/js/components/settings/kit/fields/RoleEmailTemplates.jsx` around lines 42
- 75, The row rendering in RoleEmailTemplates.jsx is using the array index as
the React key, which causes WysiwygField/TextEditor instances to be reused with
shifted data after removals. Update the rows managed by the add/update/remove
flow to carry a stable unique id per row and use that id for the key in the
rows.map render, while keeping the existing name props aligned to the same row
identity.
Source: Linters/SAST tools
| <input | ||
| type="text" | ||
| id={ name } | ||
| value={ value || field.default || '' } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Field can't be cleared when a non-empty field.default exists.
value || field.default || '' treats an intentionally-empty value ('') as falsy and falls back to field.default. After a user clears the input, onChange(name, '') updates the store, but on re-render the box snaps back to the default and becomes effectively un-clearable. TextareaField (Line 26) already handles this correctly with an explicit null/undefined check; mirror that here.
🐛 Proposed fix
- value={ value || field.default || '' }
+ value={ value !== undefined && value !== null ? value : ( field.default || '' ) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| value={ value || field.default || '' } | |
| value={ value !== undefined && value !== null ? value : ( field.default || '' ) } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/js/components/settings/kit/fields/TextField.jsx` at line 35, The
TextField value fallback is treating an intentionally empty string as missing,
so a cleared field snaps back to field.default and cannot stay empty. Update the
value handling in TextField to mirror TextareaField by checking only for
null/undefined instead of using the truthy chain, so value={ value ||
field.default || '' } preserves ''. Use the TextField component’s value prop
logic and field.default as the key symbols to fix.
Comment on lines
+66
to
+82
| <span | ||
| role="checkbox" | ||
| aria-checked={ checked } | ||
| title={ checkTitle } | ||
| onClick={ ( e ) => { | ||
| e.stopPropagation(); | ||
| if ( onToggle ) { | ||
| onToggle(); | ||
| } | ||
| } } | ||
| className={ `wpuf-absolute wpuf-right-1.5 wpuf-top-1.5 wpuf-flex wpuf-h-4 wpuf-w-4 wpuf-items-center wpuf-justify-center wpuf-rounded wpuf-border ${ | ||
| onToggle ? 'wpuf-cursor-pointer' : 'wpuf-cursor-default' | ||
| } ${ checked ? 'wpuf-border-primary wpuf-bg-primary wpuf-text-white' : 'wpuf-border-gray-300 wpuf-bg-white' }` } | ||
| > | ||
| { checked && <Check /> } | ||
| </span> | ||
| ) } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Cloudflare enable toggle isn't keyboard operable.
The corner span role="checkbox" is the only control that writes enable_turnstile, but it has no tabIndex or onKeyDown, so keyboard/AT users cannot focus or toggle it. Same root cause as the payment gateway cards — see the consolidated note in PaymentGateways.jsx.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/js/components/settings/kit/GeneralSettings.jsx` around lines 66 - 82, The
enable toggle in GeneralSettings is implemented with a span acting as a
checkbox, but it is not keyboard accessible. Update the span used for the
Turnstile/Cloudflare toggle to be focusable and operable via keyboard by adding
the appropriate tab index and keyboard handler, and make it trigger the same
toggle logic as onClick when activated with Enter or Space. Use the existing
checked, onToggle, and Check behavior as the anchor points so the control
remains consistent with the payment gateway card fix.
Comment on lines
+124
to
+140
| <span | ||
| role="checkbox" | ||
| aria-checked={ isEnabled } | ||
| onClick={ ( e ) => { | ||
| e.stopPropagation(); | ||
| toggleEnable( gid ); | ||
| } } | ||
| className={ `wpuf-absolute wpuf-right-2 wpuf-top-2 wpuf-flex wpuf-h-4 wpuf-w-4 wpuf-cursor-pointer wpuf-items-center wpuf-justify-center wpuf-rounded wpuf-border ${ | ||
| isEnabled ? 'wpuf-border-primary wpuf-bg-primary wpuf-text-white' : 'wpuf-border-gray-300 wpuf-bg-white' | ||
| }` } | ||
| > | ||
| { isEnabled && ( | ||
| <svg className="wpuf-h-3 wpuf-w-3" viewBox="0 0 20 20" fill="currentColor"> | ||
| <path fillRule="evenodd" d="M16.7 5.3a1 1 0 010 1.4l-8 8a1 1 0 01-1.4 0l-4-4a1 1 0 011.4-1.4l3.3 3.3 7.3-7.3a1 1 0 011.4 0z" clipRule="evenodd" /> | ||
| </svg> | ||
| ) } | ||
| </span> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Gateway enable checkbox is not keyboard accessible.
The corner span role="checkbox" is the sole control to enable/disable a gateway (toggleEnable), but it isn't focusable (no tabIndex) and has no onKeyDown, so keyboard-only users can view a gateway via the parent button yet can never enable/disable it. Make it operable via Enter/Space.
♿ Proposed fix
<span
role="checkbox"
+ tabIndex={ 0 }
aria-checked={ isEnabled }
onClick={ ( e ) => {
e.stopPropagation();
toggleEnable( gid );
} }
+ onKeyDown={ ( e ) => {
+ if ( e.key === 'Enter' || e.key === ' ' ) {
+ e.preventDefault();
+ e.stopPropagation();
+ toggleEnable( gid );
+ }
+ } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <span | |
| role="checkbox" | |
| aria-checked={ isEnabled } | |
| onClick={ ( e ) => { | |
| e.stopPropagation(); | |
| toggleEnable( gid ); | |
| } } | |
| className={ `wpuf-absolute wpuf-right-2 wpuf-top-2 wpuf-flex wpuf-h-4 wpuf-w-4 wpuf-cursor-pointer wpuf-items-center wpuf-justify-center wpuf-rounded wpuf-border ${ | |
| isEnabled ? 'wpuf-border-primary wpuf-bg-primary wpuf-text-white' : 'wpuf-border-gray-300 wpuf-bg-white' | |
| }` } | |
| > | |
| { isEnabled && ( | |
| <svg className="wpuf-h-3 wpuf-w-3" viewBox="0 0 20 20" fill="currentColor"> | |
| <path fillRule="evenodd" d="M16.7 5.3a1 1 0 010 1.4l-8 8a1 1 0 01-1.4 0l-4-4a1 1 0 011.4-1.4l3.3 3.3 7.3-7.3a1 1 0 011.4 0z" clipRule="evenodd" /> | |
| </svg> | |
| ) } | |
| </span> | |
| <span | |
| role="checkbox" | |
| tabIndex={ 0 } | |
| aria-checked={ isEnabled } | |
| onClick={ ( e ) => { | |
| e.stopPropagation(); | |
| toggleEnable( gid ); | |
| } } | |
| onKeyDown={ ( e ) => { | |
| if ( e.key === 'Enter' || e.key === ' ' ) { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| toggleEnable( gid ); | |
| } | |
| } } | |
| className={ `wpuf-absolute wpuf-right-2 wpuf-top-2 wpuf-flex wpuf-h-4 wpuf-w-4 wpuf-cursor-pointer wpuf-items-center wpuf-justify-center wpuf-rounded wpuf-border ${ | |
| isEnabled ? 'wpuf-border-primary wpuf-bg-primary wpuf-text-white' : 'wpuf-border-gray-300 wpuf-bg-white' | |
| }` } | |
| > | |
| { isEnabled && ( | |
| <svg className="wpuf-h-3 wpuf-w-3" viewBox="0 0 20 20" fill="currentColor"> | |
| <path fillRule="evenodd" d="M16.7 5.3a1 1 0 010 1.4l-8 8a1 1 0 01-1.4 0l-4-4a1 1 0 011.4-1.4l3.3 3.3 7.3-7.3a1 1 0 011.4 0z" clipRule="evenodd" /> | |
| </svg> | |
| ) } | |
| </span> |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 135-137: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 136-136: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/js/components/settings/kit/PaymentGateways.jsx` around lines 124 - 140,
The gateway enable control in PaymentGateways is only clickable with a mouse and
cannot be used from the keyboard. Update the corner span with role="checkbox" so
it is focusable and operable via Enter/Space by adding keyboard handling and a
tab stop, and keep it wired to toggleEnable(gid) with the current isEnabled
state reflected correctly. Ensure the span’s accessible interaction matches the
parent gateway item’s behavior so keyboard-only users can enable or disable a
gateway.
Comment on lines
+10
to
+14
| export function getSections( state ) { | ||
| if ( state.sections && state.sections.length > 0 ) { | ||
| return; | ||
| } | ||
| return loadSettings(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
In @wordpress/datacreateReduxStore resolvers, does a resolver receivestate as its first argument, or only selector arguments?
💡 Result:
In the @wordpress/data package, a resolver does not receive the state as its first argument; instead, it receives the same arguments that were passed to the corresponding selector [1]. According to the official WordPress Block Editor Handbook, the resolver function is passed the same arguments used in the selector call, excluding the state argument [1]. This is distinct from selectors themselves, which do receive the state as their first argument [1]. While some secondary sources or community discussions may occasionally contain contradictory or confusing examples [2], the official documentation explicitly states that resolvers receive only the arguments passed to the selector [1]. Methods within a resolver, such as isFulfilled or shouldInvalidate, may receive the state as an argument (e.g., isFulfilled: ( state, ...args ) => ...) [1], but the primary resolver function (the fulfill method or the plain resolver function) does not receive the state [1].
Citations:
- 1: https://developer.wordpress.org/block-editor/reference-guides/packages/packages-data/
- 2: Is there a better way to to make async API calls using the @wordpress/data module? WordPress/gutenberg#41537
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and related store definitions.
git ls-files 'src/js/stores-react/settings/*' 'src/js/stores-react/**/*settings*' | sed -n '1,120p'
printf '\n--- resolvers.js ---\n'
cat -n src/js/stores-react/settings/resolvers.js
printf '\n--- search store usage ---\n'
rg -n "getSections|createReduxStore|resolvers|selectors" src/js/stores-react -SRepository: weDevsOfficial/wp-user-frontend
Length of output: 1413
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and related store definitions.
git ls-files 'src/js/stores-react/settings/*' 'src/js/stores-react/**/*settings*' | sed -n '1,120p'
printf '\n--- resolvers.js ---\n'
cat -n src/js/stores-react/settings/resolvers.js
printf '\n--- search store usage ---\n'
rg -n "getSections|createReduxStore|resolvers|selectors" src/js/stores-react -SRepository: weDevsOfficial/wp-user-frontend
Length of output: 1413
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target resolver and its selector contract.
cat -n src/js/stores-react/settings/resolvers.js
printf '\n--- selectors.js ---\n'
cat -n src/js/stores-react/settings/selectors.js
printf '\n--- store definitions ---\n'
rg -n "createReduxStore\\(|registerStore\\(|resolvers:|selectors:" src/js/stores-react/settings -SRepository: weDevsOfficial/wp-user-frontend
Length of output: 2640
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the small relevant files and inspect the resolver shape.
printf '\n--- resolvers.js ---\n'
cat -n src/js/stores-react/settings/resolvers.js
printf '\n--- selectors.js ---\n'
cat -n src/js/stores-react/settings/selectors.js
printf '\n--- store registration references ---\n'
rg -n "createReduxStore|registerStore|resolvers\\s*:|selectors\\s*:" src/js/stores-react/settings -SRepository: weDevsOfficial/wp-user-frontend
Length of output: 2763
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/js/stores-react/settings/index.js
printf '\n--- reducer.js ---\n'
cat -n src/js/stores-react/settings/reducer.js
printf '\n--- actions.js ---\n'
cat -n src/js/stores-react/settings/actions.js
printf '\n--- any custom data wrappers ---\n'
rg -n "registerStore\\(|createReduxStore\\(|`@wordpress/data`" src/js/stores-react src/js -SRepository: weDevsOfficial/wp-user-frontend
Length of output: 10201
Don't dereference state in this resolver. Resolvers receive selector arguments, not store state, so state is undefined here and state.sections can throw. Move the guard to the selector or make this resolver load unconditionally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/js/stores-react/settings/resolvers.js` around lines 10 - 14, The
getSections resolver is incorrectly treating its argument as store state, which
can be undefined and make state.sections throw. Update getSections to either
load settings unconditionally, or move the sections existence check into the
selector that calls it; use the getSections symbol to locate the resolver and
avoid dereferencing state inside the resolver.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/pages/settingsReact.ts`:
- Around line 52-55: `fieldByLabel` is too narrowly scoped to `<input>`, which
breaks label-based lookup for other form controls. Update the locator logic in
`SettingsReact.fieldByLabel` so it can return the field associated with the
label regardless of whether it is an input or textarea-style control, while
keeping the same label-matching behavior rooted at `#wpuf-settings-root`.
In `@tests/e2e/tests/settingsReactTest.spec.ts`:
- Line 113: The settingsReactTest spec is swallowing failures by chaining
.catch(() => {}) onto test actions, which can hide real regressions. Remove the
catch blocks from the affected settings.fillByLabel calls and let the
action/assertion failures surface naturally so the test fails when the UI
behavior is broken.
- Around line 68-69: The spec is using hardcoded selectors instead of the
SettingsReactPage page object, which breaks the POM contract and duplicates
locator logic. Move the selector/visibility checks into SettingsReactPage (or
reuse an existing method there) and update settingsReactTest.spec.ts to call
page object methods only; use SettingsReactPage as the single place for these
locators across all affected assertions.
- Around line 85-101: The Social Login and Facebook-provider assertions in
settingsReactTest.spec are Pro-only and must be gated so Lite-only runs don’t
fail. Update the affected tests in the existing settings test suite (including
the Social Login sub-tab case and the facebook provider search case) to
conditionally skip or short-circuit when Pro features are unavailable, using the
same pattern already used for other Pro-dependent e2e specs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a1c41bfd-5e5d-4721-b64f-a41bb9390f32
📒 Files selected for processing (4)
tests/e2e/pages/selectors.tstests/e2e/pages/settingsReact.tstests/e2e/playwright.parallel-two.config.tstests/e2e/tests/settingsReactTest.spec.ts
✅ Files skipped from review due to trivial changes (1)
- tests/e2e/pages/selectors.ts
Comment on lines
+68
to
+69
| await expect(page.locator('#wpuf-settings-root nav.wpuf-space-y-1 button', { hasText: tab })).toBeVisible(); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Move direct selectors out of the spec and into the Page Object.
These lines bypass SettingsReactPage and hardcode selectors in the spec, which breaks the POM contract and duplicates locator logic.
As per coding guidelines, tests/e2e/pages/**/*.ts: “Use the Page Object Model pattern for page interactions in pages/ directory — do not put selectors directly in test specs” and “Reuse existing Page Object methods before adding new ones in the Page Object Model.”
Also applies to: 74-75, 89-90, 107-108, 114-114, 160-160, 167-167, 180-180, 183-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/tests/settingsReactTest.spec.ts` around lines 68 - 69, The spec is
using hardcoded selectors instead of the SettingsReactPage page object, which
breaks the POM contract and duplicates locator logic. Move the
selector/visibility checks into SettingsReactPage (or reuse an existing method
there) and update settingsReactTest.spec.ts to call page object methods only;
use SettingsReactPage as the single place for these locators across all affected
assertions.
Source: Coding guidelines
| test('@Test_SR0008 : Editing a field marks dirty', async () => { | ||
| await settings.goto(); | ||
| await settings.openTab('General'); | ||
| await settings.fillByLabel('Custom CSS codes', `/* ${faker.string.uuid()} */`).catch(() => {}); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t swallow assertion/action failures in tests.
Line 113 and Line 136 use .catch(() => {}), which can mask real regressions and produce false positives.
Suggested fix
- await settings.fillByLabel('Custom CSS codes', `/* ${faker.string.uuid()} */`).catch(() => {});
+ await settings.fillByLabel('Custom CSS codes', `/* ${faker.string.uuid()} */`);
...
- await settings.save().catch(() => {});
+ await settings.save();Also applies to: 136-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/tests/settingsReactTest.spec.ts` at line 113, The settingsReactTest
spec is swallowing failures by chaining .catch(() => {}) onto test actions,
which can hide real regressions. Remove the catch blocks from the affected
settings.fillByLabel calls and let the action/assertion failures surface
naturally so the test fails when the UI behavior is broken.
Validated the settings spec end-to-end on an isolated SQLite WP site: - panelTitle → the gray-900 h2 (the other h2 is the app header). - unsavedModalTitle → the modal h1 by id (text=Unsaved Changes also matched the footer badge + body copy → strict-mode violation). - fieldByLabel → match input OR textarea (custom_css is a textarea). - SR0005/06 → free "My Account" sub-tab so they run without a Pro license. - SR0016 (Facebook provider) → gated: skips when the Social Login Pro module is inactive (per tests/e2e/CLAUDE.md), instead of failing. - Spec context uses ignoreHTTPSErrors. Result: 16 passed, 1 skipped (Pro-gated), ~21s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a "Pro Tests" describe (SR0024–0030), each GATED so it skips on Lite/no license and runs on a Pro+licensed site (per tests/e2e/CLAUDE.md): - SR0024 Social Login provider cards (Facebook/Google) - SR0025 Tax UI (Enable Tax + Base Country + Add Rate) - SR0026 Invoices section - SR0027 SMS provider cards (Twilio) - SR0028 AI Settings provider cards + model field - SR0029 Pro email notification templates - SR0030 Pro field save round-trip (Tax) Verified end-to-end on an isolated SQLite site (license-valid option + active modules): 17 free + 7 Pro = 24 passed, ~22s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/tests/settingsReactTest.spec.ts (1)
168-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
gateOnProfor the Facebook gate.This inline
waitFor(...).then(...).catch(...)+test.skip(...)is the exact logic already extracted intogateOnPro(Lines 212-216). Function declarations are hoisted, so it's usable here. Reusing it removes the duplicated locator-gating pattern.♻️ Proposed refactor
- // Facebook lives in the Social Login Pro module — gate so a Lite/no-license - // run skips instead of failing (per tests/e2e/CLAUDE.md). - const hasFacebook = await page.getByText('Facebook', { exact: false }).first() - .waitFor({ state: 'visible', timeout: 4000 }).then(() => true).catch(() => false); - test.skip(! hasFacebook, 'Social Login (Pro module) inactive — provider fields unavailable'); + // Facebook lives in the Social Login Pro module — gate so a Lite/no-license + // run skips instead of failing (per tests/e2e/CLAUDE.md). + await gateOnPro(page.getByText('Facebook', { exact: false }), 'Social Login (Pro module) inactive — provider fields unavailable');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/tests/settingsReactTest.spec.ts` around lines 168 - 172, The Facebook availability check duplicates the locator-gating logic already encapsulated in gateOnPro; replace the inline waitFor(...).then(...).catch(...) plus test.skip(...) in settingsReactTest.spec.ts with a call to gateOnPro for the Facebook text locator. Since gateOnPro is function-hoisted and already used for other gated providers, reuse it here to keep the gating pattern consistent and avoid duplicated code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/tests/settingsReactTest.spec.ts`:
- Around line 286-298: `@Test_SR0030` currently only checks that the Tax sub-tab
UI renders after reload, which does not verify persistence. Update the test in
settingsReactTest.spec.ts to assert the actual saved state after
`settings.save()` and `settings.goto()`, using the `Enable Tax` checkbox state
or the `Base Country` value via the existing `settings.openSubTab('Tax')` flow.
Also make the test idempotent by capturing the prior tax setting before toggling
and restoring it at the end so repeated runs under `configureSpecFailFast` do
not depend on mutated persisted state.
---
Nitpick comments:
In `@tests/e2e/tests/settingsReactTest.spec.ts`:
- Around line 168-172: The Facebook availability check duplicates the
locator-gating logic already encapsulated in gateOnPro; replace the inline
waitFor(...).then(...).catch(...) plus test.skip(...) in
settingsReactTest.spec.ts with a call to gateOnPro for the Facebook text
locator. Since gateOnPro is function-hoisted and already used for other gated
providers, reuse it here to keep the gating pattern consistent and avoid
duplicated code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 575b7283-0f25-4703-a627-4d08bb8f0f27
📒 Files selected for processing (4)
tests/e2e/.gitignoretests/e2e/pages/selectors.tstests/e2e/pages/settingsReact.tstests/e2e/tests/settingsReactTest.spec.ts
✅ Files skipped from review due to trivial changes (1)
- tests/e2e/.gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/pages/selectors.ts
- tests/e2e/pages/settingsReact.ts
…SR0020 - SR0012: footer Cancel reverts unsaved edits (POM cancel()). - SR0031: the 11 Login Form Colors (Pro) render their color pickers under the "Login Form Colors" accordion on the Login/Registration sub-tab — verifies the color data loads + renders end-to-end (REST returns them deduped, type=color). - Drop SR0020 depends_on E2E: "Enable Turnstile" sits behind the General security card's reCAPTCHA↔Cloudflare switcher (too brittle to assert). The depends_on logic is covered by SettingsSection's valueMatches. Full run on the licensed SQLite site: 26 passed, ~23s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The login_form_settings_section field (type=html + a PHP callback that emits classic-screen table markup) fell to the default ProNote in React, so the "Login Form Customization" heading was missing on the Login/Registration tab. Map the callback to HtmlField and give the field a clean `desc` (heading + description) that React renders. Legacy ignores `desc` for callback fields, so the classic screen still uses its callback unchanged — no behavior change there. Verified: header renders in React; full settings E2E 26/26 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SR0030 now captures the prior Enable Tax state, asserts the toggled value round-trips across reload, and restores the original so reruns stay idempotent. SR0016 drops the duplicated inline locator gate in favor of the hoisted gateOnPro helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Replaces the legacy
WeDevs_Settings_APIsettings screen with a React app served overGET/POST wpuf/v1/settings.Highlights
wpuf_settings_sections()/wpuf_settings_fields()produce (free + Pro + modules), dispatched by fieldtype.depends_onconditional fields, global + provider-aware search, loading skeleton, save confirmation, error modal.wpuf_settings_rest_data/wpuf_settings_savedextraside-channel.wpuf_settings_ui_modeoption (+?wpuf_settings_ui=override,WPUF_LEGACY_SETTINGSconstant). Both screens share storage.docs/settings-react.md.Notes
npm run build:settings-react).Issue: weDevsOfficial/wpuf-pro#1569
Pro counterpart: weDevsOfficial/wpuf-pro#1570
🤖 Generated with Claude Code
Summary by CodeRabbit