mapbox / mapbox-gl-language

Switch language of your Mapbox GL JS style
https://mapbox.github.io/mapbox-gl-language/
BSD 3-Clause "New" or "Revised" License
198 stars 51 forks source link

map.addControl not work and without errors sometimes #27

Closed hijiangtao closed 3 years ago

hijiangtao commented 5 years ago

I use this package to change the display language of maps to Chinese, with following codes to control it when map fires onLoad event:

import MapboxLanguage from '@mapbox/mapbox-gl-language';

// the method I called after onLoad event
const addControlHandler = (event) => {
    const map = event && event.target;
    if (map) {
      map.addControl(new MapboxLanguage({
        defaultLanguage: 'zh',
      }));
      // map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh']);
    }
};

// The actual code in my map JSX code
// StaticMap is a module of package `react-map-gl`
return (
    <StaticMap 
        onLoad={addControlHandler}
    >
);

But it seems the Map Control will not work SOMETIMES, and without any errors in console, just keep silence and nothing happened. I thought it could be an internal problem of this package.

Once, I add another line to operate map instance, the Control will work well again. I don't know what's the problem?

// after `map.addControl` line
map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh']);

...
  1. GRAMMAR: The grammar I wrote is the same as https://github.com/mapbox/mapbox-gl-language/blob/master/examples/zh.html , however I wrote in react, rather than plain JavaScript.
  2. THIRD-PARTY PACKAGE:: I don't think it could be a third-party package's problem, though I use react-map-gl to provide a JSX style in creating map with mapbox, as I discussed in their issue https://github.com/uber/react-map-gl/issues/683#issuecomment-455959292 I could access the map variable and map.addControl is also not NULL when the onLoad event is fired.
  3. WHY: I got right result sometimes, just the same as I got wrong result, why could that be an occasional event, rather than a definite one?
  4. WHY2: Why would all the things get back to normal after I add one line code called map.setLayoutProperty?

Any ideas about it, thanks very much.

hijiangtao commented 5 years ago

I had noticed that load event will be fired when necessary resources been loaded, so does the necessary resources include map styles or label properties? Since I did an operation inside load event?

Fired immediately after all necessary resources have been downloaded and the first visually complete rendering of the map has occurred. - from https://docs.mapbox.com/mapbox-gl-js/api/#map.event:load

If not, what's the recommend time for developers to call addControlHandler method?

hijiangtao commented 5 years ago

I use it inside react application, but all the demos come from vanilla JavaScript, I couldn't figure out if it's right to addControl in load event? I also tried to do the same thing in styledata event, but it seems not work either.

Here is the code I use to reproduce the problem: https://gist.github.com/hijiangtao/a53a5922428331b63c44f6d2d7cb07d3

VarLog commented 5 years ago

Hello, @hijiangtao! I had the same problem with mapbox-gl-language plugin and react-mapbox-gl package. I looked through source code of the mapbox-gl-language and figured out that it is safe to call MapboxLanguage.prototype._initialStyleUpdate() directly.

The way that I use map is:

handleLoad = map => {
    const MapboxLanguage = require('@mapbox/mapbox-gl-language');
    const language = new MapboxLanguage();
    map.addControl(language);

    // force call style update
    language._initialStyleUpdate();
}

render() {
    return <MapComponent onStyleLoad={this.handleLoad} />
}

It is not the perfectest solution, but it is works :)

hijiangtao commented 5 years ago

Hi @VarLog . Thanks for your comments!

I found the problem may be caused by react-map-gl internals, and I fixed it with PR https://github.com/uber/react-map-gl/pull/704.

By the way, I also think it's not safe to call ._initialStyleUpdate() in your application, all private methods have the possibility of being changed in any future versions.

ryanhamley commented 3 years ago

Closing as solved