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

draws grey remaining-in-route line for RouteBuilder routes #3307

Merged
merged 11 commits into from
Aug 10, 2023

Conversation

lindseyehrlich
Copy link
Collaborator

Resolves #3204

Draws grey "remaining in your route" line in the mission complete maps for routes created with RouteBuilder.

Before/After screenshots (if applicable)
before screenshot after screenshot
Testing instructions
Things to check before submitting the PR
  • I've written a descriptive PR title.
  • I've added/updated comments for large or confusing blocks of code.
  • I've included before/after screenshots above.

@misaugstad
Copy link
Member

@lindseyehrlich so it looks like the bug you fixed may have been introduced since the time that I filed #3204! In the screenshot I shared there, the remainder of the route is being drawn correctly. Thank you for fixing that so that it's working again 😅

But the issue that was brought up was that the remainder of the task that the user is in the middle of working on isn't getting drawn in gray. It isn't entirely clear from your screenshot if this is working correctly or not, but I think it's not.

Pasting my original screenshot below for reference. On the right side, you can see that I was part of the way through that street, and the part I've finished is shown in green. But the remainder of the street isn't shown at all, and should be shown in gray. If you make your routes into loops then it's really easy to see if it's working :)
Screenshot from 2023-04-13 09-59-38

@lindseyehrlich
Copy link
Collaborator Author

Ah, I see! I think the code I wrote fixes both issues. I tested it a few times with different routes and I haven't come across any gaps between green and grey lines. The for loop I wrote draws a grey line for any street that's incomplete and the green line is drawn on top depending on how far the user got into the street. Here's a screenshot from one of my tests in which I made the route into a loop:
route loop

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Looks like it's working for "Routes" but it isn't working in the general case! If you're not on a route and someone has already audited a street that you've partially finished, it isn't showing the rest of the line as having been audited by someone else.
Screenshot from 2023-07-27 12-40-34

That's because of this if statement:

if (userOldStreets.indexOf(otherUserStreet) === -1 && newStreets.indexOf(otherUserStreet) === -1)

It's basically saying that any street that's part of the current mission shouldn't be drawn. But we should at least draw the portion that that you haven't finished auditing yet on your current street!

@jonfroehlich
Copy link
Member

@crescendochu and I are reviewing her study routes and we also came across a related issue. There should be no disconnect here:
image

It should look like this:
image

Should we file a new issue? Also, this ideally would be fixed before the study occurs...

@misaugstad
Copy link
Member

we also came across a related issue. There should be no disconnect here ... Should we file a new issue?

This is the exact issue that's being fixed here!

this ideally would be fixed before the study occurs...

Given that @lindseyehrlich fixed this for the case or user-defined routes in this PR already, we should be able to just merge this one before the study and then she can work on the other case for another PR!

@lindseyehrlich
Copy link
Collaborator Author

Ready for another PR review

Copy link
Member

@misaugstad misaugstad left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @lindseyehrlich !!

@misaugstad misaugstad merged commit 75e3581 into develop Aug 10, 2023
@misaugstad misaugstad deleted the 3204-draw-remainder-of-explore-mission-route branch August 10, 2023 23:07
@misaugstad misaugstad mentioned this pull request Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Explore mission complete map not showing remainder of current street
3 participants