-
Notifications
You must be signed in to change notification settings - Fork 2
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
Disruption list route filter #1133
Conversation
5974436
to
c1e8d8b
Compare
c1e8d8b
to
ba89215
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.
Looks good! Played around with it locally and all is working. Just have one non-blocking question/suggestion.
@@ -20,7 +20,11 @@ defmodule ArrowWeb.DisruptionV2Controller.Filters do | |||
|
|||
@behaviour Behaviour | |||
|
|||
@type t :: %__MODULE__{view: Calendar.t() | Table.t()} | |||
@type t :: %__MODULE__{ | |||
kinds: MapSet.t(atom()), |
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.
question: I know kinds
is a term from v1, but it feels odd to me. Does it make sense to just go with routes
instead since it will always hold route IDs anyway?
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.
Hmm, that's a good point. The one thing that gives me pause about that though is that it will eventually also include Commuter Rail, which is a mode as opposed to a route, so it's actually kind of a heterogeneous mix of routes and modes, and I'm not really sure of a better term that captures both of those things.
<% show_as_active? = MapSet.size(@filters.kinds) == 0 or kind in @filters.kinds %> | ||
<% active_class = if(show_as_active?, do: "active", else: "") %> | ||
|
||
<.link |
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 wonder if we should look into this page using LiveView in the future? Would be nice if the filters worked without a page load.
Summary of changes
Asana Ticket: 🏹 Implement route selection + button filters
[Please include a brief description of what was changed]
Reviewer Checklist