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

Keep the user's query in the NL search bar when landing on the place page #4796

Merged
merged 7 commits into from
Dec 13, 2024

Conversation

dwnoble
Copy link
Contributor

@dwnoble dwnoble commented Dec 13, 2024

Demo

Keep.the.users.query.in.the.NL.search.bar.when.landing.on.the.place.page.4796.webm

@dwnoble dwnoble requested review from juliawu and gmechali December 13, 2024 18:18
@gmechali
Copy link
Contributor

We also do redirection directly from the autocomplete result. Should we also add that query there? It might just be a partial query like typing "Unite", select "United States", gets redirected to USA place page.

I might argue at that point it's better not to add the query? Just calling it out in case we do want to add it.

const url = PLACE_EXPLORER_PREFIX + `${result.dcid}`;

@dwnoble dwnoble enabled auto-merge (squash) December 13, 2024 19:42
@dwnoble
Copy link
Contributor Author

dwnoble commented Dec 13, 2024

We also do redirection directly from the autocomplete result. Should we also add that query there? It might just be a partial query like typing "Unite", select "United States", gets redirected to USA place page.

I might argue at that point it's better not to add the query? Just calling it out in case we do want to add it.

const url = PLACE_EXPLORER_PREFIX + `${result.dcid}`;

Good call- ill address this in a follow up since autocomplete isn't turned on yet. From the discussion with @miss-o-soup :
(1) if the user arrives at the page from a search, we'll show the search that they typed in (which is what this PR adds)
(2) if the user arrives at the place page any other way, we'll show the place name in the search bar instead of the default placeholder text. This should also cover when the user clicks on a result in the autocomplete list

@dwnoble dwnoble merged commit 6883b9c into datacommonsorg:master Dec 13, 2024
8 checks passed
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.

2 participants