Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Filters] Add onQueryFocused callback #1948

Merged
merged 1 commit into from
Aug 12, 2019

Conversation

joshuajay
Copy link
Contributor

WHY are these changes introduced?

Current functionality in web relies on an onQueryFocus callback on the ListFilters component in polaris-next. This is used to instrument the search behaviour, where a focus event on the query field indicates the beginning of the users intent to search.

Tim's excellent PR to replace the polaris-next ListFilters with the proper Filter component from Polaris means we can no longer do this. (see https://github.com/Shopify/web/pull/15425#discussion_r311870970)

WHAT is this pull request doing?

Adding a callback to the Filters component to be triggered when the query field is focused.

🎩 checklist

@BPScott BPScott temporarily deployed to polaris-react-pr-1948 August 9, 2019 13:52 Inactive
@joshuajay joshuajay force-pushed the add-query-focus-callback-to-filters branch from beb1936 to 6991354 Compare August 9, 2019 13:53
@BPScott BPScott temporarily deployed to polaris-react-pr-1948 August 9, 2019 13:53 Inactive
@joshuajay joshuajay requested review from tmlayton and AndrewMusgrave and removed request for AndrewMusgrave August 9, 2019 13:53
Copy link
Contributor

@tmlayton tmlayton left a comment

Choose a reason for hiding this comment

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

Can you add a test? Otherwise, looks good!

@joshuajay joshuajay force-pushed the add-query-focus-callback-to-filters branch from 6991354 to e332afc Compare August 12, 2019 18:30
@BPScott BPScott temporarily deployed to polaris-react-pr-1948 August 12, 2019 18:30 Inactive
@joshuajay
Copy link
Contributor Author

@tmlayton Thanks Tim! Test has been added.

@joshuajay joshuajay self-assigned this Aug 12, 2019
@joshuajay joshuajay requested a review from tmlayton August 12, 2019 18:39
@joshuajay joshuajay merged commit ad44f13 into master Aug 12, 2019
@joshuajay joshuajay deleted the add-query-focus-callback-to-filters branch August 12, 2019 18:56
@tmlayton
Copy link
Contributor

Will setup a new shipit stack to cut 3.21.1 with this change

tmlayton pushed a commit that referenced this pull request Aug 12, 2019
@tmlayton tmlayton mentioned this pull request Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants