Implementing dynamic sub-menus

[rexagod]

Dynamic sub-menus will allow proper space expansion and allocation, and will greatly aid the toolbar UIs (especially "popup"). I'll try to keep the API as much exposed and semantic as possible.

Based off @jywarren's comment here. Using the Leaflet.toolbar library to incorporate this functionality.

/cc @sashadev-sky

[rexagod]

About time we incorporated @justinmanley's logic. Working on this as well.

---

Rename LeafletToolbar namespace to L.Toolbar2

This matches the recommended API design described in the Leaflet plugin guide. In addition, it eliminates the need for special support in this library for CommonJS and AMD (i.e. "require('leaflet-toolbar')") - that support comes for free with Leaflet.

[rexagod]

Building this with the subToolbar field at the core, more info here.

[sashadev-sky]

That link you shared is the most useful thing 👍This repo is so organized. It looks like he just renamed LeafletToolbar.ToolbarAction to L.Toolbar2.Action just for the sake of making the name shorter right?

The gif looks great so far! @rexagod

[justinmanley]

The purpose of renaming L.Toolbar -> L.Toolbar2 was to make the library compatible with Leaflet.draw ( https://github.com/Leaflet/Leaflet.toolbar/issues/47).

Previously, both Leaflet.draw and Leaflet.toolbar had defined L.Toolbar, which made it impossible to use them together at head (the only way to use them together was to use a special modified version of Leaflet.draw; see https://github.com/Leaflet/Leaflet.toolbar/issues/31).

As part of the renaming, I also refactored the Leaflet.toolbar library and pulled out the Leaflet.draw integration into a separate library: https://github.com/justinmanley/leaflet-draw-toolbar.

[rexagod]

Thanks for the clarification on this, @justinmanley! I do want to mention though that one of the reasons behind considering renaming this was the increasing amount docs that incorporate the L.Toolbar2 examples instead of LeafletToolbar, thus causing some confusion to the newer contributors. But I want to hear out your thoughts on this, should we build on the L.Toolbar2 for faster engagement of the docs with the newer contributors, or just let it be as it is right now (I can't seem to find any docs about LeafletToolbar though)? Also I think it's worth mentioning that many properties of the LeafletToolbar don't seem to work with L.Toolbar2 and vice-versa. Please let me know if I'm wrong here, since I'm somewhat confused regarding this right now, and it goes without saying that I really appreciate you helping us out here, thank you!

[sashadev-sky]

@justinmanley It's so great to hear from you on here! We've been building on your library and I've been really curious about the person who made it! Is it possible I could get in touch with you via email or a phone call to discuss it with you? It's a truly amazing library and I would love the opportunity to talk to the creator :)

[justinmanley]

Pranshu -

Leaflet.Toolbar is the namespace used in the initial version of the Leaflet.toolbar library. That namespace conflicted with Leaflet.draw, so the current version of Leaflet toolbar uses the L.Toolbar2 namespace instead.

The L.Toolbar2 namespace is intended to be easier for clients to use, since it is compatible with the versions of Leaflet.draw published on npm out of the box.

All changes to the Leaflet.toolbar library going forward will be in the L.Toolbar2 namespace.

All functions in the Leaflet.Toolbar namespace should have been ported over to either the new Leaflet.Toolbar2 namespace in the Leaflet.toolbar library, or moved to github.com/justinmanley/leaflet-draw-toolbar. If there are functions missing, that is a bug (either with the library or the docs), so I would be very grateful if you would create issues when you encounter such missing functionality.

Sasha - sure, I'd be happy to talk. I've sent you an email to coordinate a time to meet.

Justin

On Tue, Apr 16, 2019, 9:17 AM Pranshu Srivastava notifications@github.com wrote:

Thanks for the clarification on this, @justinmanley https://github.com/justinmanley! I do want to mention though that one of the reasons behind considering renaming this was the increasing amount docs that incorporate the L.Toolbar2 examples instead of LeafletToolbar, thus causing some confusion to the newer contributors. But I want to hear out your thoughts on this, should we build on the L.Toolbar2 for faster engagement of the docs with the newer contributors, or just let it be as it is right now (I can't seem to find any docs about LeafletToolbar though)? Also I think it's worth mentioning that many properties of the LeafletToolbar don't seem to work with L.Toolbar2 and vice-versa. Please let me know if I'm wrong here, since I'm somewhat confused regarding this right now, and it goes without saying that I really appreciate you helping us out here, thank you!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/Leaflet.DistortableImage/issues/207#issuecomment-483730470, or mute the thread https://github.com/notifications/unsubscribe-auth/ABl1h9lhEHkFjL6bKYRJAy_oIV9ZBDwoks5vhfd9gaJpZM4crAc0 .

[rexagod]

Great input, Justin! This definitely cleared out my doubts on the confusion above, and regarding the missing functionality, I'll surely be creating some issues to bring them to your notice. Many thanks!

[rexagod]

@sashadev-sky @jywarren I've managed to successfully incorporate the subToolbar functionality into the main branch of this repository (refer gif below), so the hard part's done with. What I want your input on is the approach on how we are going to consider "bunching" different functionalities together. Do we start off in a static fashion, grouping together similar actions in the toolbar for now, or do we focus on a more user-preferred (inputted) dynamic approach straightaway? Also, under what factors do two actions qualify to be "grouped" together? I have some ideas in mind but would like to hear on what you have to say about this, just so we're on the same page.

---

[jywarren]

This is great!!!!

Let's try making a nested list just to help us brainstorm. I think we'd probably start with a static implementation, but perhaps later refactor to allow something like addToolGroup() and group.addTool() ?

For the list, what do you think of something like:

[ 'rotate/distort', 'outline', 'export', 'delete', '...': [ 'help', 'geolocate', 'move to top', 'move to bottom' ], ]

On Thu, Apr 18, 2019 at 4:07 PM Pranshu Srivastava notifications@github.com wrote:

@sashadev-sky https://github.com/sashadev-sky @jywarren https://github.com/jywarren I've managed to successfully incorporate the subToolbar functionality into the main branch of this repository (refer gif below), so the hard part's done with. What I want your input on is the approach on how we are going to consider "bunching" different functionalities together. Do we start off in a static fashion, hardcoding and grouping together similar actions in the toolbar for now, or do we focus on a more user-preferred (inputted) dynamic approach straightaway? Also, under what factors do two actions qualify to be "grouped" together? I have some ideas in mind but would like to hear on what you have to say about this, just so we're on the same page.

[image: submenu] https://user-images.githubusercontent.com/33557095/56387097-18347c80-6241-11e9-8660-1ae51d54f34b.gif

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/Leaflet.DistortableImage/issues/207#issuecomment-484670102, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAF6J7RUSGY6JKERTQEBA3PRDIGJANCNFSM4HFMA42A .

[rexagod]

@jywarren I've maintained the order of the first four items of your list below, while grouping all the other ones in the last action, to give you a preview of this implementation. What do you think? Should we proceed with this? Any other modifications?

Also, I do agree on starting this off static, and making it dynamic along the way, employing methods such as addToolGroup() and group.addTool() as you suggested.

---

[jywarren]

This looks great. Does it work in popup-style too?

I think we should be able to find a ... style menu for the "more tools" group, that might be a little more recognizable as an openable menu, what do you think?

On Thu, Apr 18, 2019 at 4:59 PM Pranshu Srivastava notifications@github.com wrote:

@jywarren https://github.com/jywarren I've maintained the order of the first four items of your list below, while grouping all the other ones in the last action, to give you a preview of this implementation. What do you think? Should we proceed with this? Any other modifications?

Also, I do agree on starting this off static, and making it dynamic along the way, employing methods such as addToolGroup() and group.addTool() as you suggested.

[image: submenu] https://user-images.githubusercontent.com/33557095/56390622-1622eb80-624a-11e9-8a6b-d193cfb50f8e.gif

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/Leaflet.DistortableImage/issues/207#issuecomment-484687604, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAF6J3LK6ZCGDYFWQVCRNDPRDOHVANCNFSM4HFMA42A .

[rexagod]

@jywarren Initially, this worked well in the control mode, but I experienced some CSS bugs in the subtoolbar on switching to popup. But I've fixed that, and this implementation works well for both of these modes now. Also, I've replaced the "more tools" icon with the one you suggested. Please have a look!

---

popup UI before modifications (image distorted for better view of the subtoolbar)

popup UI after modifications