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

Add support for Accept Language in Geolocators #6982

Conversation

dev-newvisibility
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
License MIT

What's in this PR?

Both Nominatim and Google Geolocator support additional parameters to optimize the result of the query, this PR adds a new optional parameter to the GeolocatorInterface::locate method to pass additional options specific for a single query.

As an initial implementation this PR adds the ability to pass to the geolocator an "accept language" string.

Why?

Google Geolocator, if you don't pass the language query string parameter or an Accept-Language http header, tries to infer the desired results' language from the remote IP that it's calling its API, but the server in which runs the Sulu instance may not be located in the same country of the user, this gives a wrong output (wrong labels and/or wrong order).

Example Usage

@alexander-schranz alexander-schranz added the Feature New functionality not yet included in Sulu label Feb 7, 2023
@alexander-schranz alexander-schranz changed the base branch from 2.5 to 2.6 February 7, 2023 10:14
@alexander-schranz alexander-schranz added the To Discuss The core team has to decide if this will be implemented label Mar 14, 2023
@alexander-schranz
Copy link
Member

I'm here a little bit splitted what the correct language to send to this controller should be. Is it really the accept language of the browser sending us or when used inside translateable form if we should send then the language to the controller of that content. That would mean on a german form they may need to input Wien and on the english Vienna for the same result. Or if we stay with the browser language it even could be if the browser language is fr even on german and english the best result would be the user then inputting Vienne. 🤔

@sulu/core-developer what do you think?

@Prokyonn
Copy link
Member

I'm here a little bit splitted what the correct language to send to this controller should be. Is it really the accept language of the browser sending us or when used inside translateable form if we should send then the language to the controller of that content. That would mean on a german form they may need to input Wien and on the english Vienna for the same result. Or if we stay with the browser language it even could be if the browser language is fr even on german and english the best result would be the user then inputting Vienne. 🤔

@sulu/core-developer what do you think?

By default, I would recommend using the locale of the request to avoid inconsistencies, such as displaying an English address in a German format. However, I would still provide the option to always use the browser language, in case the project requires it.

@Prokyonn Prokyonn force-pushed the feature/geolocator-accept-language branch from 1daa928 to 3e41ac6 Compare April 30, 2024 08:31
@alexander-schranz alexander-schranz changed the title Geolocator support passing accept language string Add support for Accept Language in Geolocators May 2, 2024
@alexander-schranz alexander-schranz removed the To Discuss The core team has to decide if this will be implemented label May 2, 2024
@alexander-schranz alexander-schranz removed the request for review from chirimoya May 2, 2024 11:37
@alexander-schranz alexander-schranz force-pushed the feature/geolocator-accept-language branch from 1411405 to 766a304 Compare May 2, 2024 11:37
UPGRADE.md Outdated Show resolved Hide resolved
@alexander-schranz alexander-schranz merged commit 022cb6c into sulu:2.6 May 2, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New functionality not yet included in Sulu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants