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

Setters for zoom, lat, lon should change / move the map location #588

Closed
prushforth opened this issue Nov 22, 2021 · 1 comment
Closed
Assignees
Labels
API Map and associated elements' non-declarative API (events and methods) bug simplification web platform compatibility

Comments

@prushforth
Copy link
Member

There is the zoomTo method to change all three at once, yes. But from an API perspective, if these attributes and properties aren't read-only, setting them should have a visible effect. I don't recall if there's an actual reason that they don't, but if it's a problem we should document that, or fix it.

@prushforth
Copy link
Member Author

Our discussion this morning we proposed to (continue to) make lat,lon and zoom IDL attributes reflect to the content attributes of the same name. This seems to be demanded by the Web Platform Design Principles Section 3.4. We also agreed that the getters and setters named lat, lon and zoom should (continue to) not update the map (cannot, because we will create an infinite loop and consequent stack overflow, we believe), but will update the content attributes of the same names. That is currently the case.

In keeping with the HTMLMediaElement.defaultMuted <video muted> relationship, we have considered creating three new, differently named getters/setters: HTMLMapmlViewerElement.defaultLat, .defaultLon, and .defaultZoom, which would update the content attributes named lat, lon and zoom respectively (and vice versa), but which would not update the map state.

In our current design and implementation, JavaScript can only change the visual state of the map by using the .zoomTo(lat,lon,zoom) IDL method, which not only updates the map view, but also reflects post-move values to the lat,lon,zoom IDL and content attributes. Neither the content nor the IDL attribute setters change the dynamic state of the map.

We are not sure how useful creating new default* IDL attributes would be (without other changes, discussed below), as it simply creates more names for the developer to think about and remember what they do. The new setters would still not change the view (visual state) of the map. Creating these new IDL attributes might make more sense if we renamed the existing content attributes lat, lon and zoom to defaultlat etc., which would leave the lat, lon and zoom IDL attributes free to update the map state. The latter approach seems to be recommended by this commenter:

change the API to with a defaultValue IDL attribute, then have a value IDL attribute (with no corresponding content attribute) for the current value, which is modified by user input.

But we don't like this path either, because 'lat' and 'lon' are already abbreviations, so defaultlat and defaultlon look funny and may be (even) hard(er) to understand for non-native english speakers, among others, so we would likely want to rename them to defaultlatitude/defaultLatitude, defaultlongitude/defaultLongitude and defaultzoom/defaultZoom . However, this approach would leave us with lat, lon and zoom (or latitude, longitude and zoom) IDL attributes free to update the map state. Unfortunately, moving the map along one geocentric axis direction does not seem particularly useful, and may be potentially even less useful on projected coordinate reference systems in which the latitude and longitude axes are not parallel to the easting and northing axes. Even if this behaviour is desired, it can be accomplished with the zoomTo(lat,lon,zoom) method, by varying one parameter while keeping the others stable. In sum, we probably don't want to do what the subject of this issue suggests, but perhaps we will want to rename existing content/IDL attributes if there is pushback about "lat" etc. No need to be hasty, anyway.

Having the ability to change the map viewer state via a setter or method with no corresponding content attribute follows the design of the HTMLMediaElement.muted IDL setter attribute, which affects the state of the player (visually and audibly), but despite appearances, has no corresponding content attribute. Note that setting HTMLMediaElement.muted = true does not change the HTMLMediaElement.volume value, it only changes the visual and audible state of the player (presumably, the .volume is retained in order to reset player state when unmuting).

Finally, in most cases changing the content attributes should not have an effect on the map, according to this comment:

My takeaway would be that using content attributes to control dynamic state is a bit fraught and should be thought about carefully. Whereas using IDL attributes or methods is relatively safe.

Which is advice we should take into consideration when we review or create other attributes.

Closing, will close the associated PRs and issues, too:

Maps4HTML/web-map-doc#116
#796
Maps4HTML/MapML-Specification#239
w3ctag/design-principles#424

Thanks for your work, my sincere apologies for your wasted effort. I guess we learned a few things though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Map and associated elements' non-declarative API (events and methods) bug simplification web platform compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants