Instant Search: Render disabled filter widgets #13332

jsnmoon · 2019-08-26T22:25:02Z

Changes proposed in this Pull Request:

When a user performs a search using a Jetpack Search widget, the search page will now include a dynamically generated list of filters on the sidebar like so:

The checkbox inputs are currently disabled at this time. Instant Search remains gated by a JETPACK_SEARCH_PROTOTYPE define.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Yes, this is a new feature being added onto Jetpack Instant Search (p3QzjZ-V3).

Testing instructions:

Add define( "JETPACK_SEARCH_PROTOTYPE", true ); to your wp-config.php.
Ensure that your site has the Jetpack Pro plan and has Jetpack Search enabled.
Add a Jetpack Search widget to the Search page sidebar.
Add a filter or two to the Jetpack Search widget.
Enter a query into a search widget.
Alternatively, navigate to a search page like /?s=privacy.
Ensure that you see a list of disabled checkboxes in the Search page sidebar.

Proposed changelog entry for your changes:

No changelog entry necessary; these changes will not be accessible to our users.

jetpackbot · 2019-08-26T22:27:05Z

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: September 3, 2019.
Scheduled code freeze: August 27, 2019

Generated by 🚫 dangerJS against f624a3a

gibrown

Looking good. We could hold off on the filter results formatting, etc for another PR given the current size. Also there is some overlap with my PR and merging this could help us clean that up. I think the two blockers are that the search box is disappearing for me and the asset size stuff.

modules/search/instant-search/components/search-filter-dates.jsx

gibrown · 2019-08-27T19:52:33Z

modules/search/instant-search/components/search-filter-dates.jsx

+											type="checkbox"
+										/>
+										<label htmlFor={ `jp-instant-search-filter-dates-${ bucket.key_as_string }` }>
+											{ strip( bucket.key_as_string ) } ({ bucket.doc_count })


This should get localized and we should remove the hours, minutes, secs. There is some config in the date config that indicates if we are displaying months or years.

When you say date config, are you referring to a query value that could be specified in the API request?

Ya there is interval information from the widget filter config: https://cloudup.com/cD3amQW3Ba1

That should be applied both for the API request and for determining how to display the results (if year, then only show the year, if month, then show year and month).

Could we push the interval formatting and localization to the API? Otherwise, we'll need to consider adding a date formatting library to the client, which could add a minimum of 15kB to our production bundle.

The interval value is already being passed in the API call.

Possibly ya. Is the 15kb from pulling in https://www.npmjs.com/package/@wordpress/date which looks like it includes date_i18n()? We may want something like this anyways to correctly display in search results though.

gibrown · 2019-08-27T19:52:55Z

modules/search/instant-search/components/search-filter-dates.jsx

+						]
+							// TODO: Remove this reverse & slice when API adds filter count support
+							.reverse()
+							.slice( 0, 5 ) }


I think the count comes from the config.

The API currently throws a 400 when I specify a size query value for aggregations[date_histogram_0][date_histogram][size]. See this example query.

Ya, the api will always return all of them, but we have an option (https://cloudup.com/cD3amQW3Ba1) that lets the user indicate how many to show. 5 shouldn't be hard coded.

Is it possible to support the size query value as it does for taxonomy and post_type aggregations?

In the meanwhile, I'll look into linking up the widget configuration values to the aggregation values in the client in a follow-up PR.

ES doesn't support it itself, so we would have to post process is somehow on the API side. I'd rather not deviate from the ES standards for aggs and filters though as that could make things harder to support long term.

modules/search/class.jetpack-search.php

modules/search/instant-search/index.jsx

modules/search/instant-search/lib/api.js

webpack.config.js

modules/search/instant-search/lib/api.js

gibrown · 2019-08-29T22:10:53Z

@jsnmoon since we are going to try a separate branch to merge into, maybe what is in here is good to go and you should just make a new PR against that branch, I or @bluefuton can cast some eyes over it and then we can handle the bug and display fixes in separate PRs?

…ion (#13365) Refactor search results and improve their display. Add display of when we make spelling corrections.

…earch Everything seems to work.

gibrown · 2019-08-30T19:41:36Z

I have opened this PR against the instant-search-master instead in #13371

jsnmoon · 2019-09-03T22:50:27Z

We'll be continuing this work in #13371 and landing our work in a feature branch instead of in master.

Jason Moon added 7 commits August 26, 2019 16:06

Polish styling for search results

29a00bc

Send aggregation information in API request

cc766bf

Move API utility into lib folder

103768c

Refactor DOM utility into a lib file

ee461ff

Render disabled filter widgets

a6860d4

Bump maximum entry point size to 35kb

e50355b

Render disabled filter widgets, part 2

06345b5

jsnmoon added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. [Feature] Search For all things related to Search Instant Search labels Aug 26, 2019

jsnmoon requested review from gibrown and a team August 26, 2019 22:25

jsnmoon self-assigned this Aug 26, 2019

matticbot added the Touches WP.com Files label Aug 26, 2019

gibrown mentioned this pull request Aug 27, 2019

Instant Search: Implement minimal search results and spelling correction #13313

Closed

gibrown requested changes Aug 27, 2019

View reviewed changes

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Aug 28, 2019

jsnmoon mentioned this pull request Aug 28, 2019

Instant Search: Add sidebar filters support #13195

Closed

Jason Moon added 5 commits August 28, 2019 14:20

Omit filter control if no aggregation value

5e1f562

Filter on key_as_string for date filter

a5d8d60

Revert changes to search-result component

5282036

Map registered post types for rendering

2c6eec0

Simplify injection by using Portals-only component

3b9cb07

jsnmoon added [Status] In Progress and removed [Status] In Progress [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Aug 28, 2019

jsnmoon added the [Status] Needs Review This PR is ready for review. label Aug 29, 2019

jsnmoon requested a review from gibrown August 29, 2019 21:05

Instant Search: Implement minimal search results and spelling correct…

a4e056f

…ion (#13365) Refactor search results and improve their display. Add display of when we make spelling corrections.

gibrown mentioned this pull request Aug 30, 2019

Instant Search: Add filtering display #13371

Merged

Merge branch 'instant-search-master' into add/filtering-for-instant-s…

f624a3a

…earch Everything seems to work.

jeherve added [Status] Blocked / Hold and removed [Status] Needs Review This PR is ready for review. labels Sep 3, 2019

jsnmoon closed this Sep 3, 2019

jsnmoon removed the [Status] Blocked / Hold label Sep 3, 2019

gibrown deleted the add/filtering-for-instant-search branch September 4, 2019 15:10

Instant Search: Render disabled filter widgets #13332

Instant Search: Render disabled filter widgets #13332

Uh oh!

Conversation

jsnmoon commented Aug 26, 2019

Changes proposed in this Pull Request:

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Testing instructions:

Proposed changelog entry for your changes:

Uh oh!

jetpackbot commented Aug 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gibrown left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsnmoon Aug 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gibrown commented Aug 29, 2019

Uh oh!

gibrown commented Aug 30, 2019

Uh oh!

jsnmoon commented Sep 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jetpackbot commented Aug 26, 2019 •

edited

Loading

jsnmoon Aug 29, 2019 •

edited

Loading

jsnmoon commented Sep 3, 2019 •

edited

Loading