kepler.gl icon indicating copy to clipboard operation
kepler.gl copied to clipboard

[Bug] Custom Map Style loading failure

Open CalypsoR opened this issue 1 year ago • 14 comments

Describe the bug I am not able to load my Mapbox map style in Kepler.

To Reproduce Steps to reproduce the behavior:

  1. Go to 'Kepler.gl/demo'
  2. Select 'Base Map' tab
  3. Click on 'Add Map style'
  4. Paste style URL 'mapbox://styles/leon-sildano/clrq8rugo001g01qvbzpudmn4'
  5. Add the name 'Locala map'

Expected behavior Add style button should be selectable.

Screenshots Capture d’écran 2024-04-11 à 15 03 54

Desktop (please complete the following information):

  • OS: IOS
  • Browser Chrome
  • Version 123.0.6312.105 (Build officiel) (x86_64)

CalypsoR avatar Apr 11 '24 13:04 CalypsoR

Confirmed this is reproducible. Here's the stack trace with this url:

       Failed to load resource: net::ERR_FAILED
bundle.js:82 Error: Failed to fetch
    at bundle.js:19:128310
_onEvent @ bundle.js:82
bundle.js:419 Fetch API cannot load mapbox://mapbox.mapbox-streets-v7,examples.0fr72zt8. URL scheme "mapbox" is not supported.
(anonymous) @ bundle.js:419
bundle.js:419 Fetch API cannot load mapbox://sprites/examples/[email protected]. URL scheme "mapbox" is not supported.
(anonymous) @ bundle.js:419
bundle.js:419 Fetch API cannot load mapbox://sprites/examples/[email protected]. URL scheme "mapbox" is not supported.
(anonymous) @ bundle.js:419
3bundle.js:82 Error: Failed to fetch
    at bundle.js:19:128310
_onEvent @ bundle.js:82

akre54 avatar Apr 18 '24 20:04 akre54

@ilyabo @birkskyum Some leftovers from the maplibre transition?

ibgreen avatar Apr 18 '24 21:04 ibgreen

The bug with the "Add style" button not being clickable actually predates the work on maplibre transition.

This bug was i.e. reported here Nov 29, 2023:

  • https://github.com/keplergl/kepler.gl/issues/2458

The maplibre transition PR merged and published Dec 19-21, 2023

  • https://github.com/keplergl/kepler.gl/pull/2461

That said, MapLibre iirc also requires the style to be a normal http(s):// url instead of the mapbox:// - I think we can look up somewhere what the actual url to prepend is if the mapbox api license allow for it.

birkskyum avatar Apr 18 '24 22:04 birkskyum

Looks like the issue (or one issue) is that the confirmButton in the AddMapStyle modal is always disabled. It's checking for mapStyle.inputStyle.style which is always null since the input onChange listener only sets a url and not a style property. Also addCustomMapStyleUpdater is checking for state.inputStyle.id which is also null.

Making these changes seems to help:

diff --git a/src/components/src/modal-container.tsx b/src/components/src/modal-container.tsx
index a26e7579..77152824 100644
--- a/src/components/src/modal-container.tsx
+++ b/src/components/src/modal-container.tsx
@@ -434,7 +434,7 @@ export default function ModalContainerFactory(
               onConfirm: this._onAddCustomMapStyle,
               confirmButton: {
                 large: true,
-                disabled: !mapStyle.inputStyle.style,
+                disabled: !mapStyle.inputStyle.url,
                 children: 'modal.button.addStyle'
               }
             };
diff --git a/src/components/src/modals/add-map-style-modal.tsx b/src/components/src/modals/add-map-style-modal.tsx
index f5e27343..72703488 100644
--- a/src/components/src/modals/add-map-style-modal.tsx
+++ b/src/components/src/modals/add-map-style-modal.tsx
@@ -194,7 +196,7 @@ function AddMapStyleModalFactory() {
                 <InputLight
                   type="text"
                   value={inputStyle.url || ''}
-                  onChange={({target: {value}}) => this.props.inputMapStyle({url: value})}
+                  onChange={({target: {value}}) => this.props.inputMapStyle({url: value, id: 'Custom Style
' })}
                   placeholder="e.g. https://basemaps.cartocdn.com/gl/positron-gl-style/style.json"
                 />
               </StyledModalSection>

Of course, the icon is broken, but this appears to be the basic fix.

akre54 avatar Apr 19 '24 01:04 akre54

The getStyleImageIcon called from inputMapStyleUpdater is returning a Mapbox url even for non-mapbox basemaps. For instance, inputting the example url "https://basemaps.cartocdn.com/gl/positron-gl-style/style.json" gives me "https://api.mapbox.com/styles/v1/https://basemaps.cartocdn.com/gl/positron-gl-style/style.json/static/-122.3391,37.7922,9,0,0/400x300?access_token={snip}&logo=false&attribution=false"

akre54 avatar Apr 19 '24 15:04 akre54

It doesn't really make sense to continue supporting Mapbox URLs unless the actual Mapbox library is added as an alternative option

Let's decide that we are going to remove the mapbox URL protocol. However, to avoid future confusion, we should do it right.

There are a bunch of references to in docs and code to mapbox:// url protocol. There could also be some screenshots. If we no longer support it, that should be at least superficially cleaned up, in a quick PR.

The functionality to load mapbox V1 styles works after a bugfix. I don't feel like having a preview icon is enough reason to break this workflow. What do you think?

If we can keep support for V1 styles that would be preferable, but I am still confused as to what is supported in maplibre and what is not.

ibgreen avatar Apr 29 '24 20:04 ibgreen

Any mapbox v1 style will still be supported by maplibre, albeit when passed as http://, not mapbox://, because there has yet to be introduced a breaking change in the style spec. The style spec is in general quite stable and changes mainly by addition.

birkskyum avatar Apr 29 '24 20:04 birkskyum

So then it seems reasonable to not support mapbox:// for base urls, and instead prefer http resources.

I've seen a few cases where assets within map style jsons are loaded via mapbox://, which might necessitate rewriting the urls or dropping support, but I think fixing the loading is probably enough for now. My preference would be for this to happen in maplibre but I understand if / why that's less than desirable. We can revisit if there's enough interest. Hows that sound?

akre54 avatar Apr 29 '24 20:04 akre54

I've seen a few cases where assets within map style jsons are loaded via mapbox://

@akre54 that's exactly my case 😅 I have a base url defined as https://api.mapbox.com/styles/v1/${MAPBOX_USER_ID}/${STYLE_ID}?access_token=${MAPBOX_TOKEN}, but can't use it, since it uses mapbox:// assets and I have a very limited control over it to be able to reconfigure styles. Is there anything I can do to help implementing this fix?

ph-kamil-galladzhov avatar Sep 16 '24 18:09 ph-kamil-galladzhov

to be able use mapbox:// in maplibre, can use request transformer like this https://github.com/rowanwins/maplibregl-mapbox-request-transformer

sahitono avatar Oct 02 '24 03:10 sahitono

Great find @sahitono. Would you be able to open a pull request?

akre54 avatar Oct 02 '24 07:10 akre54

Any updates on this one? Are you going to open a PR that fixes it using maplibregl-mapbox-request-transformer?

MoustafaWehbe avatar Oct 07 '24 08:10 MoustafaWehbe

Hi @akre54 , i just opened the PR #2693 but not sure where is the maplibre map initialize. would be helpful if maybe you can show me where is the maplibre map initialize

sahitono avatar Oct 14 '24 03:10 sahitono

Nice! Thanks for adding this.

What do you mean by the maplibre map initialize?

Kepler uses react-map-gl to construct the base maplib. If you need to call a function on load, for instance, you can use the onLoad callback.

akre54 avatar Oct 14 '24 04:10 akre54