Skip to content

Commit

Permalink
fix: elevator closures sorting instability
Browse files Browse the repository at this point in the history
The previous code mutated the `stations` prop when sorting, and also did
not implement a stable sort function[^1]. As a result, the closure list
would be reversed each time a data request occurred. In the common case
where the widget has two pages, this resulted in certain closures never
being shown, since they would always be on the "other" page.

[^1]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#description
  • Loading branch information
digitalcora committed Dec 12, 2024
1 parent c584592 commit e459d6d
Showing 1 changed file with 24 additions and 14 deletions.
38 changes: 24 additions & 14 deletions assets/src/components/v2/elevator/elevator_closures_list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,23 @@ const OutsideClosureList = ({
}: OutsideClosureListProps) => {
const ref = useRef<HTMLDivElement>(null);

// Each index represents a page number and each value represents the number of rows
// on the corresponding page index.
const sortedStations = [...stations].sort((a, b) => {
const aInStation = a.id === stationId;
const bInStation = b.id === stationId;

// Sort all in-station closures above other closures. Within each of those
// groups, sort by station name.
if (aInStation && !bInStation) {
return -1;
} else if (!aInStation && bInStation) {
return 1;
} else {
return a.name.localeCompare(b.name);
}
});

// Each index represents a page number and each value represents the number of
// rows on the corresponding page index.
const [rowCounts, setRowCounts] = useState<number[]>([]);

const numPages = Object.keys(rowCounts).length;
Expand Down Expand Up @@ -167,18 +182,13 @@ const OutsideClosureList = ({
}
ref={ref}
>
{stations
.sort((a, _b) => (a.id === stationId ? 0 : 1))
.map((station) =>
station.id === stationId ? (
<CurrentStationClosureRow
station={station}
key={station.id}
/>
) : (
<ClosureRow station={station} key={station.id} />
),
)}
{sortedStations.map((station) =>
station.id === stationId ? (
<CurrentStationClosureRow station={station} key={station.id} />
) : (
<ClosureRow station={station} key={station.id} />
),
)}
</div>
}
</div>
Expand Down

0 comments on commit e459d6d

Please sign in to comment.