Expose CANVAS class in constants

[neodescis]

The intent is ultimately to close #1182.

To begin, I've pulled in the one mapbox-specific CSS class in the JavaScript code not yet in the list of constants. Thanks to #1100, these can already be updated readily before instantiating MapboxDraw, e.g.:

MapboxDraw.constants.classes.CONTROL_BASE = "maplibregl-ctrl";
MapboxDraw.constants.classes.CONTROL_PREFIX = "maplibregl-ctrl-";
MapboxDraw.constants.classes.CONTROL_GROUP = "maplibregl-ctrl-group";
...

If using @types/mapbox__mapbox-gl-draw, the current typings complain about this as they are marked readonly there (needing a @ts-ignore to compensate), so it would probably be good to create a pull request there too.

This doesn't make the library functional with MapLibre out-of-the-box though. The CSS distributed with mapbox-draw depends on the original values of the class constants, as well as mapbox class names. I'm not entirely sure what we would want to do about that either, though. Any thoughts, @HarelM or @stepankuzmin?

[HarelM]

To replay to what you wrote:

1. Changing the constants from being readonly to not readonly is needed.
2. The initialization when using MapLibre should be documented in the MapLibre draw example - this library is Mapbox first, and until this is changed, we need to document how to use it with MapLibre. I don't see a way around that unless we want to introduce here an option of some kind to let the library know which one is being used (i.e. have another constant classes object that this library will use in case of a parameter sent in the initialization of the plugin - i.e. useMaplibre or something). Which means that it supports both Mapbox and MapLibre out of the box with no additional "hacking".
3. In continue to the above, one option is to have the classes doubled - i.e. have both mapbox and maplibre in the css. Another option is to do the dynamic css part that is mapbox/maplibre specific in the code and not in the css, thus allowing the dynamic nature of it. Since we are not talking about a lot of classes I think this is a possible solution.
4. If 3 is not possible one could create a css and host it on gist or something to support mapbox draw "missing" MapLibre css. this however will probably break at one point or another.

My two cents anyway, but I'm not maintaining this lib and I don't know if the out of the box support for MapLibre is welcomed or not. I would say that there are a lot of people who are using MapLibre with this lib (I got a large number of tickets on it in MapLibre's repo) and I believe they will want to contribute to this lib as well instead of forking, I would also like to avoid forking if possible.
Note that react-map-gl is also supporting both MapLibre and Mapbox so we could potentially learn from them.
You can see here that the docs has both maplibre and mapbox reference:
https://visgl.github.io/react-map-gl/docs/api-reference/map

[neodescis]

Well said. I think you hit all of the points I've been thinking of. So, I guess the question is whether Mapbox Draw wants to support MapLibre out of the box. If so, I would be happy to continue this work down that path. If not, then I think this concludes the work in this repo at least and I can mark this pull request as ready. @stepankuzmin, what do you think?

Regardless, the TypeScript typings will need to be updated to enable setting the class names, so I can work on a pull request for that. The MapLibre example will also need to be updated once we've decided how much "hacking" will be required.

[birkskyum]

@neodescis , thanks. The MapLibre example lives here: https://github.com/maplibre/maplibre-gl-js/blob/main/test/examples/mapbox-gl-draw.html

[gowin20]

+1 to maintaining MapLibre support out of the box in this plugin. I use Mapbox GL Draw frequently in my MapLibre apps, and I know several others who do the same. I'd be happy to contribute to this issue as well in order to continue maintaining compatibility.

[stepankuzmin]

Hi all! Thanks for the PR, @neodescis. Overall, I 👍 approach and appreciate your contribution. However, we'd need to address the constants typings.

[mike8161990]

Any movement on this? I'm seeing the same bug and would love if this fix was incorporated.

[neodescis]

I have not had the time to continue working on this. The way I see it, there are two additional things to resolve:

1. Either we need to update the typings for Mapbox Draw to allow the "constants" to be settable, or we need some other way of making them configurable.
2. We need some way to have the CSS's class names use the constants. Maybe this means moving the CSS into the JavaScript code, so that it can be generated on the fly?

However, I do not think those necessarily need to be handled in this pull request, so perhaps this could be merged as-is?

[stepankuzmin]

Hey all! I'm going to merge this. Again, thanks for the contribution, @neodescis