-
Notifications
You must be signed in to change notification settings - Fork 13
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(Live.TripPlanner): Add switch origin/destination button #2317
Conversation
current_params | ||
|> Map.merge(params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I'm sending events for a single input at a time (example: %{"from" => %{}}
with no "to" specified), that causes %{"input_form" => params}
to have params which don't always contain all the form's needed params. Processing them without incorporating the existing rest of the form (in socket.assigns.input_form.changeset.params
) produced weird results sometimes. So I added this merge step.
name={location_f[subfield].name} | ||
/> | ||
</.inputs_for> | ||
<.feedback :for={{msg, _} <- @field.errors} :if={used_input?(@field)} kind={:error}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is creating some spurious errors (as soon as you enter an origin, it complains that there's no destination, and vice versa), but it's also conflicting with main
. I wonder if resolving the conflicts with main
would fix this.
(I think resolving the conflicts with main
would actually just main using exactly this version of this file, but with the new "Enter [an origin|a destination] location" placeholders, since the big change to main
was re-adding the error feedback component, which you also added here.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think resolving the conflict fixed it?!? Feel free to try again :)
also refactor AlgoliaAutocomplete hook
529deea
to
c9b70a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this!
I'm ready to approve once the placeholder text is back.
Co-authored-by: Josh Larson <[email protected]>
Co-authored-by: Josh Larson <[email protected]>
Co-authored-by: Josh Larson <[email protected]>
Summary of changes
Asana Ticket: Add switch origin/destination button
Building off of Josh's earlier work in #2231 and elsewhere, implements switching the locations on the form, thus switching the displayed markers on the map, and rerunning the trip planner with new results.
This also includes some cleanup on the
AlgoliaAutocomplete
Phoenix JS hook. Now instead of triggering 4 "change" events for each location input change, we more elegantly fire off one (1) event for the LiveView to handle (just another"input_form_change"
event, transmitting data selected from the location dropdown).