Allow Multiple extents #632
Conversation
There was a problem hiding this comment.
Ideally each mapml-layer-extent would be exposed the same to ATs as the layers are, i.e.:
- Change
divtofieldset - Have the
fieldsetlabelled (aria-labelledby) by the correspondingmapml-layer-item-name
Also, I think it'd be desired to have all mapml-layer-extent elements themselves be children of a group (<fieldset>) which is labelled (using <legend> or aria-label, probably the latter to keep it consistent) "Sublayers" (or "Extents"?).
|
|
@Anshpreet8 they state:
I don't know if that's the expectation, but even if that's true I don't see how that's a problem.
That's an incorrect statement, if a fieldset is labelled - as suggested - the user will know which inputs belong to that fieldset. I say go ahead 👍🏼 Edit: not to mention that the HTML spec has an example of nested fieldsets: https://html.spec.whatwg.org/multipage/form-elements.html#:~:text=You%20can%20also%20nest%20fieldset%20elements |
Anshpreet8
force-pushed
the
multipleExtents
branch
from
238d24e to
faaa475
Compare
|
The issue with zoom to layer is that the The copy extent menu item also uses the parent The The functions referenced are: The contextmenu event is also per layer item, so anything inside the layer item is for the layer (the menu you see is for the associated layer item not the extent), you'd have to attach new event handlers for the multiple extents |
undefined, see this [article](https://codeburst.io/javascript-null-vs-undefined-20f955215a2#5317)).
prushforth
force-pushed
the
multipleExtents
branch
from
523aebb to
aa0f4a4
Compare
|
The issue with Zoom to layer was, as @Anshpreet8 suggested, an issue with extractInputBounds on the main branch, which has been fixed and merged into this branch. |
Remove TemplatedTileLayer.addTo function (redundant).
Remove getCombinedExtentsLayerBounds, make calls for this behaviour go through _setLayerElExtent only.
map-meta name=extent, used as the extent in case there are no map-extent elements with their inputs specifying native bounds for content.
load slowly and which fire the `extentload` event are handled properly, which is more common with `<layer- src="...">` layers, and which are created (typically though not exclusively) through drag and drop.
map.removeLayer anyway)
_extent._queries property was ignored during layer remove events, fixed.
|
Hi @Anshpreet8 This PR is very solid work, thank you! Sorry to have taken so long to properly review, most changes are minor and to help the reader (me) understand the intent.
Can you clarify for me what's left to do for the unchecked items above? I will try to address them if they still need attention. The only thing that stands out to me was the need to cycle the debug mode off / on to reflect extent removal/addition. I will take a look to see if that can be easily fixed. I may just merge this PR and log an issue for later so that we can move on to other work. |
Hi @prushforth , when there were was a feature query extent and a non-feature query extent, the behaviour was kind of odd, the non-feature link only pops up once and when I tried going forward and back to the previous queries, it was no longer there. |
|
List of things to do related to multiple extents (feel free to add/suggest, remove items):
|
Each Extent added is presented in the layer control, under the corresponding layer. If the label attribute is missing from the extent, it's inherited from the layer (for now).
Note, there are some issues with opacity and query links when unchecking then checking an extent that I'm currently working on.
Other things I have not yet considered or taken a look at the behaviour of are:
There might be other things or issues, feel free to list them.
Closes #439