Add walking legs to the trip plan details #2246
Conversation
anthonyshull
commented
Dec 5, 2024
•
anthonyshull
commented
| <.leg | ||
| start_time={leg.start} | ||
| end_time={leg.stop} | ||
| from={leg.from} | ||
| to={leg.to} | ||
| mode={leg.mode} | ||
| realtime={leg.realtime} | ||
| realtime_state={leg.realtime_state} | ||
| /> |
There was a problem hiding this comment.
📓 We should probably rename this to TransitLeg and not destructure the leg.
There was a problem hiding this comment.
did you look inside DotcomWeb.Components.TripPlanner.Leg.leg/1? it's basically a wrapper that shows a transit leg or walking leg. Might want to either (a) move this to there or (b) simplify leg/1 internal logic to remove handling of the walking directions, then rename that to transit_leg.
There was a problem hiding this comment.
That's what I meant by this comment. I don't think it is in the scope of this ticket to refactor the previous work. But, we should do it because the two different legs have completely different rendering methods. And, they should both take a leg instead of that destructuring.
There was a problem hiding this comment.
Yeah, I agree. I just think since this change makes part of the existing leg/1 logic unreachable already, it feels a little odd not to make that refactor here. Just my 2c.
| @@ -129,11 +129,6 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerForm do | |||
| </.inputs_for> | |||
| </div> | |||
| </:content> | |||
| <:extra :if={used_input?(f[:modes])}> | |||
There was a problem hiding this comment.
Can we undo this removal? This removes the "please select a mode" message if all modes are left unchecked, and this change doesn't seem to have anything to do with the rest of this PR anyway.
There was a problem hiding this comment.
I removed that attribute from the accordion because you said we didn't need it anymore. It was giving a warning once I removed it. So, I removed it here. Are you using it for the errors? I assumed not so removed it here.
There was a problem hiding this comment.
Ah, gotcha.
As I just described: "Please select at least one mode of transit". But this could just as easily be included as a <div> after the accordion instead of using the prior extra attribute.
There was a problem hiding this comment.
Yep, that way they all work the same.
| <.leg | ||
| start_time={leg.start} | ||
| end_time={leg.stop} | ||
| from={leg.from} | ||
| to={leg.to} | ||
| mode={leg.mode} | ||
| realtime={leg.realtime} | ||
| realtime_state={leg.realtime_state} | ||
| /> |
There was a problem hiding this comment.
did you look inside DotcomWeb.Components.TripPlanner.Leg.leg/1? it's basically a wrapper that shows a transit leg or walking leg. Might want to either (a) move this to there or (b) simplify leg/1 internal logic to remove handling of the walking directions, then rename that to transit_leg.
anthonyshull
force-pushed
the
ags/walking-leg-component
branch
from
3ad3199 to
4b11b9b
Compare
anthonyshull
force-pushed
the
ags/walking-leg-component
branch
from
1abadff to
130846c
Compare
