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

Fly to nearby MapRoulette challenge #1618

Merged
merged 7 commits into from
Dec 3, 2024
Merged

Fly to nearby MapRoulette challenge #1618

merged 7 commits into from
Dec 3, 2024

Conversation

tannerwuster
Copy link
Contributor

@tannerwuster tannerwuster commented Nov 20, 2024

Navigation to Nearby Tasks:

  • Implemented logic to fly to a nearby task if the feature is enabled.
Screen.Recording.2024-11-20.at.14.56.47.mov

closes #1613

Copy link
Contributor

@bhousel bhousel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some things I would do differently about this:

  1. I am not a fan of using a checkbox in the Map Data pane. This list is mostly for data that the user can toggle on-off, not really for random settings related to that layer. I would expect a setting like this to be a checkbox down near the action buttons (I fixed it, can't complete)
  2. Why does filterNearbyTasks then do a secondary fetch to get the challenge details? I would expect that the user means to work on a nearby task on the same challenge - and that we have all the nearby tasks fetched already - we wouldn't make a new QAItem here?
  3. I'd avoid introducing d3/UI code into the MapRouletteService file. I see stuff to set the header title, and that should all just happen automatically once you do context.enter('select', {selection})

I guess I'll comment some more in the files

@bhousel
Copy link
Contributor

bhousel commented Dec 3, 2024

This seems ok, but I'm having trouble testing it because my MapRoulette token is broken.
Screenshot 2024-12-03 at 11 14 33 AM

Going to merge it as-is and fix the token thing separately.
I thought this was fixed as of #1456 / #1428 but I'll dig in and see.

@bhousel bhousel merged commit ae6bd11 into main Dec 3, 2024
5 checks passed
@bhousel bhousel deleted the fly_to_nearby_challenge branch December 3, 2024 16:19
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.

"Fly to nearby challenge" on completion
3 participants