Skip to content

Search 3.0: hide the Off row of the feature-selector on WordPress.com (RSM-2400)#48558

Open
adamwoodnz wants to merge 2 commits intotrunkfrom
rsm-2400-search-dashboard-feature-selector-hide-the-off-row-on
Open

Search 3.0: hide the Off row of the feature-selector on WordPress.com (RSM-2400)#48558
adamwoodnz wants to merge 2 commits intotrunkfrom
rsm-2400-search-dashboard-feature-selector-hide-the-off-row-on

Conversation

@adamwoodnz
Copy link
Copy Markdown
Contributor

Fixes RSM-2400

Why

WordPress.com admins on the Search dashboard manage Search activation through the .com side, so the legacy <ModuleControl> deliberately hides its "Enable Jetpack Search" toggle on .com. The new feature-selector (shipped behind jetpack_search_blocks_enabled in #48500) was rendering all four rows regardless of platform, which would let a .com admin pick Off, hit Save, and trigger Modules::deactivate( 'search' ) — the exact action the legacy UI hides. This PR brings the new selector to platform parity.

Proposed changes

  • <FeatureSelector> now reads the existing isWpcom selector (seeded from Helper::is_wpcom() in class-initial-state.php) and filters EXPERIENCE_ORDER to drop EXPERIENCE.OFF when true. The remaining rows (Embedded / Overlay / Theme search) render unchanged.
  • New unit test hides the Off row on WordPress.com (parity with legacy ModuleControl) seeds siteData: { isWpcom: true } and asserts the Off radio is absent while the other three are present.
  • Inline comment in the source documents the parity decision so a future reader doesn't accidentally re-introduce the row.

Related product discussion/links

Does this pull request change what data or activity we track or use?

No.

Testing instructions

The whole feature-selector is gated behind jetpack_search_blocks_enabled, so set that on a test site to see the new UI. The legacy <ModuleControl> continues to render alongside the new selector, so admins on .com retain the instant-search-vs-classic toggle that ModuleControl already shows unconditionally.

Automated

cd projects/packages/search
pnpm jest tests/js/dashboard/feature-selector/

Expected: 17 tests pass across 2 suites.

Manual

  1. On a self-hosted Jetpack site, open WP Admin → Jetpack → Search with the filter jetpack_search_blocks_enabled returning true.
  2. Confirm all four radio rows render (Embedded, Overlay, Theme search, Off) — baseline.
  3. On a WordPress.com Simple site (or a site where Helper::is_wpcom() returns true), open the same dashboard.
  4. Confirm only three rows render (Embedded, Overlay, Theme search). The Off row is not present.
  5. Confirm the heading, Save button, and remaining-row interactions still work as before.
  6. Below the new selector, the legacy <ModuleControl> continues to render its instant-search toggle on .com (its own module-active toggle remains hidden, same as before this PR).

Mirrors the legacy `<ModuleControl>`'s `! isWpcom` gate around its
"Enable Jetpack Search" toggle. On WordPress.com, search activation is
managed from the .com side, so the new `<FeatureSelector>` shouldn't
expose Off as a save target either. We were rendering all four rows
regardless of platform, which would let a .com admin pick Off, hit
Save, and dispatch `Modules::deactivate( 'search' )` — the exact action
the legacy UI deliberately hides.

Reads `isWpcom` via the existing store selector (seeded from
`Helper::is_wpcom()` in `class-initial-state.php`), filters
`EXPERIENCE_ORDER` to drop `EXPERIENCE.OFF` when true, and renders the
filtered list.

The legacy `<ModuleControl>` continues to render alongside the new
selector while RSM-2291 lands, so admins on .com still see the
instant-search-vs-classic toggle that ModuleControl already renders
unconditionally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adamwoodnz adamwoodnz added the [Status] Needs Review This PR is ready for review. label May 6, 2026
@adamwoodnz adamwoodnz self-assigned this May 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the rsm-2400-search-dashboard-feature-selector-hide-the-off-row-on branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack rsm-2400-search-dashboard-feature-selector-hide-the-off-row-on

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions github-actions Bot added [Package] Search Contains core Search functionality for Jetpack and Search plugins [Tests] Includes Tests labels May 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

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:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

@adamwoodnz adamwoodnz requested a review from kangzj May 6, 2026 08:28
@jp-launch-control
Copy link
Copy Markdown

jp-launch-control Bot commented May 6, 2026

Code Coverage Summary

Coverage changed in 2 files.

File Coverage Δ% Δ Uncovered
projects/packages/search/src/dashboard/components/feature-selector/index.jsx 11/15 (73.33%) -1.67% 0 💚
projects/packages/search/src/dashboard/store/selectors/site-data.js 10/15 (66.67%) 6.67% -1 💚

Full summary · PHP report · JS report

Copy link
Copy Markdown
Contributor

@kangzj kangzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few small notes from a self-review pass — nothing blocking, mostly polishing the test assertion and a wording clarification. Behavior-wise this looks great and faithful to the legacy gate.

image

expect( screen.getByRole( 'radio', { name: /embedded search/i } ) ).toBeInTheDocument();
expect( screen.getByRole( 'radio', { name: /overlay search/i } ) ).toBeInTheDocument();
expect( screen.getByRole( 'radio', { name: /theme search/i } ) ).toBeInTheDocument();
expect( screen.queryByRole( 'radio', { name: /^off$/i } ) ).not.toBeInTheDocument();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One tiny thing on this assertion — the Off radio's accessible name is computed from the whole <label>, so it actually ends up looking like "Off WordPress default search." (see how <ExperienceOption> lays out the title plus description inside the label, around experience-option.jsx:86-97). That means /^off$/i (anchored, exact-match) won't match even when the row is rendered, so this line would pass regardless of whether the Off row is gone.

The good news: the expect( radios ).toHaveLength( 3 ) above is what's actually proving the row is hidden, so the test is sound. Just this last line ends up being a tautology. If you want the negative assertion to pull its weight, maybe drop the anchors to /off/i (matches the substring form used in the existing tests on lines 37 and 92), or just delete it since the length check has us covered.

);
// On WordPress.com, search activation is managed from the .com side, so we
// hide the Off row here to mirror the legacy `<ModuleControl>`'s
// `! isWpcom` gate around the "Enable Jetpack Search" toggle.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small wording suggestion — and to be clear this isn't your invention, the same loose phrasing lives in the legacy <ModuleControl> comment already. But while we're here:

Helper::is_wpcom() only checks the IS_WPCOM constant, which is defined on Simple sites and not on Atomic/WoA. So in practice an Atomic admin will still see the Off row here. That's actually correct behavior — the legacy <ModuleControl> gate has the same Simple-only meaning, so we're at parity — but a future reader scanning the comment might assume "WordPress.com" includes Atomic and get confused. Mind tightening to something like "On WordPress.com Simple sites" or "where IS_WPCOM is defined"? Just so the Atomic case isn't ambiguous.

// On WordPress.com, search activation is managed from the .com side, so we
// hide the Off row here to mirror the legacy `<ModuleControl>`'s
// `! isWpcom` gate around the "Enable Jetpack Search" toggle.
const isWpcom = useSelect( select => select( STORE_ID ).isWpcom(), [] );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny style musing, totally take or leave: this brings the component to five useSelect calls. Could batch them into a single call that returns an object — experience-option.jsx:34-40 already does that for two values and reads nicely. Pre-existing pattern in this file though, so happy to leave it for a follow-up if you'd rather keep this PR tight.

@kangzj
Copy link
Copy Markdown
Contributor

kangzj commented May 6, 2026

Plus when search blocks enabled, both the new selector and the old toggle show up together. I wonder what went wrong there:

image

Copy link
Copy Markdown
Contributor Author

Intentional for now while the backend isn't ready. Without the old UI you can't change settings 🙂

- Batch the five useSelect calls into one returning an object, matching
  the pattern already used in experience-option.jsx.
- Clarify the WPcom comment: only Simple sites (where IS_WPCOM is
  defined) hit this gate, not Atomic.
- Drop the anchors on the negative Off-row matcher so the regex actually
  matches the row's accessible name; the toHaveLength(3) above is the
  load-bearing check, but the assertion now pulls its weight too.
@adamwoodnz adamwoodnz added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Search Contains core Search functionality for Jetpack and Search plugins [Status] Ready to Merge Go ahead, you can push that green button! [Tests] Includes Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants