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

feat: Populate detours list with channels #2962

Merged
merged 6 commits into from
Feb 28, 2025

Conversation

hannahpurcell
Copy link
Collaborator

@hannahpurcell hannahpurcell commented Feb 12, 2025

Asana ticket: https://app.asana.com/0/1205385723132845/1208390246838825

In terms of tests, I'm not sure what else to cover? Ideally, I'd like to verify some integration flow -- like activating a detour triggers the channel to be affected -- but I struggled with that.

Known issue: preventing a drafted detour from appearing in the channel and passing through the handleDrafted function before it has all it's returned values (like nearestIntersection). To be discussed in subsequent PR.

Also: have confirmed with Brian that the spinner is great!

@hannahpurcell hannahpurcell requested a review from a team as a code owner February 12, 2025 22:50
@hannahpurcell hannahpurcell requested review from firestack and removed request for a team February 12, 2025 22:50
@hannahpurcell hannahpurcell marked this pull request as draft February 14, 2025 15:21
@hannahpurcell hannahpurcell marked this pull request as ready for review February 20, 2025 19:30
@hannahpurcell hannahpurcell added the do-not-merge Do not merge PR's with this label. label Feb 21, 2025
Copy link
Member

@firestack firestack left a comment

Choose a reason for hiding this comment

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

Looks good! A few comments but nothing that big, ready to approve after dialoging on this.

@hannahpurcell hannahpurcell force-pushed the hp/populate-detours-list-with-channels branch from ce7ece5 to 9770dea Compare February 26, 2025 22:14
Copy link
Member

@firestack firestack left a comment

Choose a reason for hiding this comment

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

waiting to resolve the issue with nearestIntersection but otherwise lgtm 👍🏻

* don't try to save snapshot without nearestIntersection present

* Update assets/tests/components/detours/diversionPage.saveDetour.test.tsx

Co-authored-by: Kayla Firestack <[email protected]>

---------

Co-authored-by: Kayla Firestack <[email protected]>
@hannahpurcell hannahpurcell merged commit b175ae8 into main Feb 28, 2025
10 checks passed
@hannahpurcell hannahpurcell deleted the hp/populate-detours-list-with-channels branch February 28, 2025 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-to-dev-green do-not-merge Do not merge PR's with this label.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants