-
Notifications
You must be signed in to change notification settings - Fork 82
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
FE changes to support location autocomplete within the NL Search bar #4649
base: master
Are you sure you want to change the base?
Conversation
…into autocomplete_fe
bp = Blueprint("autocomplete", __name__, url_prefix='/api') | ||
|
||
|
||
@bp.route('/autocomplete', methods=['GET', 'POST']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use just GET here since the request is read-only and idempotent.
Json object represnting 5 location predictions for the query. | ||
""" | ||
lang = request.args.get('hl') | ||
original_query = request.args.get('query') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this just be query = ...
?
@@ -0,0 +1,89 @@ | |||
# Copyright 2023 Google LLC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: 2024
Returns: | ||
List[str]: containing all subqueries to execute. | ||
""" | ||
words_in_query = user_query.split(" ") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: split on \s+
regex
RESPONSE_COUNT_LIMIT = 5 | ||
|
||
|
||
def find_queries(user_query: str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: add typing def find_queries(user_query: str) -> List[str]:
(and to other functions below)
@@ -697,6 +690,16 @@ def placeid2dcid(): | |||
return Response(json.dumps(result), 200, mimetype='application/json') | |||
|
|||
|
|||
@bp.route('/placeid2dcid') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this endpoint still needed?
@@ -0,0 +1,67 @@ | |||
# Copyright 2022 Google LLC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2024
place_id_to_dcid = json.loads(findplacedcid(place_ids).data) | ||
|
||
final_predictions = [] | ||
# TODO(gmechali): See if we can use typed dataclasses here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
@@ -0,0 +1,66 @@ | |||
# Copyright 2020 Google LLC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: 2024
}, []); | ||
|
||
// memoize the debounce call with useMemo | ||
const SendDebouncedAutoCompleteRequest = useMemo(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: sendDebouncedAutoCompleteRequest
All FE changes to add a dropdown on the NL search bar for autocompleting location search.
See screencast: https://screencast.googleplex.com/cast/NDkxMjUyMjc3MDUxMzkyMHxkZjg3ZDUxMC05MA
Also adds a webdriver test to verify the presence of the suggestion results.