mapbox-gl-js icon indicating copy to clipboard operation
mapbox-gl-js copied to clipboard

Create a consistent convention for getters, setters, and constructor options

Open lucaswoj opened this issue 8 years ago • 11 comments

Many attributes of the map have getters, setters, and constructor options (for example, minZoom, getMinZoom, setMinZoom 👍 )

Some have only a subset of the three, even though it would be practical and useful to have all three. (for example, maxBounds and setMaxBounds exist but getMaxBounds does not) (for example, renderWorldCopies exists as a constructor option but we do not provide setRenderWorldCopies or getRenderWorldCopies).

We should do a review of all getters, setters, and constructor options to ensure that

  • for every constructor option, there exists a getter
  • for every constructor option that could be changed at runtime, there exists a setter
  • for every getter that could be set in the constructor, there exists a constructor option
  • for every setter, there exists a getter

We may want to create some infrastructure to automatically plumb together constructor options and getters.

lucaswoj avatar Jan 20 '17 19:01 lucaswoj

We may want to create some infrastructure to automatically plumb together constructor options and getters

@lucaswoj might be worth considering doing this auto-plumbing in conjunction with https://github.com/mapbox/mapbox-gl-js/issues/2741

anandthakker avatar Jan 20 '17 23:01 anandthakker

Opened https://github.com/mapbox/mapbox-gl-js/issues/4039 before I saw this issue!

mollymerp avatar Jan 23 '17 22:01 mollymerp

We may want to consider using ES6 getters / setters in leu of getFoo / setFoo methods.

lucaswoj avatar Jan 30 '17 19:01 lucaswoj

I'd love to have getters/setters for each properties. While this gets implemented, I found a small workaround for setting interaction on/off (currently only possible per constructor settings) and getMaxBounds (there is a setter, but strangely no getter for this).

In this snippet, map is an instance of mapboxgl.Map

// A collection of handler names
const mapInteractionHandlers = [
    'scrollZoom', 'dragPan', 'touchZoomRotate', 'dragRotate', 'keyboard', 'boxZoom', 'doubleClickZoom'
];

// Returns true if at least one of the above handlers is enabled
function isInteractionEnabled(map) {
    return mapInteractionHandlers.reduce((isEnabled, nextHandler) => {
        return map[nextHandler].isEnabled() && isEnabled;
    }, true);
}

// Enable map interaction (all handlers)
function enableInteraction(map) {
    for (let handler of mapInteractionHandlers) {
        map[handler].enable();
    }
}

// Disable map interaction (all handlers)
function disableInteraction(map) {
    for (let handler of mapInteractionHandlers) {
        map[handler].disable();
    }
}

// Returns true if the maxBounds have been set using the corresponding setter
function hasMaxBounds(map) {
    return (!Array.isArray(map.transform.latRange) || map.transform.latRange.length === 0) &&
        (!Array.isArray(map.transform.lngRange) || map.transform.lngRange.length === 0);
}

// Returns maxBounds from map.transform as Array<Array> if they are set
// or null
function getMaxBounds(map) {
    if (!hasMaxBounds(map)) {
        return null;
    }
    else {
        let transform = map.transform;
        let { latRange, lngRange } = transform;
        let ne = [lngRange[1], latRange[1]];
        let sw = [lngRange[0], latRange[0]];
        return [ne, sw];
    }
}

lamuertepeluda avatar Apr 18 '17 07:04 lamuertepeluda

@lamuertepeluda are you interested in submitting a PR based on that? It looks like you should be able to simply place this code within GL JS just adding JS doc and tests.

andrewharvey avatar Jun 25 '17 22:06 andrewharvey

I've attempted to implement a getMapBounds at #4890.

/cc @lamuertepeluda

andrewharvey avatar Jun 26 '17 10:06 andrewharvey

@andrewharvey thanks, I'm sorry I couldn't do it but I'm happy if somebody gets to integrate this stuff... I think it's useful to have all setters and getters coherently for each parameter.

I also hope the E6/7 setters/getters convention will be used in the next releases.

I'd also like a function for atomically set/get all or some map state parameters for serialization/deseralization and other purposes, e.g. something as

map.setState({
  zoom,
  center,
  pitch,
  bearing,
  boundingBox,
 ...
});

let mapState = map.getState();  // mapState = {zoom, center, pitch, bearing, boundingBox, ...}

This would be helpful in combination with state management libraries such as redux, or simply for storing and reading the map state in the local storage or remotely...

lamuertepeluda avatar Jun 26 '17 11:06 lamuertepeluda

so are the methods available? enableInteraction disableInteraction

rares-lupascu avatar Feb 28 '19 11:02 rares-lupascu

also curious about this, specifically interested in pitchWithRotate

waissbluth avatar Mar 24 '20 22:03 waissbluth

Seconding interest in a pitchWithRotate setter, as a previous approach to setting this value no longer works in v1.10.0. https://github.com/mapbox/mapbox-gl-js/issues/9666

allison-strandberg avatar May 07 '20 18:05 allison-strandberg

get/set cooperativeGestures would be nice. For example to disable it when the map takes up the full window (so no page scroll, but not fullscreen at the browser level).

andrewharvey avatar Jun 28 '22 04:06 andrewharvey