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

Support for raw style JSON #812

Open
tsemerad opened this issue Nov 28, 2017 · 7 comments
Open

Support for raw style JSON #812

tsemerad opened this issue Nov 28, 2017 · 7 comments

Comments

@tsemerad
Copy link
Contributor

I'm curious if there are plans or considerations to allow passing a full style JSON into the component, where the component intelligently diffs the previous and next style to transition the map to the new style.

Such functionality is in common use in Mapbox GL JS when used with React (such as in Uber's react-map-gl, or as described in this Mapbox blog post), so I believe supporting this functionality in RNMBGL would be a big win for cross-platform codebases.

Mapbox GL JS has a built-in diffStyles function where you give it the current style and the next style, and it will return the necessary commands that need to be made to transition the map to the next style. This Mapbox blog post goes over it a bit.

I'm utilizing it in my own Mapbox GL JS components by using the following componentWillReceiveProps:

componentWillReceiveProps(nextProps) {
  const currentStyle = this.props.mapboxStyle
  const nextStyle = nextProps.mapboxStyle
  if (!this.state.initialMapboxStyle && nextStyle) {
    this.setState({ initialMapboxStyle: nextStyle.toJS() })
  } else if (!this._map && nextStyle) {
    // If the style changes before the map is mounted, update initialMapboxStyle
    this.setState({ initialMapboxStyle: nextStyle.toJS() })
  }

  if (this._map && currentStyle && nextStyle && !currentStyle.equals(nextStyle)) {
    const changes = diffStyles(currentStyle.toJS(), nextStyle.toJS())
    for (const change of changes) {
      try {
        if (change.command === 'setData') {
          const [sourceId, geoJson] = change.args
          this._map.getSource(sourceId).setData(geoJson)
        } else {
          this._map[change.command](...change.args)
        }
      } catch (error) {
        // if a change fails, fall back to setting the full style
        this._map.setStyle(nextStyle.toJS())
        break
      }
    }
  }
}

My team investigated implementing runtime styling in this manner in RNMBGL back when runtime styling was first implemented in the iOS and Android SDKs. This issue (#416) goes over it a bit. We ended up exposing addLayer, removeLayer, addSource, and removeSource on the map component for now so that we could utilize diffStyles like we do in Mapbox GL JS, but we think that ultimately the RNMBGL component could diff the passed in style JSON objects itself.

@1ec5's comment near the end, #416 (comment), explains where implementing such a feature was held up by the iOS SDK. It looks like this is the one blocker remaining.

Anyway, I know this would be a major change, but I wanted to bring it up again to see if there's still other developers who would like to see such a feature. Thanks for your consideration.

@nitaliano
Copy link
Owner

nitaliano commented Nov 28, 2017

To me this does seem like it would be possible to do now with a component that wraps MapboxGL.MapView. It would be able to keep track of it's current raw style, so it would be able to diff it's next style and know how to convert that code into something that the MapboxGL.StyleSheet can understand. It would know about all of it's children, and would be able to update the render method intelligently.

It could even call the mapbox-gl-js diff function directly and the component on top of MapboxGL.MapView could convert it to what the RN code is expecting as well.

@1ec5
Copy link
Contributor

1ec5 commented Nov 30, 2017

mapbox/mapbox-gl-native#9256 implemented smooth transitions between partially differing styles. mapbox/mapbox-gl-native#6386 would indeed block the most straightforward implementation of what you’re requesting, but in the meantime, it might be possible for RNMBGL to circumvent the runtime styling API and set the style to a temporary JSON file generated on the JavaScript side using diffStyles.

@tsemerad
Copy link
Contributor Author

tsemerad commented Dec 1, 2017

Thanks for the responses.

It would be able to keep track of it's current raw style, so it would be able to diff it's next style and know how to convert that code into something that the MapboxGL.StyleSheet can understand. It would know about all of it's children, and would be able to update the render method intelligently.

What you describe here sounds like we'd create a MapboxGL.StyleSheet in response to passed in style JSON. If there isn't a problem with constructing new MapboxGL.StyleSheets over and over again throughout the component's lifecycle, this could work, but it would still require the end-developers like myself to maintain a "conversion" function that converts any possible style JSON into its equivalent MapboxGL.StyleSheet API (providing a constructor function such as MapboxGL.StyleSheet.fromJSON would be a godsend for such a scenario). More importantly, with the current API, is it possible to set a single MapboxGL.StyleSheet that describes all styles for all layers and all of their sources (basically, the equivalent of the full raw style JSON needed to render a Mapbox GL map)? It looks like the API is currently designed to just add additional layers on top of an existing style (which I'm sure is a fine limitation for many devs). But if I want to control the entire style in my redux state, it looks like my best option is to try to initialize my map with a mostly empty style to start off with, then render a separate MapboxGL.LineLayer, MapboxGL.ShapeSource, etc. for all 100+ layers in my full style JSON. I'll let you know if I end up trying that. And let me know if you know of a better way.

It could even call the mapbox-gl-js diff function directly and the component on top of MapboxGL.MapView could convert it to what the RN code is expecting as well.

I don't think the current MapboxGL.MapView and MapboxGL.StyleSheet work well with the existing mapbox-gl-js diffStyles function, since that function returns an array of imperative "commands" that need to be performed (such as add layer, remove layer, add source, etc). I have gotten diffStyles to work well enough by exposing addLayer, removeLayer, etc on my fork, but of course exposing those functions on the map component isn't a good idea since it is imperative, and the MapboxGL.MapView itself should only be manipulating the styles internally.

@1ec5 said

it might be possible for RNMBGL to circumvent the runtime styling API and set the style to a temporary JSON file generated on the JavaScript side using diffStyles

diffStyles doesn't generate a JSON file of the new style, but rather an array of commands to be run to transition to the new style.

@tsemerad
Copy link
Contributor Author

tsemerad commented Dec 1, 2017

@1ec5 You mentioned passing a "temporary JSON file" to the native SDK - are you saying that RNMBGL could pass a full style JSON and the iOS SDK could then transition the map to that style? The first link you posted, mapbox/mapbox-gl-native#9256, implies that that might be possible. If so, that would save me a lot of effort, as I'd no longer need a conversion function to convert the full mapbox gl style spec to output to the RNMBGL API.

@1ec5
Copy link
Contributor

1ec5 commented Dec 1, 2017

diffStyles doesn't generate a JSON file of the new style, but rather an array of commands to be run to transition to the new style.

You’re right, and it wouldn’t be necessary anyways because, in your use case above, the application code would come up with the final state of the style.

You mentioned passing a "temporary JSON file" to the native SDK - are you saying that RNMBGL could pass a full style JSON and the iOS SDK could then transition the map to that style? The first link you posted, mapbox/mapbox-gl-native#9256, implies that that might be possible. If so, that would save me a lot of effort, as I'd no longer need a conversion function to convert the full mapbox gl style spec to output to the RNMBGL API.

Yes, I haven’t tried this in RNMBGL to know how well it performs, but with the native SDK, you could write out a temporary file containing the entire style JSON, then call -[MGLMapView setStyleURL:] to switch to it. It’s a bit of a weird API, but MGLStyle.transitionOptions controls the fade transition between the old and new styles.

@nitaliano
Copy link
Owner

nitaliano commented Dec 1, 2017

@tsemerad you can update the StyleSheet object as many times as you want, it's just creating a JSON payload so the native code can understand what it's trying to render. Then once it hits the native code I just loop thru the styles and update what needs to be changed.

As, for the component on top of MapboxGL.MapView this would be something that Mapbox would provide as a react component with actions and reducers. Right now when working with redux, I have all my actions listed out as addSource, addLayer, removeLayer, removeSource, removeLayer, updateLayer and updateSource . I'm just working with JSON for my map and passing in JSON style objects and it's smart updating under the hood, so it's not a long shot away from being able to have a setStyle action. MapboxGL.StyleSheet.fromJSON also sounds like a great idea!

@1ec5 currently when the style url is changed I remove all source and layers and re add them back to the new style once it is ready, so you can think of all the children under the map component as what you described above

@1ec5
Copy link
Contributor

1ec5 commented Dec 4, 2017

currently when the style url is changed I remove all source and layers and re add them back to the new style once it is ready, so you can think of all the children under the map component as what you described above

Interesting, so you’re working around mapbox/mapbox-gl-native#6180, mimicking what the annotation API does. Unfortunately, I think removing and readding the layers and sources would defeat the smooth transitions between styles, because Mapbox GL doesn’t support transitioning the addition or removal of a layer or source (or style image).

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