Search 3.0: dashboard feature-selection UI (RSM-2116)#48500
Merged
adamwoodnz merged 55 commits intotrunkfrom May 6, 2026
Merged
Search 3.0: dashboard feature-selection UI (RSM-2116)#48500adamwoodnz merged 55 commits intotrunkfrom
adamwoodnz merged 55 commits intotrunkfrom
Conversation
adamwoodnz
added
[Status] Needs Review
github-actions
Bot
added
[Package] Search
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
adamwoodnz
added
[Status] In Progress
and removed
[Status] Needs Review
Code Coverage SummaryCoverage changed in 22 files. Only the first 5 are listed here.
3 files are newly checked for coverage.
Full summary · PHP report · JS report Coverage check overridden by
I don't care about code coverage for this PR
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a Search 3.0–gated “feature-selection” control on the Jetpack Search dashboard, replacing the legacy two-toggle UI with a single radio-list experience picker and a Save action, and wires the feature flag through the dashboard’s initial state/store.
Changes:
- Add a new
<FeatureSelector>(radio list + Save) and supporting components/styles (including a new<Badge>). - Extend the dashboard store with “experience” selectors/actions and a
searchBlocksEnabledinitial-state flag. - Add JS + PHP tests for the new store behavior and UI branching, plus a Search package changelog entry.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/packages/search/src/dashboard/class-initial-state.php | Exposes searchBlocksEnabled (mirrors jetpack_search_blocks_enabled) to the React initial state. |
| projects/packages/search/src/dashboard/store/selectors/site-data.js | Adds selector for the new searchBlocksEnabled initial-state flag. |
| projects/packages/search/src/dashboard/store/selectors/jetpack-settings.js | Adds experience derivation + new “pending/selected/active/dirty” selectors. |
| projects/packages/search/src/dashboard/store/actions/jetpack-settings.js | Adds saveExperience + pending/last-saved experience actions and analytics event. |
| projects/packages/search/src/dashboard/components/pages/dashboard-page.jsx | Switches bottom controls to <FeatureSelector> when searchBlocksEnabled is true. |
| projects/packages/search/src/dashboard/components/feature-selector/index.jsx | New feature-selection form (fieldset of radio rows + Save). |
| projects/packages/search/src/dashboard/components/feature-selector/experience-option.jsx | New radio-row component with badges + plan-gating disabled state. |
| projects/packages/search/src/dashboard/components/feature-selector/constants.js | Defines experience IDs, order, labels/descriptions, and icons. |
| projects/packages/search/src/dashboard/components/feature-selector/style.scss | WPDS-token-based styling for the selector card and rows. |
| projects/packages/search/src/dashboard/components/badge/index.jsx | New local Badge primitive for “Recommended”/“Active” labels. |
| projects/packages/search/src/dashboard/components/badge/style.scss | Badge styling aligned to WPDS tokens. |
| projects/packages/search/tests/php/Initial_State_Test.php | PHPUnit coverage for the new searchBlocksEnabled initial-state field. |
| projects/packages/search/tests/js/dashboard/store/jetpack-settings.test.js | Jest coverage for new selectors/actions and saveExperience behavior. |
| projects/packages/search/tests/js/dashboard/pages/dashboard-page.test.jsx | Jest coverage for FeatureSelector vs ModuleControl branching. |
| projects/packages/search/tests/js/dashboard/feature-selector/index.test.jsx | Jest coverage for FeatureSelector rendering/disabled-save/plan gating. |
| projects/packages/search/tests/js/dashboard/feature-selector/experience-option.test.jsx | Jest coverage for ExperienceOption badges, checked state, and disabled rows. |
| projects/packages/search/changelog/rsm-2116-search-dashboard-feature-selection-ui | Changelog entry for the new dashboard UI (gated). |
adamwoodnz
added a commit
that referenced
this pull request
May 5, 2026
`updateJetpackSettings` catches its own errors and returns an
error-notice action instead of throwing, which meant `saveExperience`
was unconditionally calling `setLastSavedExperience` and clearing
`pending_experience` even when the API call failed — moving the
Active badge to the failed selection and removing the user's pending
choice so they couldn't retry without re-clicking.
Capture the inner generator's return value and gate the promotion
on `notice.status === 'is-success'`. On failure we exit early and
pending stays put, so the user can retry. Added a test for the
failure path; updated the existing success-path test to use the
real notice shape rather than an `{ ok: true }` placeholder.
Caught by Copilot review on PR #48500.
adamwoodnz
added a commit
that referenced
this pull request
May 5, 2026
Leftover from when the heading was a visually-hidden legend with a separate visible label. No JSX renders an element with this class anymore, so the rule was dead CSS. Caught by Copilot review on PR #48500.
adamwoodnz
commented
May 5, 2026
Comment on lines
+174
to
+176
| <div className="jp-search-dashboard-wrap jp-search-feature-selector-wrap"> | ||
| <div className="jp-search-dashboard-row"> | ||
| <div className="lg-col-span-12 md-col-span-8 sm-col-span-4"> |
Contributor
Author
There was a problem hiding this comment.
This can all be removed when we move to the tabs
Contributor
Author
|
@claude review please |
Contributor
|
Claude finished @adamwoodnz's task in 3m 55s —— View job Review
Overall this is a well-structured, carefully-gated feature with good separation of concerns and solid test coverage. No blockers found. A few things worth discussing before merge: Store — actions (
|
kangzj
reviewed
May 5, 2026
| export const EXPERIENCE = Object.freeze( { | ||
| EMBEDDED: 'embedded', | ||
| OVERLAY: 'overlay', | ||
| CLASSIC: 'classic', |
Contributor
There was a problem hiding this comment.
we are retiring Classic search if I'm not mistaken. The experience should be 'inline' search? @gibrown
Contributor
Author
There was a problem hiding this comment.
This string corresponds to the 'Your theme' option. So Jetpack Search enabled but not using overlay or blocks. I used the classic term because it was already used in the code, but if that's confusing perhaps we could change it to theme, or inline.
Contributor
Author
There was a problem hiding this comment.
@kangzj is this decision blocking? Keen to merge and get the backend going too. Happy to switch to inline if you think that's best.
Contributor
Author
There was a problem hiding this comment.
I can see other references to classic (eg. is_swap_classic_to_inline_search) so I've renamed it to inline in 60a349e
kangzj
reviewed
May 5, 2026
| * dashboard React app can gate the new feature-selection UI on the | ||
| * same flag the back end uses to register the blocks themselves. | ||
| */ | ||
| 'searchBlocksEnabled' => (bool) apply_filters( 'jetpack_search_blocks_enabled', false ), |
kangzj
previously approved these changes
May 5, 2026
Contributor
kangzj
left a comment
kangzj
left a comment
There was a problem hiding this comment.
This looks like a good start and is not affecting current dashboard. 🚢
Would be good to improve mobile view:
adamwoodnz
added a commit
that referenced
this pull request
May 5, 2026
`updateJetpackSettings` writes the optimistic settings into the store before yielding the API control. On failure the catch block restored only `module_active` / `instant_search_enabled` — the new `experience` field was left at the unconfirmed value. No selector reads `jetpackSettings.experience` today (we read `last_saved_experience`), so this was latent — but cheap to fix and keeps the store consistent for whenever consumers do start reading it. Caught by claude[bot] review on PR #48500.
adamwoodnz
added a commit
that referenced
this pull request
May 5, 2026
…M-2116) The booleans returned by the REST endpoint can't distinguish Embedded from Theme search — both are `module_active=true, instant_search_enabled=false`. Add an inline NOTE so future contributors don't add an `'embedded'` branch and silently change the fallback behaviour for sites with that boolean pair. Caught by claude[bot] review on PR #48500.
`<Stack>` with no `direction` defaults to row (it sets `flexDirection: undefined`, so the browser falls back to the CSS default). Drop the explicit `direction="row"` from the option-stack, title, and footer Stacks; the column Stacks (outer options list, option-body) keep `direction="column"` since that's a real override.
Now that `@wordpress/ui` is a direct dep (Stack, Badge), use its Button
for the Save action too. Maps the previous `@wordpress/components` API
across:
- `variant="primary"` → drop (defaults `variant="solid" tone="brand"`
give the same primary-brand button)
- `isBusy` → `loading`
- `aria-disabled={...}` → `disabled={...}`. `@wordpress/ui` Button has
`focusableWhenDisabled=true` by default, so it still renders
`aria-disabled="true"` rather than the native `disabled` attribute,
preserving focus order. Existing tests that assert
`aria-disabled="true"`/`"false"` continue to pass unchanged.
Update the docblock to reflect the new behaviour and drop the stale
"would pull a new runtime dep" justification.
`updateJetpackSettings` writes the optimistic settings into the store before yielding the API control. On failure the catch block restored only `module_active` / `instant_search_enabled` — the new `experience` field was left at the unconfirmed value. No selector reads `jetpackSettings.experience` today (we read `last_saved_experience`), so this was latent — but cheap to fix and keeps the store consistent for whenever consumers do start reading it. Caught by claude[bot] review on PR #48500.
…M-2116) The booleans returned by the REST endpoint can't distinguish Embedded from Theme search — both are `module_active=true, instant_search_enabled=false`. Add an inline NOTE so future contributors don't add an `'embedded'` branch and silently change the fallback behaviour for sites with that boolean pair. Caught by claude[bot] review on PR #48500.
`isSaveDisabled = !isDirty || isUpdating`, and `isDirty` already guarantees `pending_experience` is non-null when it's true. So when `isSaveDisabled` is false, `pendingExperience` is guaranteed non-null — the `! pendingExperience` branch is dead code that implied the value could be falsy when the save is enabled. Caught by claude[bot] review on PR #48500.
Two `useSelect` subscriptions per row × four rows was eight store listeners; collapse to one per row. Same observable values, half the subscription churn on store updates. Caught by claude[bot] review on PR #48500.
…2116) Improve mobile / narrow-viewport behavior (kangzj review on PR #48500). Previously the option row was a single flex row that wrapped on phone- width, which let the radio drop to its own line below the title. The new layout pins the radio to its own grid column on the left, vertically centered against the content; everything else (icon · title · description · Recommended badge · Active badge) lives in a wrapping Stack in column 2 and wraps among itself when the viewport is narrow. The mobile-specific `flex-wrap: wrap` override is no longer needed — the column-2 Stack always wraps — so the phone-down rule shrinks back to just the smaller card padding.
…2116) Move the wrap point from the column-2 Stack to the title Stack: - column-2 (icon · body · active badge) no longer wraps, so the icon always shares a row with the body's title and the description sits directly beneath it. The active badge stays vertically centered to the body's right edge. - the title Stack (label text + Recommended badge) now wraps, so on narrow viewports the Recommended badge drops onto a second line beneath the label text rather than pushing the rest of the row out of shape.
WP Admin's `wp-admin/css/forms.css` ships `margin: -4px 4px 0 0` on `input[type=radio]` to fudge alignment with adjacent label baselines in classic forms. That same negative margin was leaking into the feature-selector option row, pulling the radio about 2px above the icon's vertical centre. Reset the margin to 0 on `.jp-search-feature-selector__option-radio` so the grid's `align-items: center` puts the radio on the same baseline as the icon. Verified in Chrome: previous deltaY between radio centre and icon centre was -2px, now 0.
WP Admin's `input[type="checkbox"], input[type="radio"]` rule (with the bad `margin: -4px 4px 0 0`) has specificity (0,1,1), the same as our previous `.jp-search-feature-selector__option-radio` reset (0,1,0) — except the WP rule wins because of the attribute selector. Qualify ours with `input[type="radio"]` so the reset lands at (0,2,1) and reliably overrides.
Restructure each feature-selector row so the icon sits next to the title, the description wraps full width below them, and the Active badge anchors a third grid column. The Recommended badge stays on the title line when there's room and wraps to its own line on narrow viewports without dropping below the description. Previously the row was a 2-column grid (radio | flex row of icon, body, badge) with the description nested under the title. The new shape is a 3-column grid (radio | content stack | active badge), which keeps the radio and Active badge centred against the whole content block and lets the headline Stack handle its own wrap.
The headline row's `md` gap left the icon floating noticeably away from its title; dropping to `sm` brings them visibly together as a single label unit. The content stack's `xs` gap, by contrast, made the description crowd the title — `sm` gives the description the breathing room it needs to read as secondary copy.
The h2 was inheriting WP Admin's tight default line-height, which left the descenders crowding the card below. Apply `--wpds-typography-line-height-md` so the heading breathes in line with the rest of the WPDS-tokenised dashboard typography.
"Pick what visitors see when they search" framed the choice as a content question, but the four options are really four different search products. "Select a search experience for your visitors" matches the language used in the option labels and badges (Embedded search / Overlay search / Theme search) and reads as a configuration choice, which is what this section actually is.
37987ec reworded the feature-selector h2 from "Pick what visitors see when they search" to "Select a search experience for your visitors" but left two tests that look up the fieldset by its old accessible name. Update both name patterns so the suite reflects the shipping copy.
Each feature-selector row was a CSS grid (auto 1fr auto) that hand-rolled the same flex layout @wordpress/ui's Stack already provides. Swap to a row Stack that renders as the wrapping <label> via Base UI's `render` prop, so the radio | content | active-badge layout, gap, and vertical centering all come from design-system tokens instead of bespoke grid CSS. The middle column now grows via `flex: 1 1 auto` on the content Stack (the row Stack's other two children — the radio and the trailing Active badge — keep their intrinsic widths). A `flex-shrink: 0` on the radio prevents the description's wrapping content from squashing it on narrow viewports, where grid's `auto` column had been doing that work for free.
adamwoodnz
force-pushed
the
rsm-2116-search-dashboard-feature-selection-ui
branch
from
decb802 to
cacf057
Compare
Backend (RSM-2291) settled on `experience` as the field name in the REST response and initial state, replacing the temporary `last_saved_experience` forward-compat name. Update the dashboard store to match: - Rename the store slot `last_saved_experience` → `experience`. - Drop the redundant `getLastSavedExperience` selector — `getActiveExperience` is the only consumer of that slot, and it now reads `experience` directly with the legacy-boolean derivation as defence-in-depth. - Rename `setLastSavedExperience` → `setActiveExperience` (paired with `getActiveExperience`); `saveExperience` writes through it after a successful save as a belt-and-braces complement to the post-save `fetchJetpackSettings` round-trip in `updateJetpackSettings`. - Update test fixtures (`baseSettings` / `settings` blocks) and the jetpack-settings store test to the new field/action names. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`classic` collides with the older Jetpack Search "classic search"
terminology, which refers to a deprecated experience. Rename the new
non-instant non-embedded option to `inline` everywhere it appears:
the `EXPERIENCE` enum, `EXPERIENCE_ORDER`, the label/description/icon
switches, the legacy-fallback derivation in the selectors, and the
test fixtures that exercise the value.
User-facing labels are unchanged ("Theme search", "Your theme's search
layout, with faster results.") — only the wire/storage value changes.
Backend (PR #48540) needs a matching rename in `Module_Control`
constants (EXPERIENCE_CLASSIC → EXPERIENCE_INLINE) and validation list
so the wire format stays consistent end-to-end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…or (RSM-2116) The new feature selector can't actually persist changes until the back-end `experience` field lands (RSM-2291 / PR #48540), so until then the dashboard would be effectively read-only when `jetpack_search_blocks_enabled` is on. Render `ModuleControl` regardless of the feature flag for now so admins can keep managing Search settings. The `FeatureSelector` still appears above it when the flag is on, giving a preview of the new UI. Updated the dashboard-page branching test to assert both components render together when the flag is on, and that ModuleControl always renders. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adamwoodnz
added a commit
that referenced
this pull request
May 6, 2026
Match the frontend rename in PR #48500 — `classic` collided with the deprecated original Jetpack Search "classic search" terminology. Wire value is now `'inline'`; storage shape is unchanged (still deleted on write, since inline is the absence of an opt-in). - `Module_Control::EXPERIENCE_CLASSIC` → `EXPERIENCE_INLINE` (value `'inline'`) - `update_experience()` switch case + validation list - `get_experience()` legacy-fallback return value and docblock - Test names + values in `Module_Control_Test` and `REST_Controller_Test` - Changelog entry Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
[Status] UI Changes
adamwoodnz
added a commit
that referenced
this pull request
May 6, 2026
Match the frontend rename in PR #48500 — `classic` collided with the deprecated original Jetpack Search "classic search" terminology. Wire value is now `'inline'`; storage shape is unchanged (still deleted on write, since inline is the absence of an opt-in). - `Module_Control::EXPERIENCE_CLASSIC` → `EXPERIENCE_INLINE` (value `'inline'`) - `update_experience()` switch case + validation list - `get_experience()` legacy-fallback return value and docblock - Test names + values in `Module_Control_Test` and `REST_Controller_Test` - Changelog entry Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adamwoodnz
added a commit
that referenced
this pull request
May 6, 2026
…is on (RSM-2291) The frontend PR (#48500) shipped a transitional state where ModuleControl rendered alongside FeatureSelector even with the feature flag on, so admins could still manage Search settings while the back-end `experience` field was unimplemented. With this PR adding that back end, the new selector can fully persist the user's choice on its own — the legacy toggles below it become redundant. Reverts the dashboard-page bottom-area to the either/or shape: when `jetpack_search_blocks_enabled` is on, render only `<FeatureSelector>`; otherwise render only `<ModuleControl>`. Updates the branching test to assert ModuleControl is *absent* when the flag is on. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adamwoodnz
added a commit
that referenced
this pull request
May 6, 2026
…SM-2291) (#48540) * feat(search): add backend support for experience field (RSM-2291) - Add SEARCH_MODULE_EXPERIENCE_OPTION_KEY constant and experience value constants to Module_Control - Add get_experience() method that returns persisted value or derives from legacy booleans - Add update_experience() method that writes experience and legacy booleans in lockstep - Update REST update_settings() to accept experience-only requests - Update REST get_settings() to include last_saved_experience in response - Update validate_search_settings() to allow experience-only requests - Update Initial_State to include last_saved_experience - Update existing tests to include last_saved_experience in expected responses - Add new tests for experience read/write paths - Add changelog entry" Agent-Logs-Url: https://github.com/Automattic/jetpack/sessions/40f5e762-e4d0-4932-b4ac-5bd0ec01f26b Co-authored-by: adamwoodnz <1017872+adamwoodnz@users.noreply.github.com> * Search: narrow `experience` storage shape (RSM-2291) Off and classic are no longer stored in `jetpack_search_experience`. Off is read from `Modules::is_active()` because that bit lives in the global `jetpack_active_modules` option; storing `'off'` here would drift the moment any non-Search path (Jetpack dashboard module toggle, WP-CLI, `Jetpack::activate_module()`) flipped the global. Classic is the absence of an opt-in — `update_experience('classic')` deletes the option so existing classic sites stay at zero writes and don't need a migration. The wire format still exposes all four values (`embedded | overlay | classic | off`), so JS callers and external consumers see a clean enum; `get_experience()` resolves them from the on/off bit + saved value + legacy `instant_search_enabled` fallback. Renames the REST/initial-state field from `last_saved_experience` to `experience` to match the JS selector (`getActiveExperience`) and the request-body field name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Search: rename `classic` experience to `inline` (RSM-2291) Match the frontend rename in PR #48500 — `classic` collided with the deprecated original Jetpack Search "classic search" terminology. Wire value is now `'inline'`; storage shape is unchanged (still deleted on write, since inline is the absence of an opt-in). - `Module_Control::EXPERIENCE_CLASSIC` → `EXPERIENCE_INLINE` (value `'inline'`) - `update_experience()` switch case + validation list - `get_experience()` legacy-fallback return value and docblock - Test names + values in `Module_Control_Test` and `REST_Controller_Test` - Changelog entry Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Search: address backend PR review feedback (RSM-2291) From the bot review on #48540: - Drop the unreachable `return true` after the exhaustive switch in `update_experience()`. The four valid values each return inside their case and the in_array guard rejects everything else, so the trailing return was dead code. - Fix the stale "classic" mention in the REST controller's delegation comment — it should read "inline → delete option" after the rename. - Reject requests that mix `experience` with `module_active` or `instant_search_enabled` rather than silently dropping the legacy fields. `experience` writes the legacy booleans in lockstep, so there is no scenario where the caller needs both. Returns 400 `rest_invalid_arguments` with a clear message; new REST test covers the two mixing combinations. - Tighten `test_update_experience_off_preserves_other_state` so it proves deactivation actually ran. Previously the active-modules filter was removed before the assertion, so `is_active() === false` was trivially true regardless of whether `deactivate()` had been called. Now reads `jetpack_active_modules` directly while the filter is still active, matching the pattern in `test_deactivate_module`. Two items intentionally not changed: the partial-failure rollback in the overlay branch (acknowledged as a known package-wide pattern by the reviewer; "rolling back" by deactivating the module would itself be unrequested state mutation), and the `SEARCH_MODULE_EXPERIENCE_OPTION_KEY` prefix nit (the option name is what's stored in `wp_options` and prefixing it is the correct namespace for cross-plugin coexistence). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Search: cover update_experience() error-propagation branches (RSM-2291) The four `if ( is_wp_error( $result ) ) { return $result; }` branches in `update_experience()`'s INLINE / EMBEDDED / OVERLAY cases were the only new lines in this PR not exercised by tests (per the PR's coverage report). They're behavioral — a typo in any of them would silently swallow a partial-failure error and write the experience option in an inconsistent state. Adds: - A data-provider-driven test asserting each of the three experiences that calls activate() propagates the WP_Error from a plan-without- search and leaves the experience option unwritten. - A test asserting overlay propagates the WP_Error from enable_instant_search() (plan supports search but not instant search) and leaves the experience option unwritten. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Search dashboard: stop rendering ModuleControl when feature-selector is on (RSM-2291) The frontend PR (#48500) shipped a transitional state where ModuleControl rendered alongside FeatureSelector even with the feature flag on, so admins could still manage Search settings while the back-end `experience` field was unimplemented. With this PR adding that back end, the new selector can fully persist the user's choice on its own — the legacy toggles below it become redundant. Reverts the dashboard-page bottom-area to the either/or shape: when `jetpack_search_blocks_enabled` is on, render only `<FeatureSelector>`; otherwise render only `<ModuleControl>`. Updates the branching test to assert ModuleControl is *absent* when the flag is on. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Sync: whitelist jetpack_search_experience option (RSM-2291) Per review feedback on PR #48540, add the new `jetpack_search_experience` option to the Search sync module's options whitelist so writes propagate to the WPcom shadow replicastore. Without this, the dashboard saves locally but the WPcom-side experience derivation can't see the user's choice. Note: requires a coordinated WPcom-side whitelist entry for the option to actually be retained on the cache site. * Search: type-guard `experience` before sanitize_text_field() (RSM-2291) Per PR #48540 review: passing an array or object payload through sanitize_text_field() triggers a PHP 8.1+ array-to-string deprecation notice. The downstream in_array() check in update_experience() already rejects non-string values, but the warning is unnecessary noise. Add an is_string() guard so non-string values resolve to null up front and skip the experience branch entirely. * Search: reject `experience` + `swap_classic_to_inline_search` mixing (RSM-2291) Per PR #48540 review: validate_search_settings() rejected mixing `experience` with `module_active` / `instant_search_enabled`, but let `swap_classic_to_inline_search` through. The early return in the `experience` branch of update_settings() then dropped that field silently — same footgun the existing rejection was meant to prevent. Extend the rejection to cover all three legacy fields and update the error message accordingly. Today's only caller (the new front end) sends `{ experience }` alone, so this is a defensive fix for external API consumers. Add the new mixing combination to the existing rejection test. * Search: propagate Modules::deactivate() result from update_experience('off') (RSM-2291) Per PR #48540 review: the OFF branch was discarding Modules::deactivate()'s bool and hard-returning true. Propagate the bool instead — false signals the module was already inactive (a benign no-op), true signals it was removed from jetpack_active_modules. The REST controller only branches on is_wp_error(), so a propagated false still falls through to the success response — but direct callers of update_experience() can now distinguish the two cases. INLINE / EMBEDDED / OVERLAY are unchanged: their inner update_option() and delete_option() calls return false on no-change writes (option already at the target value), which can't be safely conflated with failure. WP_Error paths in those branches are already propagated. Update test_update_experience_off_preserves_other_state to assert the new return value, and add coverage for the no-op deactivate path. * Search: stabilize update_experience('off') no-op test (RSM-2291) The new test added in caa3678 assumed an empty jetpack_active_modules option, but earlier tests in the same class activate the module via update_experience() and leak 'search' into the persisted option. Seed the option with an empty array up front so deactivate() truly is a no-op (update_option returns false when the stored value already matches). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: adamwoodnz <1017872+adamwoodnz@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gibrown
added a commit
that referenced
this pull request
May 7, 2026
This is a squashed commit from developing the feature and multiple experiments with the interface. It also includes the original plan for building the feature. This was merged with the new experience selection logic from #48500
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
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.
Fixes RSM-2116: Search dashboard feature selection UI
Why
Today site owners on the Jetpack Search dashboard juggle two independent toggles to decide what visitors see when they search, and there's no place in the UI for the upcoming block-based "Embedded Search" experience to live. Behind the Search 3.0 feature flag, this PR adds a single radio list — Embedded search, Overlay search, Theme search, or Off — with a Save button, so picking an experience is one explicit decision with the active choice clearly labelled. The legacy toggles continue to render below the new selector so admins can keep managing settings while the back-end work to actually persist the new choice lands.
Default
Classic search only
Proposed changes
<FeatureSelector>component on the Search Dashboard: a four-row radio list (Embedded search · Overlay search · Theme search · Off) under a visible<h2>heading "Select a search experience for your visitors", with a Save button that's disabled until a different option is picked.jetpack_search_blocks_enabledserver-side filter, exposed to the React app assearchBlocksEnabledin the dashboard's initial state. When the flag is on,<FeatureSelector>renders above the legacy<ModuleControl>. When the flag is off, only the legacy toggles render.<ModuleControl>now renders regardless of the flag — until the back-endexperiencefield lands (RSM-2291), the new selector can't actually persist changes, so keeping the legacy toggles visible lets admins continue managing Search settings.saveExperiencethunk that posts a single-field payload —{ experience }(one of'embedded' | 'overlay' | 'inline' | 'off') — to/jetpack/v4/search/settings. Without Search 3.0: backend support forexperiencefield in settings API (RSM-2291) #48540 the endpoint 400s on this payload and the UI handles that path correctly (error notice, pending stays put, no Active-badge promotion). With Search 3.0: backend support forexperiencefield in settings API (RSM-2291) #48540, the round-trip succeeds and the seededexperiencevalue is whatgetActiveExperiencereads.experienceslot holds the active value (seeded from initial state at boot, also written bysetActiveExperienceafter a successful save as defence-in-depth).getActiveExperiencefalls back to deriving from the legacymodule_active/instant_search_enabledbooleans ifexperienceis absent — covers unit tests and the no-back-end period before Search 3.0: backend support forexperiencefield in settings API (RSM-2291) #48540 ships.supportsOnlyClassicSearch(), the Embedded search and Overlay search rows render disabled with an "Upgrade your plan to unlock this option." tooltip.@wordpress/uiv0.12.0 primitives — adds the package as a direct dep on the search package and uses its shippedBadge(intent="informational"for Recommended,intent="stable"for Active),Stack(option list, option-row layout, body, title, footer), andButton(Save, withdisabled+loadingdrivingaria-disabled/ busy state).jetpack_search_experience_save(props:previous_experience,new_experience) on the new selector flow. The legacy per-togglejetpack_search_module_toggle/jetpack_search_instant_toggleevents continue to fire from the legacy<ModuleControl>whenever it renders.@wordpress/themedesign tokens (--wpds-dimension-*,--wpds-color-*,--wpds-border-*,--wpds-typography-*) throughout — no literal px / hex values. The selected-row border uses--wpds-color-stroke-interactive-brandso the selection ring matches the rest of the admin UI.MockedSearchandRecordMeterare unchanged; only the bottom control area gains the new selector above the existing legacy toggles.Why
inlineinstead ofclassic?The wire/storage value for the non-instant non-embedded option is
'inline'. The stringclassiccollides with the older Jetpack Search "classic search" terminology in the package, which refers to a separate (deprecated) experience. The user-facing label remains "Theme search."Related product discussion/links
experiencefield in settings API (RSM-2291) #48540 (RSM-2291) — persists theexperiencefield; required for the selector's save round-trip to succeedDoes this pull request change what data or activity we track or use?
Yes — adds one new tracks event,
jetpack_search_experience_save, on the new selector flow only. Props are two enum strings ('embedded' | 'overlay' | 'inline' | 'off') capturing the previous and newly-saved experience. Legacy per-toggle events still fire from<ModuleControl>. Tagging the PR with[Status] Needs Privacy Updatesfor review.Testing instructions
Automated
Expected: dashboard JS suites pass (selectors + actions including
saveExperiencesuccess and failure paths,<ExperienceOption>,<FeatureSelector>, dashboard-page branching with both new selector and legacy ModuleControl rendering when the flag is on).Manual smoke
The path that's testable end-to-end depends on whether #48540 is also installed on the test site:
experiencefield in settings API (RSM-2291) #48540: save round-trips will fail. Steps 7–9 below verify the UI handles that error path correctly.experiencefield in settings API (RSM-2291) #48540: save round-trips succeed. Steps 7–9 should produce success notices and a moved Active badge.<ModuleControl>(two toggles) renders. No new selector.add_filter( 'jetpack_search_blocks_enabled', '__return_true' );. Refresh the dashboard.<h2>"Select a search experience for your visitors" sits above a card containing four radio rows in order: Embedded search (Recommended badge) · Overlay search · Theme search · Off. Below the new selector, the legacy<ModuleControl>is also rendered.experiencevalue if Search 3.0: backend support forexperiencefield in settings API (RSM-2291) #48540 is installed and a value was previously saved).aria-disabled="true") until you pick a different option, and remains focusable./jetpack/v4/search/settingsbody is exactly{"experience":"embedded"}.experiencefield in settings API (RSM-2291) #48540: response is400 rest_invalid_arguments. UI shows the error notice; Active badge stays put;pending_experiencestays on Embedded for retry.experiencefield in settings API (RSM-2291) #48540: response is 200; UI shows a success notice; Active badge moves to Embedded;pending_experienceclears.experiencefield in settings API (RSM-2291) #48540, state derives from the legacy booleans (no seededexperience). With Search 3.0: backend support forexperiencefield in settings API (RSM-2291) #48540, the seededexperiencefrom initial state drives the Active badge.{"experience":"…"}request body. Note that the wire value for "Theme search" isinline, notclassic.<ModuleControl>toggles below the new selector — confirm they still drivemodule_active/instant_search_enabledindependently and the Active badge updates to match (via the legacy-boolean fallback ingetActiveExperience).