Skip to content

add line filtering for alert places #783

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.

Sign up for GitHub

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

Merged
panentheos merged 2 commits into from
Jul 16, 2026
Merged

add line filtering for alert places #783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca8322c
add line filtering for alert places
panentheos Jul 14, 2026
7d1407b
default to All for empty places list
panentheos Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions assets/js/components/Dashboard/AlertDetails.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const AlertDetails: ComponentType = () => {

const navigate = useNavigate();

const foundAlert = alerts.find((alert) => alert.id === id);
const foundAlert = (alerts ?? []).find((alert) => alert.id === id);
useEffect(() => {
if (foundAlert) {
setSelectedAlert(foundAlert);
Expand All @@ -37,9 +37,17 @@ const AlertDetails: ComponentType = () => {

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

if (!alerts || !places) {
return null;
}
Comment on lines 31 to +42

@rwaskiewicz rwaskiewicz Jul 15, 2026

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;

@panentheos panentheos Jul 16, 2026

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.

const alertPlaces = placesWithSelectedAlert(
selectedAlert,
places,
screensByAlertMap,
);
return selectedAlert ? (
// Define a new ContextProvider so state is not saved to Context used on the PlacesPage.
<PlacesListStateContainer>
<PlacesListStateContainer places={alertPlaces}>
<div className="alert-details">
<div className="page-content__header">
<div>
Expand Down Expand Up @@ -69,14 +77,10 @@ const AlertDetails: ComponentType = () => {
<div className="page-content__body">
<AlertCard alert={selectedAlert} classNames="selected-alert" />
<PlacesList
places={placesWithSelectedAlert(
selectedAlert,
places,
screensByAlertMap,
)}
noModeFilter
places={alertPlaces}
isAlertPlacesList
showAnimationForNewPlaces
showLineMap={false}
/>
</div>
)}
Expand Down
4 changes: 4 additions & 0 deletions assets/js/components/Dashboard/AlertsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ import moment from "moment-timezone";
const AlertsPage: ComponentType = () => {
const { places, alerts, screensByAlertMap } = useScreenplayState();

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

const alertsWithPlaces = alerts.filter(
(alert) => screensByAlertMap[alert.id],
);
Expand Down
2 changes: 1 addition & 1 deletion assets/js/components/Dashboard/Dashboard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ const Dashboard: ComponentType = () => {
alerts: newAlerts,
screens_by_alert: screensByAlertMap,
} = alertsData;
findAndSetBannerAlert(alerts, newAlerts);
findAndSetBannerAlert(alerts ?? [], newAlerts);
setAlerts(newAlerts, allAPIalertIds, screensByAlertMap);
}
};
Expand Down
31 changes: 20 additions & 11 deletions assets/js/components/Dashboard/PlacesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import { DirectionID } from "Models/direction_id";
import { usePrevious } from "Hooks/usePrevious";
import { sortByStationOrder } from "../../util";
import fp from "lodash/fp";

const getSortLabel = (
modeLineFilterValue: { label: string },
Expand All @@ -36,6 +37,9 @@ const getSortLabel = (
const PlacesPage: ComponentType = () => {
const { places } = useScreenplayState();

if (!places) {
return null;
}
return (
<div className="places-page">
<div className="page-content__header">Places</div>
Expand All @@ -48,16 +52,16 @@ const PlacesPage: ComponentType = () => {

interface PlacesListProps {
places: Place[];
noModeFilter?: boolean;
isAlertPlacesList?: boolean;
showAnimationForNewPlaces?: boolean;
showLineMap?: boolean;
}

const PlacesList: ComponentType<PlacesListProps> = ({
places,
noModeFilter,
isAlertPlacesList,
showAnimationForNewPlaces,
showLineMap = true,
}: PlacesListProps) => {
// ascending/southbound/westbound = 0, descending/northbound/eastbound = 1
const {
Expand Down Expand Up @@ -175,6 +179,11 @@ const PlacesList: ComponentType<PlacesListProps> = ({
statusFilterValue !== STATUSES[0] ||
screenTypeFilterValue !== SCREEN_TYPES[0];

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;
});
Comment on lines +182 to +185

@rwaskiewicz rwaskiewicz Jul 15, 2026

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;
  });


return (
<>
<Container fluid>
Expand All @@ -187,14 +196,12 @@ const PlacesList: ComponentType<PlacesListProps> = ({
/>
</Col>
<Col lg={3} className="d-flex justify-content-end pe-3">
{!noModeFilter && (
<FilterDropdown
list={MODES_AND_LINES}
onSelect={(value: any) => handleSelectModeOrLine(value)}
selectedValue={modeLineFilterValue}
className="modes-and-lines"
/>
)}
<FilterDropdown
list={modeOptions}
onSelect={(value: any) => handleSelectModeOrLine(value)}
selectedValue={modeLineFilterValue}
className="modes-and-lines"
/>
</Col>
<Col lg={3} className="place-screen-types pe-3">
<FilterDropdown
Expand Down Expand Up @@ -245,7 +252,9 @@ const PlacesList: ComponentType<PlacesListProps> = ({
}
activeEventKeys={activeEventKeys}
sortDirection={sortDirection}
filteredLine={isOnlyFilteredByRoute ? getFilteredLine() : null}
filteredLine={
showLineMap && isOnlyFilteredByRoute ? getFilteredLine() : null
}
className={isFiltered || isAlertPlacesList ? "filtered" : ""}
/>
);
Expand Down
2 changes: 1 addition & 1 deletion assets/js/hooks/usePlacesWithPaEss.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export const usePlacesWithPaEss = () => {
const { places } = useScreenplayState();
return useMemo(
() =>
places
(places ?? [])
.map((place) => ({
...place,
screens: place.screens.filter((screen) => screen.type === "pa_ess"),
Expand Down
33 changes: 27 additions & 6 deletions assets/js/hooks/useScreenplayContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { BannerAlert } from "../components/Dashboard/AlertBanner";
import { ActionOutcomeToastProps } from "../components/Dashboard/ActionOutcomeToast";
import useSWR, { KeyedMutator } from "swr";
import { getSuppressedPredictions } from "Utils/api";
import fp from "lodash/fp";

interface Props {
children: React.ReactNode;
Expand All @@ -28,9 +29,9 @@ interface FilterValue {
}

interface ScreenplayState {
places: Place[];
places?: Place[];
lineStops: LineStop[];
alerts: Alert[];
alerts?: Alert[];
allAPIAlertIds: string[];
screensByAlertMap: ScreensByAlert;
bannerAlert?: BannerAlert;
Expand All @@ -55,9 +56,9 @@ const [useScreenplayState, ScreenplayStateProvider] =
createGenericContext<ScreenplayState>();

const ScreenplayStateContainer = ({ children }: Props) => {
const [places, setPlaces] = useState<Place[]>([]);
const [places, setPlaces] = useState<Place[]>();
const [lineStops, setLineStops] = useState<LineStop[]>([]);
const [alerts, setAlerts] = useState<Alert[]>([]);
const [alerts, setAlerts] = useState<Alert[]>();
const [allAPIAlertIds, setAllAPIAlertIds] = useState<string[]>([]);
const [screensByAlertMap, setScreensByAlertMap] = useState<ScreensByAlert>(
{},
Expand Down Expand Up @@ -156,10 +157,30 @@ interface PlacesListState {
const [usePlacesListState, PlacesListStateProvider] =
createGenericContext<PlacesListState>();

const PlacesListStateContainer = ({ children }: Props) => {
interface PlacesListStateContainerProps {
children: React.ReactNode;
places?: Place[];
}

const PlacesListStateContainer = ({
children,
places,
}: PlacesListStateContainerProps) => {
const [sortDirection, setSortDirection] = useState<DirectionID>(0);
const [modeLineFilterValue, setModeLineFilterValue] = useState<FilterValue>(
PLACES_PAGE_MODES_AND_LINES[0],
() => {
return places && places.length > 0
? fp
.reverse(PLACES_PAGE_MODES_AND_LINES)
.find(
({ ids }) =>
ids[0] === "All" ||
places.every(
(place) => fp.intersection(ids, place.routes).length > 0,
),
)!
Comment on lines +173 to +181

@rwaskiewicz rwaskiewicz Jul 15, 2026

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,
          ),
      )!;

: PLACES_PAGE_MODES_AND_LINES[0];
},
);
const [screenTypeFilterValue, setScreenTypeFilterValue] =
useState<FilterValue>(SCREEN_TYPES[0]);
Expand Down
2 changes: 1 addition & 1 deletion assets/js/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export const formatEffect = (effect: string) => {
// Filters out screens that don't have the alert, then filters out places with empty
// screens array
export const placesWithSelectedAlert = (
alert: Alert | null,
alert: Alert | undefined,
places: Place[],
screensByAlertMap: ScreensByAlert,
) => {
Expand Down
7 changes: 4 additions & 3 deletions assets/tests/components/alertsPage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ import { renderWithScreenplayProvider } from "../utils/renderWithScreenplayProvi

describe("Alerts Page", () => {
describe("filtering", () => {
test("filters places by mode and route", async () => {
beforeEach(async () => {
renderWithScreenplayProvider(<AlertsPage />);
await screen.findAllByRole("button", { name: "All MODES" });
});

test("filters places by mode and route", async () => {
expect(await screen.findByTestId("1")).toBeInTheDocument();
// Verify alerts not present on a screen are not visible
expect(screen.queryByTestId("5")).toBeNull();
Expand All @@ -33,8 +36,6 @@ describe("Alerts Page", () => {
});

test("filters places by screen type", async () => {
renderWithScreenplayProvider(<AlertsPage />);

fireEvent.click(screen.getByRole("button", { name: "All SCREEN TYPES" }));
fireEvent.click(
await screen.findByRole("button", { name: "Bus Shelter" }),
Expand Down
26 changes: 8 additions & 18 deletions assets/tests/components/placesPage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ import { renderWithScreenplayProvider } from "../utils/renderWithScreenplayProvi

describe("PlacesPage", () => {
describe("filtering", () => {
test("filters places by screen type", async () => {
beforeEach(async () => {
renderWithScreenplayProvider(<PlacesPage />);
await screen.findAllByRole("button", { name: "All MODES" });
});

test("filters places by screen type", async () => {
fireEvent.click(screen.getByRole("button", { name: "All SCREEN TYPES" }));
fireEvent.click(await screen.findByRole("button", { name: "DUP" }));
expect(await screen.findByText("Davis")).toBeInTheDocument();
Expand Down Expand Up @@ -36,8 +39,6 @@ describe("PlacesPage", () => {
});

test("filters places by mode and route", async () => {
renderWithScreenplayProvider(<PlacesPage />);

fireEvent.click(screen.getByRole("button", { name: "All MODES" }));
fireEvent.click(await screen.findByRole("button", { name: "Blue Line" }));
expect(await screen.findByText("WONDERLAND")).toBeInTheDocument();
Expand All @@ -64,8 +65,6 @@ describe("PlacesPage", () => {
});

test("adds `filtered` class to PlaceRow when filtered", async () => {
renderWithScreenplayProvider(<PlacesPage />);

fireEvent.click(screen.getByRole("button", { name: "All MODES" }));
fireEvent.click(await screen.findByRole("button", { name: "Blue Line" }));
expect(
Expand All @@ -74,8 +73,6 @@ describe("PlacesPage", () => {
});

test("reset button clears filters", async () => {
renderWithScreenplayProvider(<PlacesPage />);

fireEvent.click(screen.getByRole("button", { name: "All MODES" }));
fireEvent.click(await screen.findByRole("button", { name: "Blue Line" }));
fireEvent.click(
Expand All @@ -86,9 +83,12 @@ describe("PlacesPage", () => {
});

describe("sorting", () => {
test("sort label changes depending on filter selected", async () => {
beforeEach(async () => {
renderWithScreenplayProvider(<PlacesPage />);
await screen.findAllByRole("button", { name: "All MODES" });
});

test("sort label changes depending on filter selected", async () => {
expect(screen.getByTestId("sort-label").textContent?.trim()).toBe("ABC");
fireEvent.click(screen.getByRole("button", { name: "All MODES" }));
fireEvent.click(await screen.findByRole("button", { name: "Blue Line" }));
Expand All @@ -110,8 +110,6 @@ describe("PlacesPage", () => {
});

test("sort label changes when clicked", async () => {
renderWithScreenplayProvider(<PlacesPage />);

expect(screen.getByTestId("sort-label").textContent?.trim()).toBe("ABC");
fireEvent.click(screen.getByTestId("sort-label"));
expect(
Expand Down Expand Up @@ -140,8 +138,6 @@ describe("PlacesPage", () => {
});

test("sort order changes for RL", async () => {
renderWithScreenplayProvider(<PlacesPage />);

fireEvent.click(screen.getByRole("button", { name: "All MODES" }));
fireEvent.click(await screen.findByRole("button", { name: "Red Line" }));
expect(
Expand All @@ -159,8 +155,6 @@ describe("PlacesPage", () => {
});

test("sort order changes for OL", async () => {
renderWithScreenplayProvider(<PlacesPage />);

fireEvent.click(screen.getByRole("button", { name: "All MODES" }));
fireEvent.click(screen.getByRole("button", { name: "Orange Line" }));
expect(
Expand Down Expand Up @@ -190,8 +184,6 @@ describe("PlacesPage", () => {
});

test("sort order changes for GL", async () => {
renderWithScreenplayProvider(<PlacesPage />);

fireEvent.click(screen.getByRole("button", { name: "All MODES" }));
fireEvent.click(screen.getByRole("button", { name: "Green Line" }));

Expand Down Expand Up @@ -240,8 +232,6 @@ describe("PlacesPage", () => {
});

test("sort order changes for BL", async () => {
renderWithScreenplayProvider(<PlacesPage />);

fireEvent.click(screen.getByRole("button", { name: "All MODES" }));
fireEvent.click(screen.getByRole("button", { name: "Blue Line" }));

Expand Down