JustFly1984 / react-google-maps-api

React Google Maps API
MIT License
1.82k stars 439 forks source link

feat(ia18n): being able to pass locale string #2999

Open Aarbel opened 2 years ago

Aarbel commented 2 years ago

Thanks a lot @JustFly1984, would be amazing to be able to pass a locale to the react component, so that google maps is displayed in the chosen user language.

Do you plan to implement it ?

Thanks a lot !

EthanEFung commented 2 years ago

I believe this is already implemented https://react-google-maps-api-docs.netlify.app/#loadscript

JustFly1984 commented 2 years ago

@Aarbel @EthanEFung Yes it is implemented in , but it could be buggy and memory leak prone, and not tested with readt@17+. and not possible to implement with official google script loader. I'm thinking about adding this to hook version of our custom script loader.

EthanEFung commented 2 years ago

@JustFly1984 I could help with that if you need

JustFly1984 commented 2 years ago

@EthanEFung Cool, just make PR into react-18 branch

EthanEFung commented 2 years ago

@JustFly1984 While researching I came across an official google maps demo that I'm assuming encapsulates @Aarbel 's feature request. https://developers-dot-devsite-v2-prod.appspot.com/maps/documentation/javascript/demos/localization

Digging into the source code, the demo is doing a clever thing where each time either the language or the region is changed a query search parameter is added to the url and the entire page is re-rendered with the language or region that was specified.

and not possible to implement with official google script loader

The way I'm interpreting this statement is that developers might have an expectation that they could write something like

function Example() {
  const [language, setLanguage] = React.useState('en');
  return (
    <>
      <select onChange={(event) => setLanguage(event.currentTarget.value)}>
        <option value="en">English</option>
        <option value="fr">French</option>
        {/* ... */}
      </select>
      <LoadScript language={language}>
        {/*...*/}
      </LoadScript>
    </>
  )
}
// or LoadScriptNext or hook flavored script loader

But this isn't the case because this library's load script implementations add the script straight to the document head and the official script loader currently doesn't support clean up of the dozens of elements it appends to the DOM. It doesn't seem straight forward to support a feature like this.

proposed changes

  1. Create a new Dynamic Script Loader (yet another way to load the script) that wraps all the google maps components in an iframe and append the script to the head of the iframe DOM. This sounds complex.
  2. change all the script loaders to append to an iframe...This sounds monstrous.
  3. write an article / tutorial of how to create the UX in the official google maps demo above using react-google-maps-api

@Aarbel - It could be however, this was not your intention when proposing this feature. If you were intending to set the language or region onLoad only, then I still believe that the current implementations of LoadScript, LoadScriptNext, useLoadScript, and useJsApiLoader all support this, because this is how the official google maps api designed it to work.

JustFly1984 commented 2 years ago

@EthanEFung The idea to refresh the page is going against the SPA convention. You do not really want to reload an SPA, cos you will loose a state, and sometimes you need to re-login, it takes time to render the page. I'm sure this proposed behavior will mess up the code of many applications in production as it will be unexpected.

  1. iframe and managing state across frames and main thread is complex, but doable. I personally have no time to invest in that.
  2. same as 1
  3. If you can invest time to improve documentation and gatsby example - your PR is welcome. Currently we have issues with docs. If you can manage to fix docs - it will highly improve user experience for beginners.

PS current work in going in react-18 branch