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

Is there an API I can use for calling setLayoutProperty method on the map instance? #683

Closed
hijiangtao opened this issue Dec 27, 2018 · 19 comments

Comments

@hijiangtao
Copy link
Contributor

The mapbox API provides setLayoutProperty method for developers to change the layout property of a map instance, such as the language:

According to mapbox API, I can use it in this way:

var map = new mapboxgl.Map({
    container: 'map',
    style: 'mapbox://styles/mapbox/light-v9',
    center: [16.05, 48],
    zoom: 2.9
});

// and the new property value.
map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh']);

I am developing project with react-map-gl and deck.gl, I use StaticMap exactly in my project and need to change the display language of maps.

But after looking through the docs of react-map-gl and StaticMap file, I couldn't find a method to achieve that.

I found that StaticMap provides a method called getMap():

https://github.com/uber/react-map-gl/blob/cb173b0ac405c42200ddfdaad5a6bed96d665fbf/src/components/static-map.js#L174-L176

But when I use this method to make changes to map instance, there's nothing happened.

const map = this.refMap.current.getMap();

map.on(
	'load', 
	() => map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh-Hans'])
);
...

render() {
	return (
		<StaticMap ref={e => {this.refMap = e;}} >
	);
}
@hijiangtao
Copy link
Contributor Author

I tried again and found the load event listener callback function had never been called, is it right to use getMap() method to get the map instance?

Mapbox GL JS API give a way to listen load event like this:

var map = new mapboxgl.Map({
    container: 'map',
    style: 'mapbox://styles/mapbox/light-v9',
    center: [-90.96, -0.47],
    zoom: 8
});

map.on('load', function () {});

@Pessimistress
Copy link
Collaborator

  • Yes, getMap is a public API. Though your code seems to be confusing the legacy callback ref and createRef from React.
  • You can use the onLoad prop to subscribe to the load event:
onLoad={evt => {
  const map = evt.target;
  // do something
}}
  • Your call to map.setLayoutProperty() fails most likely because there is no layer with the id country-label-lg. You can see all the layers in your current style by:
onLoad={evt => console.log(evt.target.getStyle().layers)}

@hijiangtao
Copy link
Contributor Author

hijiangtao commented Dec 28, 2018

@Pessimistress Sorry for the typos, I used this.staticMap = React.createRef(); in constructor, so I should update the codes above.

...
ref={this.staticMap}

For the third tip, I think country-label-lg is a basic layer for the default map, since I didn't make custom layer from mapbox studio. Besides, according to Change a map's language and I set the map.setLayoutProperty() operation with setTimeout, it can work well as expected.

And for the second tip, I found onLoad uses map.once API to listen the load event only once, however, I am not sure if mapbox labels will update after the first load event, so the codes below didn't work for me:

// this.addControlHandler
addControlHandler = () => {
    const map = this.staticMap.current.getMap();
    // my setLayoutProperty operations
  }

// codes in render() method
<StaticMap
            ref={this.staticMap}
            onLoad={this.addControlHandler}
            reuseMaps
            mapStyle="mapbox://styles/mapbox/dark-v9"
            preventStyleDiffing
            mapboxApiAccessToken={MAPBOX_TOKEN}
/>

Would properties passed to StaticMap like reuseMaps or preventStyleDiffing be the problems?

I am also confused why StaticMap use .once rather than .on here:

https://github.com/pranspach/react-map-gl/blob/3c426aace9044fcef293c5313f2ae5c84a44c3ca/src/components/static-map.js#L148

Another confusing point is would that cause some problems if I use map.on() when I get map instance? Since I found codes in react-map-gl declared:

https://github.com/pranspach/react-map-gl/blob/3c426aace9044fcef293c5313f2ae5c84a44c3ca/src/components/static-map.js#L150

https://github.com/pranspach/react-map-gl/blob/3c426aace9044fcef293c5313f2ae5c84a44c3ca/src/components/static-map.js#L191-L194

And you also attach event listener on this variable:

https://github.com/pranspach/react-map-gl/blob/3c426aace9044fcef293c5313f2ae5c84a44c3ca/src/components/static-map.js#L148

But it seems this code won't work for me:

map.on(
	'load', 
	() => map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh-Hans'])
);

I wonder if there is some extra configurations for StaticMap that prevent me to listen on it?

Thanks :-)

@hijiangtao
Copy link
Contributor Author

hijiangtao commented Dec 28, 2018

It seems I look an PR of this repo and directs the codes to a different developer's repo, but the .once calling can be found here, too.

https://github.com/uber/react-map-gl/blob/master/src/mapbox/mapbox.js#L282-L283

And to prevent the things you mentioned above, I used mapbox official package to change layoutProperty for me @mapbox/mapbox-gl-language and upadte the addControlHandler method in my project:

addControlHandler = () => {
    const map = this.staticMap.current.getMap();
    map.addControl(new MapboxLanguage({
      defaultLanguage: 'zh',
    }));
  }

It seems I would not modify a layer that not exist anymore, but the problem still exists.

Currently, I can passby this problem (can't modify mapbox's layout property) by adding the same operation to componentDidUpdate lifecycle in react component:

componentDidUpdate() {
    const map = this.staticMap.current.getMap();
    map.addControl(new MapboxLanguage({
      defaultLanguage: 'zh',
    }));
  }

@Pessimistress
Copy link
Collaborator

First of all, if you're using react-map-gl from the fork that you linked to, it is extremely outdated (mapbox-gl dependency on 0.37). I cannot speak to whether anything still works with that version.

Regarding your failed attempt at using map.on -- You may want to file an issue for mapbox-gl if you can confirm that once and on do not work simultaneously. But I have a hard time understanding why you cannot use the onLoad callback.

I am not sure if mapbox labels will update after the first load event, so the codes below didn't work for me

I'm fairly certain that load only fires once, per Mapbox's documentation: https://www.mapbox.com/mapbox-gl-js/api/#map.event:load

I just tried this and it works perfectly:

https://github.com/uber/react-map-gl/blob/set-language-example/examples/set-language/app.js

@hijiangtao
Copy link
Contributor Author

hijiangtao commented Dec 29, 2018

@Pessimistress Thanks for your codes and detailed explanations!

I am using the latest react-map-gl and I just use the forked repo to show the logic of .once().

I think .on and .once() are equal if load only fires once, and the reason why I didn't use onLoad callback is that I found I couldn't get the target sometimes through .onLoad callback input params, so I use this code to get map instance and listen to load event at componentDidUpdate lifecycle:

const map = this.staticMap.current.getMap();

I think I tried the same way to listen the map load event, but I would get event.target to be null sometimes:

https://github.com/uber/react-map-gl/blob/set-language-example/examples/set-language/app.js#L15-L18

_onLoad(event) {
    const map = event.target;
    map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh'])
}

Then the calling to map.setLayoutProperty method would throw an error.

A different thing that in my code is I import Staticmap, rathen than import from default map layer in your code:

// my code
import {StaticMap} from 'react-map-gl';

// your code
import MapGL from 'react-map-gl';

I replace StaticMap with the default map layer, and errors won't exist anymore. I wonder if there is some mechanism in StaticMap differs from MapGL, that causes my problem? (And I really doubt the problem I met may caused by some static process inside Staticmap. Also, the problem I mentioned above would happen more frequently when page needs to load much resources.)

As I said before, currently, I can passby this problem (can't modify mapbox's layout property) by adding the same operation to componentDidUpdate lifecycle in react component:

@Pessimistress
Copy link
Collaborator

What do you mean by "I would get event.target to be null sometimes" -- is it called multiple times during runtime? Is event.target null or event undefined (invoked by reuseMaps: https://github.com/uber/react-map-gl/blob/master/src/mapbox/mapbox.js#L247). What else changed when you swap StaticMap for InteractiveMap?

Well you can always do this:

_onLoad(event) {
    // const map = event.target;
	const map = this.staticMap.current.getMap();
    map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh'])
}

@hijiangtao
Copy link
Contributor Author

hijiangtao commented Jan 1, 2019

  1. The event.target could be null sometimes;
  2. As I described above, I just changed the import variable and without any other modification:
// before
import {StaticMap} from 'react-map-gl';

return (
	<StaticMap 
		// props
	>
);

// after
import MapGL from 'react-map-gl';

return (
	<MapGL 
		// props
	>
);

Since my project couldn't verify the runtime of firing onLoad event at this time, maybe I would try it later and give more information about the wired null. :-)

@hijiangtao
Copy link
Contributor Author

I tried again. It's pretty wired that I only caught the problem sometimes.

I use map.addControl method to change Layout properties instead this time.

// mapbox official language switch package
import MapboxLanguage from '@mapbox/mapbox-gl-language';

// the method I called in StaticMap 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']);
    }
  };

I print the event and event.target out and it seems even when it's not null, the method map.addControl will not work well as expected (nothing change on the map) sometimes.

I just add .setLayoutProperty into work (the comment line), and the .addControl method will also work well. Did react-map-gl handles something I should forbid when I use these two methods inside map instance, or is it some internal problems that I should ask mapbox for help?

@hijiangtao
Copy link
Contributor Author

@Pessimistress I quickly look through the things I mentioned above, and I thought it's not a problem of specific map like StaticMap but should belongs to an event problem.

It seems .setLayoutProperty is a method for handling things above a map style layer?

Sets the value of a layout property in the specified style layer. from https://docs.mapbox.com/mapbox-gl-js/api/#map#setlayoutproperty

And looking into the definition of mapbox 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

I am not sure whether the necessary resources mentioned in load event will include style resource that methods like .setLayoutProperty may need, but I prefer it's not included since there's a public event called styledata:

Fired when the map's style loads or changes. See MapDataEvent for more information. from https://docs.mapbox.com/mapbox-gl-js/api/#map.event:styledata

So I thought the reason why such situation I mentioned above (also mentioned in mapbox-gl-js issue mapbox/mapbox-gl-language#27 ) would appear sometimes is that I use setLayoutProperty in a wrong time of mapbox's lifecycle.

Currently, according to documents, react-map-gl only provides three events: onLoad, onResize and onError, but it seems developers may need more event types such as styledata, data, etc, if they want to control the map more precisely.

So on the hand, I thought react-map-gl may provide another event handler like styledata, and that could solve the problem I mentioned in this issue.

@Pessimistress
Copy link
Collaborator

Fix published in 4.0.10.

@hijiangtao
Copy link
Contributor Author

hijiangtao commented Jan 29, 2019

I think it's not a problem of specific map like StaticMap but should belongs to an event problem, so it’s not resolved yet (according to my last comment in this issue)?

@Pessimistress
Copy link
Collaborator

Can you create a minimal example that reproduces this issue?

@hijiangtao
Copy link
Contributor Author

hijiangtao commented Jan 29, 2019

I don't know if you can reproduce it in your computer, because it also appeared sometimes in my environment. Here's the code https://gist.github.com/hijiangtao/a53a5922428331b63c44f6d2d7cb07d3:

I make my problem more clear in this way:

  1. Problem: I want to change all labels in map to Chinese, so I use mapbox-gl-language package and follow the usage of official example (they use vanilla JavaScript, rather than react) https://github.com/mapbox/mapbox-gl-language/blob/master/examples/zh.html#L28-L30

  2. Result: The .addControl method just didn't work, the map keeps silence all the time without any change.

  3. Hacked Method: The only way to solve it is set this line to run in my code:

    map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh']);

  4. Reason: The above code will only change country label in map, but not include other labels such as road, river, lake, etc. But after run it, almost all the labels changed, so I can confirm the above code will trigger new MapboxLanguage() in some way, and MapboxLanguage help me to change all other labels to chinese.


NOT RELATED WITH ABOVE ITEMS: Besides, as I mentioned above #683 (comment) , I suggest react-map-gl to provide another event handler like styledata, or just let developer to register customized event handler, it may let the package be more scaleable. I can raise an issue to describe it in detail if you do have plans to accept features from community.

@Pessimistress
Copy link
Collaborator

Just saying, it is probably way easier if you just create a custom map style that uses Chinese for all label layers.

@hijiangtao
Copy link
Contributor Author

Maybe there are too much trivial things need to be handled, since there are too many label types that I need to modify (specify the name value one by one)? And mapbox studio may be the only way I could customize map layer. 😐

@Pessimistress
Copy link
Collaborator

@marciobarrios
Copy link

@hijiangtao I am in the same situation, only using setLayoutProperty triggers a complete language change using MapboxLanguage, it's not ideal but at least it works, thanks.

@hijiangtao
Copy link
Contributor Author

@marciobarrios It's too long from I first reported this problem, I think there's some problems (maybe not work sometimes with only just MapboxLanguage ) that need to be solved but I need to spend some time to locate the exact problem.

You can locate the problem if you are not too busy right now, and please let me know if there's any progress on it, I am glad to help solve it together.

I looked through my codes, and find I can make my codes work with addControl (with MapboxLanguage) and setLayoutProperty both calling at a time (map is a map instance created by mapbox):

const addMapControl = (map) => {
  map.addControl(new MapboxLanguage({
    defaultLanguage: 'zh',
  }));
  map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh']);
}

Just as I said in reply

I don't know if you can reproduce it in your computer, because it also appeared sometimes in my environment. Here's the code gist.github.com/hijiangtao/a53a5922428331b63c44f6d2d7cb07d3:

I make my problem more clear in this way:

  1. Problem: I want to change all labels in map to Chinese, so I use mapbox-gl-language package and follow the usage of official example (they use vanilla JavaScript, rather than react) mapbox/mapbox-gl-language:examples/zh.html@master#L28-L30
  2. Result: The .addControl method just didn't work, the map keeps silence all the time without any change.
  3. Hacked Method: The only way to solve it is set this line to run in my code:

    map.setLayoutProperty('country-label-lg', 'text-field', ['get', 'name_zh']);

  4. Reason: The above code will only change country label in map, but not include other labels such as road, river, lake, etc. But after run it, almost all the labels changed, so I can confirm the above code will trigger new MapboxLanguage() in some way, and MapboxLanguage help me to change all other labels to chinese.

NOT RELATED WITH ABOVE ITEMS: Besides, as I mentioned above #683 (comment) , I suggest react-map-gl to provide another event handler like styledata, or just let developer to register customized event handler, it may let the package be more scaleable. I can raise an issue to describe it in detail if you do have plans to accept features from community.

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

No branches or pull requests

3 participants