feat: Open the detour modal from a notification #2825
Conversation
firestack
force-pushed
the
kf/asn/detour-notifications
branch
4 times, most recently
from
a0d12e6
to
20eaec5
Compare
| detourId: 79, | ||
| headsign: "79 - Wooded Mountains", | ||
| route: "79", | ||
| direction: "Inbound", | ||
| origin: "Maple Knoll", |
There was a problem hiding this comment.
question: is it possible to not hardcode these values?
There was a problem hiding this comment.
It's a bit of a struggle when tests assume hardcoded values from factories and then in the future we decide to make them generated, so I'd prefer to start with generated values.
I'd be open to pulling in faker @mbta/skate-developers
There was a problem hiding this comment.
I agree - I did this just to get something working, but I'd rather get non-hardcoded values in before merging.
There was a problem hiding this comment.
This was done in #2819
There was a problem hiding this comment.
note(non-blocking): I would like to change the <NotificationCard/> component into a switch which then renders some other component, like, <DetourNotificationCard/> to make it a bit simpler to understand what's going on (like the activate detour modal!) I'd be open to that happening in this PR, or a followup, or just noting it as a future tech debt item.
Based on the work you did for this @joshlarson does that seem worth it or do you think that it's beneficial to keep it this way for now? I'm worried this would be too early of a "component-ization" but there's also a bunch of different logic
There was a problem hiding this comment.
I think this is a good time to component-ize, but I'm on the fence about whether I want to do it in this PR versus teasing the different notification cards apart as a "fast"(indeterminate-speed)-follow.
There was a problem hiding this comment.
Given this PR has already changed hands, I think I'll consider this a "future do"
joshlarson
force-pushed
the
jdl/feat/open-detour-modal-from-notification-card
branch
from
e698891
to
394e0a0
Compare
Asana Ticket: https://app.asana.com/0/1148853526253420/1207752079129768/f