Re-order collections array to match when using StackAction to re-order images

[jywarren]

https://github.com/publiclab/Leaflet.DistortableImage#single-image-interface-1 describes how to use the StackAction, and we implemented that here in MapKnitter:

https://github.com/publiclab/mapknitter/blob/6731eecdf187fc7e691ca9616eda7c0489e1712f/app/assets/javascripts/mapknitter/Map.js#L108

However, using this does not affect the order of opts.collection, and the export we queue up does not see that the images have been re-ordered.

Currently StackAction is here: https://github.com/publiclab/Leaflet.DistortableImage/blob/fe5f6ae2b136be6cb2a437cc9f08f2ae553ee316/src/edit/actions/StackAction.js#L34

and as highlighted there, we use _toggleOrder to then trigger Leaflet's internal _overlay.bringToBack() function which relies on CSS z-index: https://leafletjs.com/reference.html#divoverlay-bringtoback

https://github.com/publiclab/Leaflet.DistortableImage/blob/62da2c0488b7382ea88691f10b47ed1f499375b0/src/edit/DistortableImage.Edit.js#L457-L474

What we should do is sort the image to the top or bottom of the array as well, which should be a simple array.splice() (syntax let arrDeletedItems = array.splice(start, length)) followed by array.push() (to back) or array.unshift() (to front).

This would be great to write a test for as well!

[jywarren]

Noting that we don't need to know for this fix, but for background info, Leaflet uses the order in the HTML DOM to set display order, and the bringToBack() methods in Leaflet reorder the DOM elements: https://github.com/Leaflet/Leaflet/blob/37d2fd15ad6518c254fae3e033177e96c48b5012/src/dom/DomUtil.js#L95-L102

[jywarren]

We've completed a fix in https://github.com/publiclab/mapknitter/commit/e5054f0c33f2a8f3807338dd322fd499aade0360 which covers our production site, so less urgent now!

But this is still an important part of LDI functioning correctly locally. Help certainly wanted, thanks!!! 🎉

[abd123codes]

@jywarren is this issue still open

[jywarren]

Yes it is. Do you feel like you have enough information to know where to begin? Thank you!

On Fri, Oct 14, 2022, 9:12 AM Abdulsamad Adams @.***> wrote:

@jywarren https://github.com/jywarren is this issue still open

— Reply to this email directly, view it on GitHub https://github.com/publiclab/Leaflet.DistortableImage/issues/665#issuecomment-1278306342, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J56NIQSGD72ZPKE4DDWDCQNTANCNFSM4NQ3XTLQ . You are receiving this because you were mentioned.Message ID: @.***>

[7malikk]

@THEDARKMAN007 Would you like to collaborate on this? as @jywarren provides us with more information

[abd123codes]

@jywarren actually no I do not, but I wud like to collaborate with @7malikk, perhaps id learn something. Thank you. And yes @7malikk I'd luv to collaborate on it with you.

[abd123codes]

@7malikk I think we shud share our WhatsApp contact so we could communicate more effectively on this issue. this is my contact +2347018100267

[abd123codes]

@jywarren @TildaDares @cesswairimu. I and @7malikk have started working on understanding the source of this issue. we are looking to test this issue live by running the project and seeing stackAction in use. but after going through the installation guide, we had difficulties setting up the project.

[jywarren]

One thing to note before we begin is that the StackAction is not enabled by default in our MapKnitter Lite "archive.html" demo. To enable it, we have to add it when we initialize the DistortableImage, as we do in this example:

https://github.com/publiclab/Leaflet.DistortableImage/blob/83001cc17ba8e387d658b98fd08d7837a490e2ac/examples/select.html#L66

Then you'll be able to change order of display.

(NOTE: skip ahead to SOLUTION below... this was just my notes as i dug through the issue)

So my next unknown is: I assume that changing order of layers is independent of selecting a collection of images to export; that is, you really have to change the ordering /before/ you select a bunch of images into a collection, because once you select the collection, you can no longer get to the individual menus (try it out here: https://publiclab.github.io/Leaflet.DistortableImage/examples/select).

So, my unknown is: how do we add items to the collection? Do we get them in the order they're listed in the leaflet framework, somehow? Let's look carefully at the code:

This line is where we generate the collection JSON output, and it uses this eachLayer() function:

https://github.com/publiclab/Leaflet.DistortableImage/blob/83001cc17ba8e387d658b98fd08d7837a490e2ac/src/DistortableCollection.js#L196

So, how does eachLayer() do ordering? Since DistortableCollection is an extension of FeatureGroup, let's look there:

https://leafletjs.com/reference.html#layergroup-eachlayer doesn't give us much info about layer ordering... so I did a general search for 'leaflet eachlayer order':

https://stackoverflow.com/a/30814653 says:

Good question, it seems the LeafletJS docs don't say anything about the ordering of the layers via eachLayer, so...

Looking at the code on github, L.Map.eachLayer loops through L.Map._layers which is an object (not an array). So technically the layers could come back in any order...

That said, the individual layers are added to the _layers object in the order they are added to the map. So in all likelihood, eachLayer will be iterating over the layers in that same order (whatever order they were added to the map).

This other result (https://gis.stackexchange.com/a/162656) says you need to sort everything by CSS z-order, then use layer.bringToFront() on each one in reverse order to get the order to match.

But don't we already do that here? https://github.com/publiclab/Leaflet.DistortableImage/blob/62da2c0488b7382ea88691f10b47ed1f499375b0/src/edit/DistortableImage.Edit.js#L472

Hmm. Maybe we should double check that the ordering doesn't get changed when we use StackAction. Maybe Leaflet has changed and fixed this?

...

Solution

Gah! I should have read my own comments from way back -- it seems we solved this in MapKnitter, using this code:

https://github.com/publiclab/mapknitter/commit/e5054f0c33f2a8f3807338dd322fd499aade0360

So, shall we run this ordering just before we generate the export JSON? At the top of this function?

https://github.com/publiclab/Leaflet.DistortableImage/blob/83001cc17ba8e387d658b98fd08d7837a490e2ac/src/DistortableCollection.js#L192

[7malikk]

Oh wow, this is great @jywarren glad you left those notes, very informative @THEDARKMAN007 and I will hop on this and revert back Thanks a bunch 👍🏾

PS. Don't get tired of my pings 😁

[abd123codes]

thanks making this clear @jywarren