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

GeolocateControl - add option to hide the default button #3103

Open
swiss-chris opened this issue Sep 19, 2023 · 8 comments
Open

GeolocateControl - add option to hide the default button #3103

swiss-chris opened this issue Sep 19, 2023 · 8 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@swiss-chris
Copy link

swiss-chris commented Sep 19, 2023

User Story

As a developer, I can hide the default geolocate button that ships with the GeolocateControl, so that I can design and use my own button for this control (and use the .trigger() function on the GeolocateControl object to activate the geolocation).

Rationale

We provide a custom map for multiple software teams and this map comes with our own, custom styled buttons/controls in the top right corner of the map. We would like to offer the possibility of displaying a GeolocateControl button in our map as well, but currently the default button is hard-coded into the GeolocateControl. If I want the GeolocateControl without the default button, I have to copy paste the entire code file (~700 LOC) plus some additional utility functions from other files, and then remove all HTML and CSS code from within the copied TypeScript file. My proposal is to provide an option to hide this button (see below). The user can then call geolocateControl.trigger() to simulate clicking on the default button.

const defaultOptions: GeolocateOptions = {
    positionOptions: {
        enableHighAccuracy: false,
        maximumAge: 0,
        timeout: 6000 /* 6 sec */
    },
    fitBoundsOptions: {
        maxZoom: 15
    },
    trackUserLocation: false,
    showAccuracyCircle: true,
    showUserLocation: true,
    showButton: true, // <-- by setting this to 'false', the default button is not rendered on the map
};

Impact

Today, if I want the GeolocateControl without the default button, I have to copy paste the entire code file (~700 LOC) plus some additional utility functions from other files, and then remove all HTML and CSS code from within the copied TypeScript file. With my suggested change, a user could simply set the 'showButton' option to 'false', implement their own button, and when this button is clicked, simply call geolocateControl.trigger() on an instance of the official GeolocateControl class.

Notice the default geolocation button on the top left of this map, and the custom buttons in a different style on the top right of the map:
image

@HarelM
Copy link
Member

HarelM commented Sep 19, 2023

Adding this hideButton is not a great solution as it would litter the code with if (hideButton), which I would like to avoid.
If you would like to split the class into a "service" and the UI so that you can reuse the location service that's fine by me.
Note that the representation on the map is considered part of the UI from my point of view, but again, if you would like to create a "special animated map marker" that is used by the location control, that's perfectly fine too.
Bottom line, please propose an alternative architecture for the current control and open a PR for implementation once the design/architecture is approved.

@HarelM HarelM added enhancement New feature or request good first issue Good for newcomers labels Sep 19, 2023
@swiss-chris
Copy link
Author

Thanks for your response, @HarelM
I agree that sprinkling IF-statements all over the code should be avoided. However, I was hoping it might be possible to simply hide the whole button with a CSS class, so that the remaining logic could remain unchanged. The button would always be present in the HTML code, but not necessarily visible (depending on the new option).
If I find a way to do this, I will create a Pull Request with this simpler solution, instead of a full refactoring of the class into 2 classes as you suggested.

@HarelM
Copy link
Member

HarelM commented Sep 20, 2023

You can do it with css, there's no need to add anything to this library...

@neodescis
Copy link
Collaborator

neodescis commented Sep 20, 2023

Looks like this should work for you to hide the button:

.maplibregl-ctrl-geolocate { display: none; }

@bigDP
Copy link

bigDP commented Nov 27, 2023

Hi this story is still relevant?

@swiss-chris
Copy link
Author

Hi this story is still relevant?

I would love to see this feature implemented, but if not, we can close the story. For now we have copy pasted the code and adapted to our own needs, which works for the moment, but won't be sustainable in the long run.

@bigDP
Copy link

bigDP commented Nov 27, 2023

I want to take it

@HarelM
Copy link
Member

HarelM commented Nov 27, 2023

I'm not sure I fully understand the required design here.
I also think that this won't change a lot. I also think that it will not break easily as the public API/style-spec is pretty stable, so copying the code outside and maintaining it is not that bad of a solution.

I would advise to create a design for what is about to be implemented before opening a PR. once this design is approved a PR can be opened with the relevant changes.
From what it sounds this will affect the public API of maplibre-gl and I would prefer to make sure we consider it thoroughly as later on changing this public API is a pain, which I'm currently feeling when changing the public API of the Sources as part of version 4...

smellman added a commit to smellman/maplibre-gl-js-1 that referenced this issue May 18, 2024
smellman added a commit to smellman/maplibre-gl-js-1 that referenced this issue May 18, 2024
smellman added a commit to smellman/maplibre-gl-js-1 that referenced this issue May 18, 2024
smellman added a commit to smellman/maplibre-gl-js-1 that referenced this issue May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants