open-tacos icon indicating copy to clipboard operation
open-tacos copied to clipboard

refactor: using alertdialog versus popup

Open clintonlunn opened this issue 1 year ago • 4 comments
trafficstars


name: Pull request about: Create a pull request title: '' labels: '' assignees: ''


What type of PR is this?(check all applicable)

  • [ ] refactor
  • [ ] feature
  • [ ] bug fix
  • [ ] documentation
  • [ ] optimization
  • [ ] other

Description

Related Issues

Issue #1145

What this PR achieves

Screenshots, recordings

image

Notes

clintonlunn avatar Jul 04 '24 21:07 clintonlunn

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
open-tacos ✅ Ready (Inspect) Visit Preview Oct 13, 2024 7:16pm

vercel[bot] avatar Jul 04 '24 21:07 vercel[bot]

@vnugent Looks like I might be getting the same errors as the last pr, preventing the preview from deploying. Not necessarily done with this pr or ready for review, but is there anything I can do to get the preview working?

clintonlunn avatar Jul 05 '24 21:07 clintonlunn

@clintonlunn thanks for the fix. I did a quick test notice the picker doesn't set initial location based on the form state:

  1. From the area edit page, click on the coordinates
  2. Pick a different location. Click Confirm.
  3. Click on the coordinate again. Notice how the drop pin is set at the original location, not at what we set in step 2.

I think the fix may be having initializing the drop pin (in a useEffect() to the value in the form state

vnugent avatar Sep 15 '24 17:09 vnugent

@clintonlunn thanks for the fix. I did a quick test notice the picker doesn't set initial location based on the form state:

  1. From the area edit page, click on the coordinates
  2. Pick a different location. Click Confirm.
  3. Click on the coordinate again. Notice how the drop pin is set at the original location, not at what we set in step 2.

I think the fix may be having initializing the drop pin (in a useEffect() to the value in the form state

@vnugent Thanks, that was it. Instead of passing an initialCenter prop to the coordinate map at all, we can just watch the form context for the latlngStr value and decide where to put the map pin.

clintonlunn avatar Sep 20 '24 23:09 clintonlunn

@vnugent i refactored a bit to simplify this pr a lot. i believe all of the issues should be resolved now. please let me know if there is anything else you would like to see done

clintonlunn avatar Oct 13 '24 19:10 clintonlunn