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

refactor: Inline <.itinerary_group /> component into TripPlannerResultsSection #2241

Merged
merged 6 commits into from
Dec 2, 2024

Conversation

joshlarson
Copy link
Contributor

No Asana ticket.

I think this addresses this concern raised in #2232, by bringing both of the phx-click="set_expanded_itinerary_index"'es into the same file where the resulting expanded_itinerary_index is actually used.

@joshlarson joshlarson requested a review from a team as a code owner November 29, 2024 20:02
Copy link
Contributor

@anthonyshull anthonyshull left a comment

Choose a reason for hiding this comment

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

Only thing is we should have a fallback for itinerary_panel that logs if it doesn't get pattern matched and returns some default.

@joshlarson
Copy link
Contributor Author

@anthonyshull how does bb42934 look? IMO it doesn't look great, but also that scenario should never happen, so I don't think we need to make it look too pretty O_o

(Also, Sentry.capture_message for logging, or do we have a better way?)

Screenshot 2024-12-02 at 11 41 53 AM Screenshot 2024-12-02 at 11 41 58 AM

@anthonyshull
Copy link
Contributor

@anthonyshull how does bb42934 look? IMO it doesn't look great, but also that scenario should never happen, so I don't think we need to make it look too pretty O_o

Yeah, that looks good.

(Also, Sentry.capture_message for logging, or do we have a better way?)

Yeah, that's what we've been using. I think you can use the extra option instead of converting the assigns to a string:
https://hexdocs.pm/sentry/Sentry.html#capture_message/2-options

@joshlarson joshlarson enabled auto-merge (squash) December 2, 2024 18:53
@joshlarson joshlarson merged commit 6d85121 into main Dec 2, 2024
17 checks passed
@joshlarson joshlarson deleted the jdl/remove-itinerary-group-component branch December 2, 2024 18:58
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.

2 participants