Skip to content

Search: Implicitly activate search when widget added#8822

Merged
oskosk merged 3 commits intomasterfrom
update/search/activate-search-on-widget-add
Feb 23, 2018
Merged

Search: Implicitly activate search when widget added#8822
oskosk merged 3 commits intomasterfrom
update/search/activate-search-on-widget-add

Conversation

@Viper007Bond
Copy link
Copy Markdown
Contributor

@Viper007Bond Viper007Bond commented Feb 13, 2018 •
edited by oskosk
Loading

Fixes #8763

Changes proposed in this Pull Request:

If the user's plan supports search, then show the widget even if the search module/functionality is disabled. If the widget is used, then enable the search module automatically.

Testing instructions:

Make sure your test site has a plan that supports Jetpack Search.

  1. Remove any active search widgets and disable the search module via Settings -> Traffic.
  2. Verify that the widget still shows up in both the admin area and the customizer.
  3. Add the widget to your sidebar.
  4. Load the frontend of your site and verify that the widget shows up and works.
  5. Verify that the search module is now enabled again.

Changelog entry

  • We now implicitly update Jetpack Search when attempting to add the Jetpack Search widget.

@Viper007Bond Viper007Bond added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Search For all things related to Search labels Feb 13, 2018
@Viper007Bond Viper007Bond added this to the 5.9 milestone Feb 13, 2018
@Viper007Bond Viper007Bond self-assigned this Feb 13, 2018
@Viper007Bond Viper007Bond requested a review from a team as a code owner February 13, 2018 04:55
@jeherve jeherve added the [Status] Needs Review This PR is ready for review. label Feb 13, 2018
oskosk
oskosk reviewed Feb 14, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@oskosk oskosk left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I got to an issue testing this PR and adding the widget from the customizer.

  1. Search module is off.
  2. Get to customizer, add the widget to the sidebar, publish.
  3. Get to the frontend, the search box does not show. Although the module is indeed activated implicitly.
  4. Get to Appearance - Widgets, add a title or anything that enables the save button. Save
  5. Get to the frontend, the search input shows...

@Viper007Bond
Copy link
Copy Markdown
Contributor Author

Viper007Bond commented Feb 15, 2018

I can reproduce this but the same thing appears to happen in master as well (can you confirm?), so I'd rather address it in a separate PR.

@gibrown
Copy link
Copy Markdown
Member

gibrown commented Feb 22, 2018

Seems to all be working fine for me. There is some funkiness with the customizer, but I think we should put that into a separate issue.

Just to document it:

  • Search is disabled
  • Add the widget from the customizer
  • The preview of the widget is empty and does not update until after we reload the customizer

Screenshot:

screen shot 2018-02-22 at 10 12 42 am

gibrown
gibrown approved these changes Feb 22, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@gibrown gibrown left a comment

Choose a reason for hiding this comment

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

Code LGTM.

@Viper007Bond Viper007Bond requested a review from a team February 22, 2018 22:12
@Viper007Bond
Copy link
Copy Markdown
Contributor Author

Viper007Bond commented Feb 22, 2018

Yeah, I have a separate branch going to fix the empty widget issue as it appears to be unrelated to this PR.

oskosk
oskosk approved these changes Feb 23, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

🚢

@oskosk oskosk 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 Feb 23, 2018
@oskosk oskosk merged commit ab4c6c5 into master Feb 23, 2018
@oskosk oskosk deleted the update/search/activate-search-on-widget-add branch February 23, 2018 14:13
@oskosk oskosk mentioned this pull request Feb 23, 2018
Merged
This was referenced Feb 23, 2018
Closed
Merged
Merged
oskosk added a commit that referenced this pull request Feb 27, 2018
@oskosk
Add changelog entry for #8822
afc24d5
oskosk added a commit that referenced this pull request Feb 27, 2018
@oskosk
Changelog 5.9: running changelog (#8819)
1174ef5 
* update changelog.txt

* Update readme.txt with scaffolding for 5.9 changelog and release draft shortlink

* Add changelog entry for #8243

* Add changelog entry for #8296

* Add changelog entry for #8367

* Add changelog entry for #8686

* Add changelog entry for #8707

* Add changelog entry for #8709 and #8714

* Add changelog entry for #8729

* Add changelog entry for #8777

* Add changelog entry for #8780

* Add changelog entry for #8786

* Add changelog entry for #8787

* Add changelog entry for #8801 #8805 #8832 #8865 and #8804

* Add changelog entry for #8817

* Add changelog entry for #8822

* Add changelog entry for #8823

* Add changelog entry for #8829

* Add changelog entry for #8834

* move some items to major enhancements

* Add changelog entry for #8836

* Add changelog entry for #8839

* Add changelog entry for #8861

* Add changelog entry for #8862

* Add changelog entry for #8863

* Add changelog entry for #8866

* Add changelog entry for #8870

* Add changelog entry for #8874

* Add changelog entry for #8875

* Add changelog entry for #8881

* Add changelog entry for #8890

* Add changelog entry for #8911

* Add changelog entry for #8927

* Add changelog entry for #8931

* Add changelog entry for #8933

* Add changelog entry for #8930

* fix wording

* typo

* minor fixes

* replace partner scripts for Jetpack Start in changelog entry

* Update to-test.md

* Update to-test.md

* minor style fixes to to-test.md

* minor style fixes to to-test.md

* minor fixes on to-test.md

* Add changelog entry for #8868

* Add changelog entry for #8844

* Add changelog entry for #8664

* Add changelog entry for #8935

* Add changelog entry for #8425

* Add changelog entry for #8625
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gibrown gibrown gibrown approved these changes

+1 more reviewer

@oskosk oskosk oskosk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Viper007Bond Viper007Bond

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Search For all things related to Search Touches WP.com Files

Projects

None yet

Milestone

5.9

Development

Successfully merging this pull request may close these issues.

Search: Implicitly activate search when widget added

6 participants

@Viper007Bond @gibrown @oskosk @kraftbj @jeherve @matticbot