Instant Search: Polish search interface UX #14762
Merged
Conversation
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
* Add label and button to search box and convert to functional component * Correct lodash import
* Turn statements into sentence fragments * Add appropriate landmark roles for elements * Set portaled widget as a search form * Use ordered list for search results
* [not verified] Add product search result component * [not verified] Refactor renderResult into component * [not verified] Convert SearchResult to stateless functional component * [not verified] Move comments to new shared component * [not verified] Move Tracks out to common SearchResult * [not verified] Request additional fields for product results * Correct field names and remove ratings for now * Basic styling of cards with images * Add price where available * Read result_format from the query string * Use ordered list * Style cleanup * Minor cleanup * Remove resultFormat from component state * SearchResults component cleanup * Standardise class names * Validate result format before using as CSS class * Address review nits
* Add PhotonImage component * Bump default dimensions to 300x300 * Bump asset size limit to 100kb
* Add label and button to search box and convert to functional component * Correct lodash import
* Turn statements into sentence fragments * Add appropriate landmark roles for elements * Set portaled widget as a search form * Use ordered list for search results
* [not verified] Add product search result component * [not verified] Refactor renderResult into component * [not verified] Convert SearchResult to stateless functional component * [not verified] Move comments to new shared component * [not verified] Move Tracks out to common SearchResult * [not verified] Request additional fields for product results * Correct field names and remove ratings for now * Basic styling of cards with images * Add price where available * Read result_format from the query string * Use ordered list * Style cleanup * Minor cleanup * Remove resultFormat from component state * SearchResults component cleanup * Standardise class names * Validate result format before using as CSS class * Address review nits
* Add PhotonImage component * Bump default dimensions to 300x300 * Bump asset size limit to 100kb
… into instant-search-master
* Instant Search: use default values for getting results to avoid duplication * getResults: default to empty object Co-Authored-By: Jason Moon <[email protected]>
* Ensure there's a cross to clear search on browsers that support it * Reduce font-size on results count text * Make close button behave similarly on different themes * Ensure there's a cross to clear search on browsers that support it * Reduce font-size on results count text * Make close button behave similarly on different themes * Ensures the clear button is displayed in the search box Props to @jsnmoon
* Use link for clear filters instead of a button * Update name in CSS
* Add className to label * Ensure search box is always full-width * Improves alignment of the layout grid * Tweak close button position
…4711) Sorting buttons and "Filter" button drop down on mobile. Co-authored-by: Jason Moon <[email protected]> Co-authored-by: Greg Ichneumon Brown <[email protected]>
jsnmoon
added
[Status] Needs Review
Collaborator
This is an automated check which relies on |
gibrown
approved these changes
Feb 24, 2020
Member
gibrown
left a comment
gibrown
left a comment
There was a problem hiding this comment.
Didn't see any code issues. Mucked around with the UX a bunch. Found some minor stuff, but nothing that is a blocker for merging this.
mdbitz
approved these changes
Feb 24, 2020
Contributor
mdbitz
left a comment
mdbitz
left a comment
There was a problem hiding this comment.
👋 Overall this looks good to me. The only question I have is if we need to do any validation/whitelisting of the locale prefix. Is there concern for bad values "https://.jetpack.com" "https://werer.jetpack.com/search", or etc
| const locale_prefix = typeof props.locale === 'string' ? props.locale.split( '-', 1 )[ 0 ] : null; | ||
| const url = | ||
| locale_prefix && locale_prefix !== 'en' | ||
| ? 'https://' + locale_prefix + '.jetpack.com/search' |
Contributor
There was a problem hiding this comment.
Should there be any validation/whitelisting of accepted prefixes?
Member
There was a problem hiding this comment.
I guess in theory the locale for the site could get set to something weird though I am not sure how.
I opened #14785
Contributor
matticbot
added
[Status] Needs Changelog
and removed
[Status] Needs Review
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.
Please note that the commit log includes previously merged changes for Instant Search. The new code introduced by this PR is from Feb 12, 2020, and onwards.
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:
Follow these setup instructions to set up Jetpack Instant Search on your site. Please note that the Jetpack Search widget will need to be added to twice: once to your site sidebar/footer and once to the Jetpack Search Sidebar within the search overlay.
Submit a query into your site search input. Ensure that a search overlay appears with your search results.
Try using the new sort select controls below the overlay search input. Ensure that they behave as expected.
Try resizing your window to be narrower than 768 pixels; a Filters button should spawn adjacent to the sort select controls.
Try clicking on the Filters button in (4). Ensure that a filters popover spawns below the button, like so:
Proposed changelog entry for your changes: