The Wayback Machine - https://web.archive.org/web/20201225163953/https://github.com/nextcloud/maps/pull/365
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

added move and remove for photo cluster at max zoom level #365

Conversation

@wronny
Copy link
Member

@wronny wronny commented Apr 20, 2020

as discussed in #346 I added "Move all" and "Remove all geo data" to cluster context menu at max zoom level.
Signed-off-by: wronny forum@wron.de

as discussed in #346 I added "Move all" and "Remove all geo data" to cluster context menu at max zoom level.
Signed-off-by: wronny <forum@wron.de>
@tacruc
Copy link
Contributor

@tacruc tacruc commented Apr 21, 2020

The code looks good to me, but I haven't tested. I'm not sure if I will find time before the weekend.

@wronny
Copy link
Member Author

@wronny wronny commented Apr 21, 2020

I'm not sure if I will find time before the weekend.

No worries, no hurry

@tacruc
Copy link
Contributor

@tacruc tacruc commented Apr 23, 2020

Ok tested it, works nicely. Thanks.
There is a minor thought, I'm not shure how to solve it yet.
The left click button has two behaviors:

  • more than x pictures zoom,
  • less than x pictures slideshow

The reight click botton has two different menus:

  • not max zoom level
  • max zoomlevel

I think we should synchronize them:

  1. make both depending on the number of pictures
  2. make both depinding on the zoom level
  3. make the right click menu allways the same and keep the left click action as it is.

Any thoughts on this @eneiluj @jancborchardt @wronny ?

@wronny
Copy link
Member Author

@wronny wronny commented Apr 23, 2020

thought from my point of view are:

  • currently "move" as well as "remove geo data" was only available for single photos therefore I added "move all" and "remove all geo data" for the minimum photo cluster count as possible which I was only able to identify by zoom level (at max zoom level). However this guarantee that this can only being used for photos with the same geo data.

  • due to the fact that other (outer) zoom level will combine photos from (many) different locations it don't make much sense to allow moving them all to one geo location. And in case of using it by mistake "move all" or "remove all geo data" would be a nightmare if using it form a lower zoom level if hundreds or more photos might be affected

  • I'm not sure if left and right click needs to be synchronized because from my point of view left click is some kind of fast and default behaviour and right click provide some kind of special or detailed options.

@tacruc
Copy link
Contributor

@tacruc tacruc commented Apr 23, 2020

Well, I know the details of how things are programmed and I manged to get confused by the current situation. I don't want to know, if I would be able to explain this behavior to my Grandparents.

And at least a temporally redo action would be nice anyway, or?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.