Skip to content
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

Merged
merged 13 commits into from
Feb 10, 2025
Merged

Conversation

joshlarson
Copy link
Contributor

@joshlarson joshlarson commented Jan 31, 2025

@joshlarson joshlarson added do not merge ⏳ it's waiting on something dev-green Deploy to dev-green labels Jan 31, 2025
<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} />
Copy link
Collaborator

@thecristen thecristen Feb 1, 2025

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"
/>

Copy link
Contributor Author

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.

@github-actions github-actions bot removed the dev-green Deploy to dev-green label Feb 3, 2025
@thecristen thecristen added the dev-blue Deploy to dev-blue label Feb 3, 2025
@joshlarson joshlarson changed the title Hi-Fi Subway System Dev-Green Draft Branch Hi-Fi Subway System Dev-Blue Draft Branch Feb 3, 2025
@joshlarson joshlarson force-pushed the jdl/system-status-lofi-widget branch from 50200a2 to 9f46930 Compare February 5, 2025 14:35
@joshlarson joshlarson force-pushed the jdl/system-status/hi-fi-widget branch from 1dbebf7 to ecd27dd Compare February 5, 2025 14:35
@joshlarson joshlarson removed the dev-blue Deploy to dev-blue label Feb 5, 2025
@joshlarson joshlarson force-pushed the jdl/system-status/hi-fi-widget branch from ecd27dd to 507d7d2 Compare February 5, 2025 23:48
@joshlarson joshlarson added the dev-blue Deploy to dev-blue label Feb 5, 2025
@joshlarson joshlarson changed the base branch from jdl/system-status-lofi-widget to main February 5, 2025 23:51
@joshlarson joshlarson force-pushed the jdl/system-status/hi-fi-widget branch from a722d3f to 466c97e Compare February 7, 2025 14:53
@joshlarson joshlarson changed the title Hi-Fi Subway System Dev-Blue Draft Branch feat: Add Hi-Fi system status widget Feb 7, 2025
@joshlarson joshlarson marked this pull request as ready for review February 7, 2025 14:54
@joshlarson joshlarson requested a review from a team as a code owner February 7, 2025 14:54
@@ -49,4 +97,154 @@ defmodule DotcomWeb.Live.SystemStatus do
</details>
"""
end

defp alerts_examples() do
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@joshlarson joshlarson removed the do not merge ⏳ it's waiting on something label Feb 10, 2025
Copy link
Collaborator

@thecristen thecristen left a 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.

@joshlarson joshlarson merged commit 2da9617 into main Feb 10, 2025
21 checks passed
@joshlarson joshlarson deleted the jdl/system-status/hi-fi-widget branch February 10, 2025 20:16
@github-actions github-actions bot removed the dev-blue Deploy to dev-blue label Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants