search

falling-fruit / falling-fruit-web

Mobile-friendly website for Falling Fruit
https://beta.fallingfruit.org
GNU General Public License v3.0
29 stars 7 forks source link

Allow editing the position of an existing location, two steps on mobile #404

Closed wbazant closed 1 day ago

wbazant commented 2 weeks ago

Closes #306 .

wbazant commented 2 weeks ago

Fixed the branch locally by rebasing the main branch on it.

wbazant commented 2 weeks ago

I'll undraft the PR after fixing #398 on this branch - I just had it opened locally and browsed locations, and it did a weird jump back to the location I tried to edit earlier.

wbazant commented 2 weeks ago

See #398 , I removed the "restore old view" effect for going back from editing location.

ezwelty commented 2 weeks ago

@wbazant Testing this branch, I understand better your comment in #398.

As discussed, on desktop, the most expected behavior would be for the orange marker to become movable (and the map to not be recentered), but I think it's okay to leave it as is for now so that logic can be shared with mobile.

I didn't like that changes to the map made while in "view location" were undone by "back", so I'm glad that is fixed. But if the map is to be zoomed in and centered on "edit location", and the map/position potentially moved accidentally by the user who expected a movable marker (desktop), then I think I actually prefer if "back" (and probably "cancel" too) undid the map changes made while in "edit location". It also feels awkward (especially on mobile) if one pans the map, the orange marker staying in view in the center, and then clicking "back"/"cancel" results in returning to "view location" but no marker in sight.

A small bug is that the orange marker is not precisely overlaid on the shadowed marker: image

On mobile, I also notice a white bar below the map, presumably where the tab icons usually are? image

The first time "edit location" is invoked, and the location is not at the center and the map is auto-zoomed, the center orange marker appears to "fly in" during a gradual map transition. This doesn't seem to happen again a second time: the map transition is immediate and the orange marker appears at the center. Steps to reproduce:

wbazant commented 2 weeks ago

Thanks for spotting the missing tab list, I fixed it!

The imperfect shadow might just be a browser feature (?) so that they're both visible despite being in the same position - but we can just not draw the shadow if the marker is at original position. I noticed the fly-in effect only happens with zoom - looks buggy and not a desirable effect. Not sure what to do about it right now but I'll try to get rid of it.

ezwelty commented 5 days ago

I sorted out the positioning of the pins and did some code and style cleaning in the process. All pins use the same marker and have the same styling except color (drop shadows are gone). I think this looks cleaner; I didn't like how the dropshadow made the marker blend with the background. What do you think? If we feel the need for more separation from the background, we could consider an offset dropshadow, or using a stroke (similar to the location dots):

stroke: #ffedd1;
stroke-width: 1px;

Are we ready to merge?

wbazant commented 5 days ago

From the description, it sounds definitely the right thing do to, I'll check it out!

Can we hold off with merging? I have made a local branch on top of #410 which is actually the continuation of #396, except on desktop I've made the marker draggable and on mobile you can go back and forth between position and details screen. It still misses something ( the state of the form doesn't come back after the round trip) but I'll apply your changes there and push it, at least so we can look at both UX mechanics.

wbazant commented 1 day ago

https://github.com/falling-fruit/falling-fruit-web/pull/419 is better and has all the good changes from here so closing this.