-
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: Add Hi-Fi system status widget #2353
Conversation
<span class="flex"> | ||
<.route_pill route_id={@route_id} /> | ||
<span class="h-6 -ml-1 flex gap-0.5"> | ||
<.route_modifier :for={modifier_id <- @modifier_ids} modifier_id={modifier_id} /> |
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.
Was playing around with this and found that using the existing DotcomWeb.Components.RouteSymbols.route_icon
component here looks great too:
<.route_icon
:for={modifier_id <- @modifier_ids}
route={%Routes.Route{id: modifier_id}}
class="ring-2 ring-white rounded-full"
/>
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.
It looked funny to me in Firefox, because Firefox aligns the text in the route pills slightly differently from how it's aligned in the SVG's.
If we get SVG's for the route pills also, then I'm all in favor of using the existing <.route_icon />
components, but I'm a bit leery of having the component be partially SVG and partially text-in-a-box.
50200a2
to
9f46930
Compare
1dbebf7
to
ecd27dd
Compare
ecd27dd
to
507d7d2
Compare
a722d3f
to
466c97e
Compare
@@ -49,4 +97,154 @@ defmodule DotcomWeb.Live.SystemStatus do | |||
</details> | |||
""" | |||
end | |||
|
|||
defp alerts_examples() do |
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 comment applies to this as well.
""" | ||
end | ||
|
||
defp status_to_rows(data) do |
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.
Happy to add documentation-y comments for these private functions, but I wanted to punt that until just before merging.
c767a20
to
b13ea8b
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.
This is really effective and thanks for all the examples! My comments are mostly minor, and you'll have to fix a linting error anyway. I do hate the name "widget" for anything, IMO system_status
or maybe subway_status
would work fine.
Examples
Asana Ticket: System Status | [Hi-Fi] Component that renders grouped statuses/alerts