mapknitter icon indicating copy to clipboard operation
mapknitter copied to clipboard

Errors in latest code to resolve before publishing to live site

Open jywarren opened this issue 5 years ago • 4 comments

Hi! I'm just getting things closer to a new release we can publish to the live site, and writing down a variety of small bugs I found that I think need to be addressed before this is ready to publish. I'm sure we can do this!

  • [ ] After uploading 3 images, i found the Images tab had 3 copies of the same image instead of the 3 different images. This resolved on page reload (maybe @divyabaid16 or @cesswairimu ?)
  • [ ] Newly placed images could sometimes not be selected by clicking on them. i clicked on other things for a while before I was able to click to select them.
  • [ ] I can't seem to set the multi export scale; this used to create a prompt! Now it's set to 100 no matter what. @sashadev-sky was this taken out of the default multi-exporter? (moved to publiclab/Leaflet.DistortableImage#471)
  • [x] we can no longer zoom in very close; it used to allow up to something like zoom 18 or 20, but we're now limited to much lower; #156 perhaps related? See below. (moved to https://github.com/publiclab/mapknitter/issues/1138)

image

Finally, I think some tools (scale, simple rotate (not free rotate), and drag) should be hidden or tucked into a submenu, as they're not critical and make the menu a little overwhelming for newcomers.

I'd love help figuring these out, and then we can get closer to publishing this to the live site!

You can try this all out here:

http://mapknitter-stable.laboratoriopublico.org/maps/lee-nh/edit

jywarren avatar Dec 05 '19 04:12 jywarren

Linking to https://github.com/publiclab/Leaflet.DistortableImage/issues/471 for Add prompt for multi-export scale AND/OR allow setting this in DistortableCollection constructor

jywarren avatar Dec 05 '19 04:12 jywarren

@jywarren

  1. prompt: we never set a prompt to my knowledge
  2. zoom : on the link you provided the map goes up to 18 zoom for me (map.getZoom() in console). also looks about the same zoom in the screenshot you attached? This is set from this file: https://github.com/publiclab/Leaflet.DistortableImage/blob/main/src/mapmixins/MapMixins.js we let it go up to 21 if you set that but the default is 18.
  3. anything else in particular you noticed about the newly placed images when bug did happen vs didnt?

sashadev-sky avatar Dec 05 '19 04:12 sashadev-sky

Thanks @sashadev-sky!

re Prompt I posted at publiclab/Leaflet.DistortableImage#471 -- thanks!

Re zoom - here is a post about it - i was underestimating, it's actually max zoom 24!

I'm going to make a new issue about it and I think it can also be a great GCI task - pretty simple!

jywarren avatar Dec 05 '19 21:12 jywarren

Moved the zoom one to https://github.com/publiclab/Leaflet.DistortableImage/issues/473 - thanks!

jywarren avatar Dec 05 '19 21:12 jywarren