feat(SF2.0/UpcomingDepartures): hide for ferry + show a no service state #2874
Conversation
For ferry routes, hide the Upcoming Departures section
joshlarson
left a comment
joshlarson
left a comment
There was a problem hiding this comment.
I could be persuaded to approve anyway, but the style on the No Service callout doesn't match Figma. I'm open to either trying to convince Chase that what's in this PR is better and/or creating a follow-up ticket, but IMO the easiest path is to just match the Figma right now.
| vehicle_name={@vehicle_name} | ||
| /> | ||
| <% else %> | ||
| <.callout>{~t(No service today)}</.callout> |
There was a problem hiding this comment.
The callout CSS class appears to apply a border and a border-radius, which makes it look a bit different from what's in Figma (Figma has sharp corners and no border).
| stop_id={@stop.id} | ||
| vehicle_name={@vehicle_name} | ||
| /> | ||
| <%= if ServicePatterns.has_service?(route: @route.id) do %> |
There was a problem hiding this comment.
Idea: I wonder if UpcomingDepartures itself should have some kind of short-circuit, to return a type like {:departures, list_of_departures} | :no_service_today
(...eventually to be expanded to something like {:departures, list_of_departures} | :no_service_today | {:future_service, first_scheduled_departure} to account for the case where it should say "Predicted departure times aren’t available yet, but they’ll appear here before the scheduled first trip.")
There was a problem hiding this comment.
Yeah, you can add that when the feature you add warrants it
There was a problem hiding this comment.
I think my main thinking was that we don't have to bother even computing any predicted schedules, if we already know there's no service today (as indicated by the services endpoint). So why conflate that?
On the other hand, once we know there is service and we need to get into the more nuanced logic, then that could benefit from those short-circuited types.
There was a problem hiding this comment.
I think my main thinking was that we don't have to bother even computing any predicted schedules, if we already know there's no service today (as indicated by the services endpoint). So why conflate that?
I agree with that, but is there any reason that UpcomingDepartures.upcoming_departures/1 couldn't be the thing that calls ServicePattenrs and decides not to figure out predicted_schedules?
... in the future, I'm on board with 👇
Yeah, you can add that when the feature you add warrants it
|
@joshlarson There's a thread in Figma where we discussed the discrepancy and agreed to align with the current callout style! |
|
@thecristen Ahaa found it! I'm thinking that we don't want to use the border and rounded corners for callout-style things that are attached to the bottom of the upcoming-departures table - e.g. 👇 looks weird 
compared to 👇 
That means if we go this route, we'll have two different styles of callouts on the same page. (Three, if you count the different color background on the subway daily schedules thing). |
|
@joshlarson I agree with you on the attached callout benefiting from the original designed style. Once everything's on display together eventually our team will have to reckon with the three resulting styles. 🤷🏼♀️ |
joshlarson
left a comment
joshlarson
left a comment
There was a problem hiding this comment.
I've been convinced to approve! But with two notes for things that we may want to follow up on now or soon.:
- The visual style one that I already mentioned. Eventually we'll need to reckon with the fact that there are two different styles for basically the same thing.
- This comment feels like a code-style analog to ☝️, where we have two different code styles to accomplish similar types of things (short-circuiting upcoming-departures so that we don't actually show the UD table). You can see the approach that I followed here. Eventually it would be good (but maybe not necessary?) to reconcile the two approaches.
Summary of changes
Asana Ticket: [SF/UD] Ferry: Suppress Upcoming Departures entirely :-( and [SF/UD] All Modes: "No service today" message on days with no service
edit: I forgot to add screenshots!
(from dev-green, which doesn't yet have the next Pats game)
(from dev, which does have service)
ferry!
