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

Single step view #397

Merged
merged 24 commits into from
Dec 18, 2024
Merged

Single step view #397

merged 24 commits into from
Dec 18, 2024

Conversation

graue
Copy link
Contributor

@graue graue commented Nov 21, 2024

A usable view that allows you to step through each instruction step (for a bike leg) and each stop that a transit leg passes through.

Fixes #396

Screen Shot 2024-12-16 at 19 04 09

Next: skip the "Arrive at destination" instruction

Then: implement for single transit stop view too
Potential improvements:
- Remove the arrive step from the instructions earlier (when first
  getting from server) to avoid all this complicated logic later
  - OR dress up the arrive steps to make it more obvious you're
    transferring to transit
- Add a dressed up arrive at destination step at the very end of the
  last leg
@graue graue marked this pull request as draft November 21, 2024 23:55
@graue
Copy link
Contributor Author

graue commented Nov 23, 2024

Feedback

  • remove vertical line
  • scrolling is bad
  • skip intermediate transit stops
  • indicate where on the map you're turning
  • adjust zoom level to fit the whole step
    maybe only if step is relatively short (100m)
  • "go back" at the bottom is bad, use an X

@abhumbla
Copy link
Contributor

abhumbla commented Nov 27, 2024

request:

  • zoom back out to full route when clicking "Go Back"

@graue
Copy link
Contributor Author

graue commented Dec 1, 2024

This one looks weird without the vertical line. I think I'll keep it for boarding stops.

image

@graue graue force-pushed the graue/single-step branch from 0426944 to 2edd4b4 Compare December 1, 2024 16:12
@graue
Copy link
Contributor Author

graue commented Dec 16, 2024

zoom back out to full route when clicking "Go Back"

I've done this but it's kind of weird because if the instructions are long and you're toward the end, it zooms while the map isn't visible - except around the edges of the instructions. Oh well.

@graue graue marked this pull request as ready for review December 17, 2024 03:06
Copy link
Contributor Author

@graue graue left a comment

Choose a reason for hiding this comment

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

Comments from @abhumbla

src/components/BikeHopperMap.tsx Outdated Show resolved Hide resolved
src/components/ItineraryBikeLeg.tsx Outdated Show resolved Hide resolved
I added position: relative to the main div of ItinerarySingleTransitStop
to position the X in the top right corner. But this made that div the
offsetParent, so useScrollToRef tried to scroll within it, instead of
the actual scroll container (div.MapPlusOverlay_overlay). So I'm
modifying useScrollToRef to iterate till it finds an actually scrollable
container.
also, feedback about just using one viewingStepMarker variable
Copy link
Contributor

@abhumbla abhumbla left a comment

Choose a reason for hiding this comment

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

lgtm, you left some console logs in, not sure if that was intended

src/hooks/useScrollToRef.ts Outdated Show resolved Hide resolved
InstructionSigns.FINISH
) {
if (draft.viewingStep[0] + 1 === viewingRoute.legs.length) {
console.error('next step: would go past end');
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh I left this on purpose so we can see if that's happening and correct it. If I wrote the code correctly, this case will never occur

@graue graue merged commit 3954bc1 into main Dec 18, 2024
3 checks passed
@graue graue deleted the graue/single-step branch December 18, 2024 07:23
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.

improve design of single-step view (and add prev/next arrow buttons)
2 participants