-
Notifications
You must be signed in to change notification settings - Fork 16
Add zooming when layer is added using <map-a> #387
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
Conversation
I don't see the zoomTo functionality that I was expecting with non-fragment text/mapml links (actually haven't tested fragments yet)? For example, this link: _self: (Default if no target attribute) replace the current layer - not zooming afaict |
That's odd, I got the behavior on my end: Could you try using the featureLink.html file and see if it works there? Run the test server |
Hard to tell if it's working for me cause I don't know what the links are supposed to do, but here's a recording: Media1.mp4 |
There was an error with |
Seems to be an issue with timing, the focus() function is called on a layer before the layer's extent is fully processed, although it's strange that the geogratis ones loaded in but geoserver ones didn't. |
This was a problem in early iterations. It may have been affected by this PR, perhaps: #366
possibly threading issues on the same computer cause that delay. |
Maybe there should be an event that fires when a layers extent is set, I don't believe there's an event fired from the map that lets you know the layer is fully added, do you know of any? Leaflet has |
I used to have the extentloaded event, not sure if that still happens. |
I also remember hacking with a labelchanged event. tbh I have not fully grasped asyn programming techniques, which is probably a major issue in this space. |
Seems to be working better now. One thing I did notice is that a link that points to a projection-negotiable layer will work (it eventually negotiates the correct projection), but it doesn't zoom to the extent of the resulting layer extent. |
Probably has to do with the |
First check of a fragment url such as:
It seems to zoom to 7, lon=98, lat=38.5, but it doesn't load the states layer. EDIT: it only pans at the current zoom, does not load the states layer. |
Seems like this link doesn't work like I was expecting, although it does something (zoom/pan to the specified location):
would correspond to the _self, below, I think, with the URL defaulting to the "URL" of the current layer, whether or not it is an actual document is another matter, I guess, since the content could be inline. We should be able to use fragments like the above with each of the target types below, I think. _self: (Default if no target attribute) replace the current layer |
I think in a conversion we had I believe you mentioned that if the type=text/mapml and target=_self it should "simply reposition the viewer with zoomTo()". It was decided that the fragments would only work with type=text/mapml, in which case using the URL of the layer in the case of inline features doesn't work since the URL of the layer is pointing to an html page. In the case of templated features, what value do you get from replacing the layer with itself vs just repositioning? As for the other target types (other than _self) I can implement those behaviors, I wonder if they're consistent behaviors with how fragments work in html though. |
Right, but the default is text/mapml, correct?
That's true, but I don't think it affects the use of that URL as the context for zoom/pan to operations? I may be missing something though. I believe the intent was "simply reposition the viewer with zoomTo()". The current behaviour as I mentioned does move the map but not to the coordinates specified. There's a console error as well:
I would not replace the layer with itself - for urls that begin with #, I would not try to resolve against a base url, I would just invoke zoomTo and hope for the correct result. In a browser if you put a fragment that doesn't have a corresponding id, the browser doesn't complain, it does nothing, which seems to be the appropriate response in case of error here too. |
|
So in the case of Side note, the issue with zoomTo() seemed to be that it only accepted numbers for zoom (lat, lng could be both numbers and strings), if you gave it a string for zoom ("12" instead of 12) it wouldn't work. |
Another note is that the |
In the case of
I would not try to load anything in this case. Could it be treated as a shortcut? The idea behind fragment identifiers is that they don't identify anything on a server at all, they are simply processed client side. So when there is a URL with a fragment, the URL parsing and resolution and loading takes place and ** then ** the fragment is processed. So if there is no url, I think it might be safe to assume / perform a zoomTo without otherwise attempting to process the url.
OK, that's important to remember and document especially when documenting I think I might have had the idea that you might not know the zoom, but you wanted to position the map over a location, but it does look a little funny. |
During a discussion it was decided that If the |
LGTM - great work @ahmadayubi, let's go! |
I can't get this URL to load properly, it replaces the whole page:
|
* Add zooming when layer is added * Add inplace attribute to <map-a> * Inplace fix * Allow for async layer adding * Add comments to event handling * Add functionality for keypress * Focus new layer added on projection changes * Fix link autofocus behavior * Add test for projection negotiation and hash link * Fix zoomTo + _self behavior * Treat all sole fragments, of any target type as target=_self * Add longer time to test
Zooms map to a link url fragment, if that doesn't exist then zoom to the layer extent.
If
inplace
attribute is present then it doesn't zoom.Closes #378