Toolbar positioning

[sashadev-sky]

Extension of #187

References: #166, #126, publiclab/mapknitter#300, #38, #221, publiclab/mapknitter#542

1. README update for Corners section.

   • replace usage of private _corners with public getCorners()

   • add a Corner section that uses a new public getCorner(idx) method to get the LatLng of a specific corner

2. Added bower_components to .gitignore

3. Toolbar positioning all over the place - update: merged the implementation in An improvement to the placement of the toolbar. #210 and I will rebase on it to incorporate the other points referenced in this PR description

   • In List of small UI bugs to resolve #166 there is a request to make it update at the position of map click. I thought it made more sense to keep it in a stable location in comparison to the image.

4. get Toolbar to continue to appear on Firefox Android and Chrome Android after deselecting and re-selecting an image get Toolbar to continue to appear on Firefox Android and Chrome Android after deselecting and re-selecting an image #38

5. Fixed a bug where _toggleScale hotkey (s) only put you in scale mode and didn't ever toggle you out of it

6. Fixed a hardcoding issue with _showMarkers and _hideMarkers method - didn't account for all handles previously

7. In event of toolbar overlap with a marker can use dblclick on the image to make the z-index of the toolbar lower than the marker. click on the image again to set the z-index back to normal - pending approval - declined

8. new handles _lightBoxHandles / mode light just for moving the image replace distort as default if the user doesn't explicitly pass a mode option. dblclick on the image to rotate through the different handles available. Distort handles no longer has a toolbar when you're using it. When you return back to the light handles the toolbar places itself between the corners and we don't need to worry about overlapping corners because light corners are not meant to be dragged anyway. declined - status: commits for this have been reverted

Pending:

• if switching implementations, delete handleZoom code, and all other obsolete methods. move function to rotate toolbar with icons into a util class possibly for the future
• fix scale handle markers - the show and hide methods I wrote need to accommodate this handle and these methods should in general be refactored to accommodate any # of handles not hardcoded for current handles
• rotating icons
• add conditional check for toolbar tip to prevent error from being thrown if toolbar is changed from popup to control - since merging this first this should be handled from @rexagod's end where necessary
• open PR to fix all hotkey usage for multiple image select
• address edge case of image flipped over itself - couldn't fix one bug, open a PR for hashing out the logic for this.
• clean code
• from Planning issue for next major release of LDI #126 - thorough cross-browser UI testing of toolbar (wait for all toolbar updates by myself and @rexagod first) so hold on this one.
• review rexagods PR

[sashadev-sky]

gifs for Point 3 Toolbar positioning:
In this gif I am just highlighting how even if you click the image, showing the toolbar, at a very zoomed out level, it will still be in the same scaled position when you zoom back in.

[sashadev-sky]

@jywarren I still need to clean up the code should be quick, but there is another issue with it wanted to fix before... I need to play around with it a little more to decide what to do but let me know if you have any ideas!

1. Anchoring the toolbar to the higher handle might not work well if someone pulls one of the handles way above another so need to take into consideration a limit at which the toolbar stops moving up
2. dealing with the toolbar overlapping with another handle - was thinking maybe checking how changing the z-index of both would look, or again creating some kind of limit but this time for how close the toolbar can get to any handle
3. want to see how it would look if the toolbar location changes dynamically when moving handles

[sashadev-sky]

jywarren replied -- Hmm, this is a tough one. What if the menu sort of "runs away" upwards if you're about to hit it? Like, we essentially make a rule that it can't be lower than any handle. Is that what you were suggesting before?

[sashadev-sky]

jywarren - And, this should be good to merge once we address that! Thank you!!!

[sashadev-sky]

jywarren - One last thing - re: toolbar appearing off screen, we could consider either a) panning the map up so you can see it, or b) limiting how high it can go so it cannot be displayed off-screen? Just some ideas.

[sashadev-sky]

Those extra 2 points I will move into the new PR as well - I will make my PR description list as usual and put them there

[sashadev-sky]

There are some bugs to handle like toolbar icons, etc. but I am going to pull @rexagod's PR today and review and see what the updates are. But regardless, that is the gist of it, let me know if you want to move forward with it.

We can also implement a control to let you pick which editing mode you want if you don't want dblclick to be the only way for the user to get out of the distort handles

[sashadev-sky]

@jywarren @rexagod the code itself is a little messy, i will clean up! But here is the rotating toolbar. Works perfectly as seen in 1st gif.

But note there is one issue with it. The gif following the first one shows this issue. It is that if you flip the image corners over its other corners (upside down) then rotate it back right side up, the rotation calculations are off. it seems to maybe think the distort handles are still upside down?

2 paths to take, both a little difficult:

1. Disable the ability to flip the image (corners can't cross each other in distort mode - and we don't have the capacity to distort the image in this sense properly anyway - if the user wants it upside down they should rotate
2. Implement logic if the image is flipped the corners it references to position itself must also flip

Please let me know how you guys would like to proceed! Will put much effort into closing this issue this wknd so i'm not blocking any other PRs

[sashadev-sky]

@rexagod @jywarren ok I merged this it is just now the changes that fix the bugs detailed in the PR I checked over it 1000 times and there are aren't any other problems or things to approve here. I just really wanted to get it merged so that @rexagod can start rebasing his PRs on top of this (or technically, needs to rebase from master to main but rebasing everything to main should now include this)

[jywarren]

Hooray!!! Congrats!

…

On Fri, Apr 19, 2019, 9:58 PM Sasha Boginsky ***@***.***> wrote: @rexagod <https://github.com/rexagod> @jywarren <https://github.com/jywarren> ok I merged this it is just now the changes that fix the bugs detailed in the PR I checked over it 1000 times and there are aren't any other problems or things to approve here. I just really wanted to get it merged so that @rexagod <https://github.com/rexagod> can start rebasing his PRs on top of this (or technically, needs to rebase from master to main but rebasing everything to main should now include this) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#201 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6J5E6B2RZ4OKFDEKDVTPRJ2DDANCNFSM4HEXTFHQ> .