diff --git a/client/app/rosterhistory/components/UnitFilterBar.jsx b/client/app/rosterhistory/components/UnitFilterBar.jsx
index b2f9039..172b2bc 100644
--- a/client/app/rosterhistory/components/UnitFilterBar.jsx
+++ b/client/app/rosterhistory/components/UnitFilterBar.jsx
@@ -66,7 +66,44 @@ export function UnitFilterBar({ events, unitFilter, onSelect, onClear }) {
return battalions;
}, [events]);
- if (unitMap.size === 0) return null;
+ const hasActiveFilter = Boolean(unitFilter.battalion);
+
+ if (unitMap.size === 0) {
+ // No units in view. If a unit filter is active it filtered everything
+ // out — explain that instead of silently disappearing.
+ if (!hasActiveFilter) return null;
+
+ const filterLabel = [
+ unitFilter.battalion,
+ unitFilter.company &&
+ (unitFilter.company === "HQ" ? "HQ" : `Co. ${unitFilter.company}`),
+ unitFilter.platoon && `Plt. ${unitFilter.platoon}`,
+ ]
+ .filter(Boolean)
+ .join(" · ");
+
+ return (
+
+
+ No recorded changes for{" "}
+ {filterLabel} in
+ the current selection.
+
+
+ Units only appear here when they have recorded changes.
+
+
+
+ );
+ }
const sortedBattalions = [...unitMap.keys()].sort((a, b) => {
const ai = LINE_BATTALION_ORDER.indexOf(a);
@@ -165,6 +202,10 @@ export function UnitFilterBar({ events, unitFilter, onSelect, onClear }) {
>
)}
+
+
+ Only units with recorded changes in the current view are listed.
+
);
}
From 173690b3fdb894d81cfa3d88cd0fb36ce71ec9ab Mon Sep 17 00:00:00 2001
From: SyniRon <66834451+SyniRon@users.noreply.github.com>
Date: Fri, 5 Jun 2026 19:08:12 -0400
Subject: [PATCH 3/4] Address review findings on rosterhistory unit filter bar
(#136/#137 context)
Important fixes:
- Gate UnitFilterBar on not-loading AND not-error in HistoryView and
TodayView so a failed fetch no longer shows a false "No recorded
changes for X" empty state above the error alert (restores
pre-existing no-bar-on-error behavior).
- Only show the unit-blaming empty state when the unit filter is
genuinely the cause: parents now compute preUnitFilteredEvents (all
filters except the unit predicate) and pass preUnitFilteredCount to
UnitFilterBar; when that list is also empty the bar renders nothing
and the parent's accurate generic message (e.g. "All event types
filtered out") shows instead. Parents suppress their generic empty
messages via unitFilterIsCause rather than mere unit-filter presence.
- Restore accessible names on heatmap cell buttons lost in the Radix
Tooltip migration by adding aria-label with the tooltip string.
Suggestions:
- Unify copy to "in the current view" in the new UnitFilterBar strings.
- Use identical wording for the units-listed rule in both UnitFilterBar
locations.
- Reword the UnitFilterBar empty-state comment to describe observable
state instead of overclaiming causality.
- Strengthen suppression-contract comments in HistoryView/TodayView to
name the invariant (UnitFilterBar must receive the same
typeFilteredEvents used for totalVisible) and reflect the new gating.
Co-Authored-By: Claude Opus 4.8 (1M context)
---
.../rosterhistory/components/HistoryView.jsx | 62 ++++++++++++-------
.../rosterhistory/components/TodayView.jsx | 52 +++++++++++-----
.../components/UnitFilterBar.jsx | 22 +++++--
3 files changed, 91 insertions(+), 45 deletions(-)
diff --git a/client/app/rosterhistory/components/HistoryView.jsx b/client/app/rosterhistory/components/HistoryView.jsx
index 93d14b1..8e85fac 100644
--- a/client/app/rosterhistory/components/HistoryView.jsx
+++ b/client/app/rosterhistory/components/HistoryView.jsx
@@ -108,7 +108,10 @@ export function HistoryView() {
return s;
}, [activeData]);
- const typeFilteredEvents = useMemo(
+ // Events with every filter EXCEPT the unit predicate applied. Used to
+ // decide whether an active unit filter is the actual cause of an empty
+ // view (vs. event-type/roster-type filters having emptied it already).
+ const preUnitFilteredEvents = useMemo(
() =>
(activeData?.events ?? []).filter((e) => {
if (!activeFilters.has(e.event_type)) return false;
@@ -119,25 +122,26 @@ export function HistoryView() {
return false;
if (e.roster_type && !activeRosterTypes.has(e.roster_type))
return false;
- if (unitFilter.battalion) {
- const u = parseUnit(e.position_title || "");
- if (!u || u.battalion !== unitFilter.battalion) return false;
- if (unitFilter.company) {
- const co = u.company ?? "HQ";
- if (co !== unitFilter.company) return false;
- }
- if (unitFilter.platoon && u.platoon !== unitFilter.platoon)
- return false;
+ return true;
+ }),
+ [activeData, activeFilters, excludedRecordTypes, activeRosterTypes],
+ );
+
+ const typeFilteredEvents = useMemo(
+ () =>
+ preUnitFilteredEvents.filter((e) => {
+ if (!unitFilter.battalion) return true;
+ const u = parseUnit(e.position_title || "");
+ if (!u || u.battalion !== unitFilter.battalion) return false;
+ if (unitFilter.company) {
+ const co = u.company ?? "HQ";
+ if (co !== unitFilter.company) return false;
}
+ if (unitFilter.platoon && u.platoon !== unitFilter.platoon)
+ return false;
return true;
}),
- [
- activeData,
- activeFilters,
- excludedRecordTypes,
- activeRosterTypes,
- unitFilter,
- ],
+ [preUnitFilteredEvents, unitFilter],
);
const recordTypeCounts = useMemo(() => {
@@ -176,6 +180,11 @@ export function HistoryView() {
const totalVisible =
notable.length + recordGroups.reduce((n, g) => n + g.records.length, 0);
+ // True when an active unit filter is the genuine cause of emptiness:
+ // events survive every other filter, but none match the selected unit.
+ const unitFilterIsCause =
+ Boolean(unitFilter.battalion) && preUnitFilteredEvents.length > 0;
+
// Aggregate snapshots into heatmap buckets. Bucket size scales with range:
// ≤90d → daily, ≤365d → weekly, >365d or All → monthly
const heatmapData = useMemo(() => {
@@ -380,11 +389,13 @@ export function HistoryView() {
{heatmapData.map((entry) => {
const count = totalCount(entry);
const isSelected = entry.date === selectedDay;
+ const cellLabel = `${entry.label}: ${count} change${count !== 1 ? "s" : ""}`;
return (
-
- {`${entry.label}: ${count} change${count !== 1 ? "s" : ""}`}
-
+ {cellLabel}
);
})}
@@ -459,12 +468,13 @@ export function HistoryView() {
presentRosterTypes={presentRosterTypes}
/>
- {!isLoading && (
+ {!isLoading && !isError && (
)}
@@ -506,12 +516,16 @@ export function HistoryView() {
Loading…
)}
- {/* When a unit filter matched nothing, UnitFilterBar renders its own
- explanatory empty state — skip the generic message. */}
+ {/* Skip the generic message only when UnitFilterBar renders its own
+ unit-scoped empty state (unitFilterIsCause). Invariant: this
+ relies on UnitFilterBar receiving the same typeFilteredEvents
+ used to compute totalVisible (plus preUnitFilteredCount for the
+ cause check) — if those ever diverge, restore a fallback
+ message here. */}
{!isError &&
!isLoading &&
totalVisible === 0 &&
- !unitFilter.battalion && (
+ !unitFilterIsCause && (
No changes{" "}
diff --git a/client/app/rosterhistory/components/TodayView.jsx b/client/app/rosterhistory/components/TodayView.jsx
index 45f5883..3b01fcb 100644
--- a/client/app/rosterhistory/components/TodayView.jsx
+++ b/client/app/rosterhistory/components/TodayView.jsx
@@ -92,7 +92,10 @@ export function TodayView() {
return s;
}, [diff]);
- const typeFilteredEvents = useMemo(
+ // Events with every filter EXCEPT the unit predicate applied. Used to
+ // decide whether an active unit filter is the actual cause of an empty
+ // view (vs. event-type/roster-type filters having emptied it already).
+ const preUnitFilteredEvents = useMemo(
() =>
(diff?.events ?? []).filter((e) => {
if (!activeFilters.has(e.event_type)) return false;
@@ -103,19 +106,26 @@ export function TodayView() {
return false;
if (e.roster_type && !activeRosterTypes.has(e.roster_type))
return false;
- if (unitFilter.battalion) {
- const u = parseUnit(e.position_title || "");
- if (!u || u.battalion !== unitFilter.battalion) return false;
- if (unitFilter.company) {
- const co = u.company ?? "HQ";
- if (co !== unitFilter.company) return false;
- }
- if (unitFilter.platoon && u.platoon !== unitFilter.platoon)
- return false;
+ return true;
+ }),
+ [diff, activeFilters, excludedRecordTypes, activeRosterTypes],
+ );
+
+ const typeFilteredEvents = useMemo(
+ () =>
+ preUnitFilteredEvents.filter((e) => {
+ if (!unitFilter.battalion) return true;
+ const u = parseUnit(e.position_title || "");
+ if (!u || u.battalion !== unitFilter.battalion) return false;
+ if (unitFilter.company) {
+ const co = u.company ?? "HQ";
+ if (co !== unitFilter.company) return false;
}
+ if (unitFilter.platoon && u.platoon !== unitFilter.platoon)
+ return false;
return true;
}),
- [diff, activeFilters, excludedRecordTypes, activeRosterTypes, unitFilter],
+ [preUnitFilteredEvents, unitFilter],
);
const recordTypeCounts = useMemo(() => {
@@ -154,6 +164,11 @@ export function TodayView() {
const totalVisible =
notable.length + recordGroups.reduce((n, g) => n + g.records.length, 0);
+ // True when an active unit filter is the genuine cause of emptiness:
+ // events survive every other filter, but none match the selected unit.
+ const unitFilterIsCause =
+ Boolean(unitFilter.battalion) && preUnitFilteredEvents.length > 0;
+
function toggleFilter(type) {
setActiveFilters((prev) => {
const next = new Set(prev);
@@ -306,12 +321,13 @@ export function TodayView() {
presentRosterTypes={presentRosterTypes}
/>
- {!diffLoading && (
+ {!diffLoading && !listError && !diffError && (
)}
@@ -368,18 +384,22 @@ export function TodayView() {
)}
- {/* When a unit filter matched nothing, UnitFilterBar renders its own
- explanatory empty state — skip the generic messages. */}
+ {/* Skip the generic messages only when UnitFilterBar renders its
+ own unit-scoped empty state (unitFilterIsCause). Invariant: this
+ relies on UnitFilterBar receiving the same typeFilteredEvents
+ used to compute totalVisible (plus preUnitFilteredCount for the
+ cause check) — if those ever diverge, restore a fallback
+ message here. */}
{diff &&
totalVisible === 0 &&
diff.events.length > 0 &&
- !unitFilter.battalion && (
+ !unitFilterIsCause && (
diff --git a/client/app/rosterhistory/components/UnitFilterBar.jsx b/client/app/rosterhistory/components/UnitFilterBar.jsx
index 172b2bc..1383726 100644
--- a/client/app/rosterhistory/components/UnitFilterBar.jsx
+++ b/client/app/rosterhistory/components/UnitFilterBar.jsx
@@ -38,7 +38,13 @@ function UnitChip({ label, active, onClick, onClear }) {
);
}
-export function UnitFilterBar({ events, unitFilter, onSelect, onClear }) {
+export function UnitFilterBar({
+ events,
+ unitFilter,
+ onSelect,
+ onClear,
+ preUnitFilteredCount,
+}) {
const unitMap = useMemo(() => {
const battalions = new Map();
@@ -69,10 +75,16 @@ export function UnitFilterBar({ events, unitFilter, onSelect, onClear }) {
const hasActiveFilter = Boolean(unitFilter.battalion);
if (unitMap.size === 0) {
- // No units in view. If a unit filter is active it filtered everything
- // out — explain that instead of silently disappearing.
+ // No units in view. If a unit filter is active, no events in the current
+ // data match it — explain that instead of silently disappearing.
if (!hasActiveFilter) return null;
+ // Only blame the unit filter when it is genuinely the cause: if the
+ // events were already empty before the unit predicate was applied
+ // (e.g. all event types toggled off), the parent's generic message is
+ // the accurate one — render nothing here so it can show.
+ if (preUnitFilteredCount === 0) return null;
+
const filterLabel = [
unitFilter.battalion,
unitFilter.company &&
@@ -87,10 +99,10 @@ export function UnitFilterBar({ events, unitFilter, onSelect, onClear }) {
No recorded changes for{" "}
{filterLabel} in
- the current selection.
+ the current view.
- Units only appear here when they have recorded changes.
+ Only units with recorded changes in the current view are listed.