Skip to content
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

Restore original proportions tool and bug fixes #274

Merged
merged 44 commits into from
Jun 27, 2019

Conversation

sashadev-sky
Copy link
Member

@sashadev-sky sashadev-sky commented May 29, 2019

Fixes #272 (<=== Add issue number here)
#250
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updates
  • @mention the original creator of the issue in a comment below for help or for a review

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

=======================================================
Originally opened this to address #272 but branched out into bug fixes and enhancements

  1. functionality to restore to original proportions (via _restore method) has been added and also added to toolbar. Restores the images scale and proportions, but keeps rotation and location intact

  2. Demo coordinates flipped: A while back I brought up that we flip our corner coordinates. (the top right corner is corner 0, and top left is corner 1). I realized this is not a problem with our code, but just that the coordinates we use in our demo are flipped. Changing this should make development a lot easier in the future and also fixes a bug: all the images in our demo appear on the map flipped backwards (like a mirror image), it just happens to not be that noticable with the images we were using. I noticed this when I used a picture with words on it in the demo.

  3. Corresponding README updates and other small fixes there

  • also major readme updates: flreshing out the new Toolbar Actions sections
  1. Small refactoring - added an ImageUtil class

  2. Icons are now optimized as svgs, allowing for 1) performance enhancement, 2) ability to use any icons from any library, 3) browser support

  1. Toolbar has also been updated so that we aren't constantly creating new instances of the toolbar, instead just updating its latlng when we move img corners with new method: _updateToolbarPos
  • Restore icon added
  • Opacity icon is now dynamic

pending

  • fix the entire corners option section of README and explain correct order of corners and results of placing them incorrectly (backwards resulting in mirror image or too close together resulting in squished) and how to get around this (dont put corners at all then let it put image in center of map then it'll give you the exact calculation for the difference in corners to use by checking them on the map)
  • Get rotate angle of image layer #102 have started saving the rotation angle on the overlay as this.rotation. Figured since I went through the functionality of calculating the rotation angle, I can extract it into the methods mentioned by the user here and add it to API: image.getRotate() / image.setRotate(45)
  • add a little blurb about map coordinates to README maybe through discussing the mirror tool
  • expose _updateCorners as a public method and mention in README for easy programmatic image moving
  • find where else we can stop creating a new toolbar and use _updateToolbarPos instead

=======================================================

@sashadev-sky
Copy link
Member Author

@rexagod Please see the PR description and note point 2! Also @jywarren what do you think of a mirror image editing option? I know the toolbar is getting a little packed but we will get that fixed with #225 (dynamic sub-menus) soon

@sashadev-sky
Copy link
Member Author

@jywarren @ebarry so I used a random screenshot I found on my computer because its easier to see whats going on. please reference point 1 in PR description for the functionality description of restore.
restore

The one thing to note here is that it restores it to its original proportions, ones that we initially manipulate when we first put it on the map. So that is why it appears such a different shape. Here is another example using the actual demo image, where its much more subtle (the image is slightly larger after restore):

restore2

@sashadev-sky
Copy link
Member Author

sashadev-sky commented May 30, 2019

@jywarren ok i think this part is ready to go and i'll address the pending points in a follow up! Let me know your thoughts! I left some commented windows behind for the follow up with the rotation stuff

src/edit/RotateHandle.js Show resolved Hide resolved
src/util/ImageUtil.js Show resolved Hide resolved
@jywarren
Copy link
Member

I like this, but I'm a bit cautious about the SVG change. Will it be easy to find new SVGs that match the style, and is it straightforward how to add new SVGs? If this will be addressed in documentation, that's cool! I just want to see what you're thinking on this.

@jywarren
Copy link
Member

Thanks!!!

@sashadev-sky
Copy link
Member Author

@jywarren yes! All the icon libraries provide their fonts in svg format. Working on the docs for how to use it right now! I configured it to work with grunt so it wont be too complicated and im sure people would be interested in learning how svg works!

@sashadev-sky
Copy link
Member Author

sashadev-sky commented Jun 21, 2019

@jywarren ok this is ready! Final final UI:

Screen Shot 2019-06-20 at 9 27 57 PM

these are 20px. Did you want them at 18px? This is 18:
Screen Shot 2019-06-20 at 9 29 59 PM

It seems like the icons on the right look better in 20 and the ones on the left in 18 so not sure which to go with.

Also I bumped to version 0.4.5. Is it okay to just skip releasing 0.4.4 from the version bump in my last PR?

And here is the link to the final wiki doc: https://github.com/publiclab/Leaflet.DistortableImage/wiki/SVG-Icon-System, which I link to in the README under contributing

@sashadev-sky
Copy link
Member Author

@jywarren ok I remembered you liked them smaller and I decided 18px was better so I just went with that.

Now I just need advisal on the version bump and its ready. If we never released 0.4.4 would I still bump this to 0.4.5 if the bump fo 0.4.4 was in a previous PR?

@sashadev-sky sashadev-sky merged commit 83e3371 into publiclab:main Jun 27, 2019
@sashadev-sky sashadev-sky deleted the restore branch August 28, 2019 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

re-add button to "revert to original proportions"
3 participants