Skip to content

add line filtering for alert places - #783

Merged
panentheos merged 2 commits into
mainfrom
bhw/alert-stations
Jul 16, 2026
Merged

add line filtering for alert places#783
panentheos merged 2 commits into
mainfrom
bhw/alert-stations

Conversation

@panentheos

Copy link
Copy Markdown
Collaborator

Asana task: Screenplay: Sort/filter stations in Posted Alerts

This adds the line/mode filter to the places list on the alert detail page. The filter defaults to a certain line if all places are on the same line, and uses the corresponding station sort order. Otherwise, it defaults to All, and uses alphabetical sorting. In either case, the filter can be changed to see a subset of the impacted places.

Note that several pages now wait until the places and alerts are loaded before displaying their content, which is necessary to properly initialize the defaults, and has the side effect of removing some flashes of empty content, e.g. the erroneous "not found" message on the alert details page.

@panentheos
panentheos requested a review from a team as a code owner July 14, 2026 13:00

@rwaskiewicz rwaskiewicz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing this out, it looked like "Ferry" was auto selected as filter, which doesn't seem to be happening on main. Could you take a look?

Screen.Recording.2026-07-15.at.10.10.02.mov

Comment on lines 31 to +42
@@ -37,9 +37,17 @@ const AlertDetails: ComponentType = () => {

const validAlertId = id && allAPIAlertIds.includes(id) ? id : undefined;

if (!alerts || !places) {
return null;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to hoist the letter cahnge above the definition of foundAlert? That would eliminate the need for the change to add nullish coalescing to alerts (Note: GitHub is rendering this suggestion in a nonsensical way on my end, hopefully it comes out the other side looking better):

Suggested change
if (!alerts || !places) {
return null;
}
const foundAlert = alerts.find((alert) => alert.id === id);
useEffect(() => {
if (foundAlert) {
setSelectedAlert(foundAlert);
}
}, [foundAlert]);
const validAlertId = id && allAPIAlertIds.includes(id) ? id : undefined;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately we can't do that because that would conditionally short-circuit the useEffect, which isn't allowed.

Comment on lines +182 to +185
const allPlaceRoutes = fp.uniq(places.flatMap(({ routes }) => routes));
const modeOptions = MODES_AND_LINES.filter(({ ids }) => {
return ids[0] === "All" || fp.intersection(ids, allPlaceRoutes).length > 0;
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action required - FWIW, we could accomplish this without lodash (but would require us to update our TS config for 2024's Set methods):

  const allPlaceRoutes = new Set(places.flatMap(({ routes }) => routes));
  const modeOptions = MODES_AND_LINES.filter(({ ids }) => {
    return ids[0] === "All" || allPlaceRoutes.intersection(new Set(ids)).size > 0;
  });

Comment on lines +173 to +181
? fp
.reverse(PLACES_PAGE_MODES_AND_LINES)
.find(
({ ids }) =>
ids[0] === "All" ||
places.every(
(place) => fp.intersection(ids, place.routes).length > 0,
),
)!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action required - Similar comment above RE doing this in vanilla JS:

    ? PLACES_PAGE_MODES_AND_LINES.toReversed().find(
        ({ ids }) =>
          ids[0] === "All" ||
          places.every(
            (place) =>
              new Set(ids).intersection(new Set(place.routes)).size > 0,
          ),
      )!;

@panentheos

Copy link
Copy Markdown
Collaborator Author

Ah, that Ferry behavior is happening because the list of places is empty, so the every check succeeds on the first reversed (e.g. last) filter option. I'll add a conditional to choose "All" in that case, which seems like the sensible choice.

@panentheos
panentheos merged commit 2e0f0b3 into main Jul 16, 2026
2 checks passed
@panentheos
panentheos deleted the bhw/alert-stations branch July 16, 2026 18:37
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