Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

fix(svelte): Prefill search home page query input with (default) context filter#63740

Merged
fkling merged 2 commits into
mainfrom
fkling-srch-103-svelte-add-search-context-selector
Jul 9, 2024
Merged

fix(svelte): Prefill search home page query input with (default) context filter#63740
fkling merged 2 commits into
mainfrom
fkling-srch-103-svelte-add-search-context-selector

Conversation

@fkling

@fkling fkling commented Jul 9, 2024

Copy link
Copy Markdown
Contributor

Closes srch-103

Currently we don't show the global context filter on the search home page or the search results page (global context is the default context).

This commit does two things:

  • It prefills the search input on the search homepage with the user's default context (like the React app)
  • It the logic that pre-processed the search query and removed the context filter if it was global.

In other words we simplify the query logic by showing/submitting the search query as is. Notably this doesn't affect how the search input works on repo pages.

Test plan

  • Opening the search home page pre-fills the query input with the default search context
  • Submitting a query without a context: filter does not add a context: filter to the URL or the search input
  • If a query contains context:global that filter is preserved in the query input (it wasn't before)

@fkling fkling requested a review from camdencheek July 9, 2024 21:37
@fkling fkling self-assigned this Jul 9, 2024
@cla-bot cla-bot Bot added the cla-signed label Jul 9, 2024
Currently we don't show the global context filter on the search home
page or the search results page (global context is the default context).

This commit does two things:

- It prefills the search input on the search homepage with the user's
  default context (like the React app)
- It the logic that pre-processed the search query and removed the
  context filter if it was global.

In other words we simplify the query logic by showing/submitting the
search query as is. Noteably this doesn't affect how the search input
works on repo pages.
@fkling fkling force-pushed the fkling-srch-103-svelte-add-search-context-selector branch from 744e473 to 591e10c Compare July 9, 2024 21:38

@camdencheek camdencheek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

if (update.transactions.some(tr => tr.isUserEvent('select') || tr.isUserEvent('input'))) {
if (
update.transactions.some(
tr => tr.isUserEvent('select') || tr.isUserEvent('input') || tr.isUserEvent('delete')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added tr.isUserEvent('delete') here too to fix displaying the suggestions in the React app.

Comment on lines +55 to +57
return queryState
.setQuery(appendFilter(queryState.query, 'repo', `has.topic(${topic})`))
.toSearchURLQueryParamaters()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Some refactoring here to make it less awkward to use the API (otherwise undefined would have to be passed for search context, which is not obvious).

Comment on lines +105 to +107
public toSearchURLQueryParamaters(): string {
return buildSearchURLQuery(this.query, this.patternType, this.caseSensitive, undefined, this.searchMode)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Encapsulates the fact that undefined should be passed for search context.

@fkling fkling changed the title fix(svelte): Add back default context:global filter fix(svelte): Prefill search home page query input with (default) context filter Jul 9, 2024
@fkling fkling enabled auto-merge (squash) July 9, 2024 21:41
@fkling fkling merged commit 8237211 into main Jul 9, 2024
@fkling fkling deleted the fkling-srch-103-svelte-add-search-context-selector branch July 9, 2024 22:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants