diff --git a/docs/frontend-ui-audit-2026-07-24/TeamInboxReactLifecycle.md b/docs/frontend-ui-audit-2026-07-24/TeamInboxReactLifecycle.md new file mode 100644 index 000000000..749e5ba41 --- /dev/null +++ b/docs/frontend-ui-audit-2026-07-24/TeamInboxReactLifecycle.md @@ -0,0 +1,28 @@ +# Team Inbox React Lifecycle — Frontend UI Audit + +## Scope + +- `src/modules/MainApp/TeamInbox/components/TeamInboxList.tsx` +- `src/modules/MainApp/TeamInbox/TeamInboxView.tsx` +- `src/modules/MainApp/TeamInbox/useTeamInboxWorkItemBody.ts` + +## Summary + +| Verdict | Count | +| ---------------- | ----: | +| fix | 2 | +| keep with reason | 4 | +| abstract | 0 | + +No cross-file design-system sweep candidate was found. + +## Findings + +| Line | Element | Verdict | Reason | Suggested change | +| -------------------------------: | ------------------------------ | ---------------- | -------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- | +| `TeamInboxList.tsx:87` | Recency grouping | fix | Calling `Date.now()` during render violates React render purity and makes the memo depend on an untracked value. | Pass the load-time reference timestamp from the owning view and include it in the memo dependencies. | +| `useTeamInboxWorkItemBody.ts:38` | Selected Work Item body effect | fix | Synchronously resetting state inside the effect causes an extra render and trips the React lifecycle rule. | Tag resolved state with the request key and derive the loading fallback during render when keys differ. | +| `TeamInboxView.tsx:64` | Page load lifecycle | keep with reason | The effect owns one abort controller per request, checks cancellation before state writes, and aborts on dependency change or unmount. | Keep. | +| `TeamInboxView.tsx:313` | Loading/error/empty states | keep with reason | The shared `Placeholder` component expresses all three states consistently with the rest of the application. | Keep. | +| `TeamInboxView.tsx:328` | Split list/detail composition | keep with reason | `SplitViewLayout` and `TeamInboxList` are existing shared layout and feature boundaries; another wrapper would add indirection. | Keep. | +| `TeamInboxList.tsx:92` | Filter tabs and unread badges | keep with reason | The implementation uses the shared `TabPill` control, translated labels, and explicit accessible badge labels. | Keep. | diff --git a/src/features/Org2Cloud/useOrg2CloudRealtime.ts b/src/features/Org2Cloud/useOrg2CloudRealtime.ts index d670b2431..5a5ffbd18 100644 --- a/src/features/Org2Cloud/useOrg2CloudRealtime.ts +++ b/src/features/Org2Cloud/useOrg2CloudRealtime.ts @@ -474,6 +474,7 @@ export function useOrg2CloudRealtime(): void { const unsubscribes: Array<() => void> = []; const orgId = activeRealtimeOrgId; + const orgTeardownAt = orgTeardownAtRef.current; if (!broadcastSignals) { unsubscribes.push( connection.subscribe({ @@ -519,7 +520,7 @@ export function useOrg2CloudRealtime(): void { return () => { for (const unsub of unsubscribes) unsub(); - orgTeardownAtRef.current.set(orgId, Date.now()); + orgTeardownAt.set(orgId, Date.now()); setRosterRealtimeConnected((current) => { if (!(orgId in current)) return current; const next = { ...current }; diff --git a/src/modules/MainApp/TeamInbox/TeamInboxView.tsx b/src/modules/MainApp/TeamInbox/TeamInboxView.tsx index 115da9035..598eee068 100644 --- a/src/modules/MainApp/TeamInbox/TeamInboxView.tsx +++ b/src/modules/MainApp/TeamInbox/TeamInboxView.tsx @@ -57,6 +57,7 @@ const TeamInboxView: React.FC = ({ message: null, }); const [reloadRevision, setReloadRevision] = useState(0); + const [groupingReferenceTime, setGroupingReferenceTime] = useState(0); const [hasMore, setHasMore] = useState(false); const [loadingMore, setLoadingMore] = useState(false); @@ -67,6 +68,7 @@ const TeamInboxView: React.FC = ({ .listPage({ limit: pageSize, signal: abortController.signal }) .then((page) => { if (abortController.signal.aborted) return; + setGroupingReferenceTime(Date.now()); setItems(page.items); setHasMore(page.nextCursor != null); setLoadState({ status: "ready", message: null }); @@ -329,6 +331,7 @@ const TeamInboxView: React.FC = ({ selectedItemId={selectedItemId} totalUnread={totalUnread} unreadCounts={unreadCounts} + groupingReferenceTime={groupingReferenceTime} query={query} loading={loadState.status === "loading"} onQueryChange={setQuery} diff --git a/src/modules/MainApp/TeamInbox/components/TeamInboxList.tsx b/src/modules/MainApp/TeamInbox/components/TeamInboxList.tsx index 53ea98f58..ba7da4aae 100644 --- a/src/modules/MainApp/TeamInbox/components/TeamInboxList.tsx +++ b/src/modules/MainApp/TeamInbox/components/TeamInboxList.tsx @@ -33,6 +33,7 @@ export interface TeamInboxListProps { selectedItemId: string | null; totalUnread: number; unreadCounts: TeamInboxUnreadCounts; + groupingReferenceTime: number; query: string; loading: boolean; onQueryChange: (query: string) => void; @@ -63,6 +64,7 @@ const TeamInboxList: React.FC = ({ selectedItemId, totalUnread, unreadCounts, + groupingReferenceTime, query, loading, onQueryChange, @@ -83,8 +85,8 @@ const TeamInboxList: React.FC = ({ [items, selectedItemId] ); const groups = useMemo( - () => groupTeamInboxItemsByRecency(items, Date.now()), - [items] + () => groupTeamInboxItemsByRecency(items, groupingReferenceTime), + [groupingReferenceTime, items] ); const activeFilterUnread = unreadCounts[filter]; const filterTabs = useMemo( diff --git a/src/modules/MainApp/TeamInbox/useTeamInboxWorkItemBody.test.ts b/src/modules/MainApp/TeamInbox/useTeamInboxWorkItemBody.test.ts new file mode 100644 index 000000000..a26efea95 --- /dev/null +++ b/src/modules/MainApp/TeamInbox/useTeamInboxWorkItemBody.test.ts @@ -0,0 +1,122 @@ +// @vitest-environment jsdom +import { act, createElement } from "react"; +import { type Root, createRoot } from "react-dom/client"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; + +import type { WorkItemTarget } from "./domain"; +import { useTeamInboxWorkItemBody } from "./useTeamInboxWorkItemBody"; + +const mocks = vi.hoisted(() => ({ + readWorkItem: vi.fn(), +})); + +vi.mock("@src/api/http/project", () => ({ + projectApi: { + readWorkItem: mocks.readWorkItem, + readStandaloneWorkItem: vi.fn(), + }, +})); + +function deferred() { + let resolve: (value: T) => void = () => undefined; + const promise = new Promise((promiseResolve) => { + resolve = promiseResolve; + }); + return { promise, resolve }; +} + +const Harness = ({ target }: { target: WorkItemTarget }) => { + const state = useTeamInboxWorkItemBody(target); + return createElement("output", { + "data-body": state.body ?? "", + "data-loading": String(state.loading), + }); +}; + +const reactActEnvironment = globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; +}; + +describe("useTeamInboxWorkItemBody", () => { + let container: HTMLDivElement; + let root: Root; + + beforeAll(() => { + reactActEnvironment.IS_REACT_ACT_ENVIRONMENT = true; + }); + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + vi.clearAllMocks(); + }); + + afterAll(() => { + Reflect.deleteProperty(reactActEnvironment, "IS_REACT_ACT_ENVIRONMENT"); + }); + + it("shows loading for a new target and discards the stale response", async () => { + const first = deferred<{ body: string }>(); + const second = deferred<{ body: string }>(); + mocks.readWorkItem + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise); + + await act(async () => { + root.render( + createElement(Harness, { + target: { + kind: "work_item", + projectId: "project-a", + workItemId: "item-a", + }, + }) + ); + }); + expect(container.querySelector("output")?.dataset.loading).toBe("true"); + + await act(async () => { + root.render( + createElement(Harness, { + target: { + kind: "work_item", + projectId: "project-b", + workItemId: "item-b", + }, + }) + ); + }); + expect(container.querySelector("output")?.dataset.loading).toBe("true"); + + await act(async () => { + first.resolve({ body: "stale body" }); + await first.promise; + }); + expect(container.querySelector("output")?.dataset.loading).toBe("true"); + expect(container.querySelector("output")?.dataset.body).toBe(""); + + await act(async () => { + second.resolve({ body: "current body" }); + await second.promise; + }); + expect(container.querySelector("output")?.dataset.loading).toBe("false"); + expect(container.querySelector("output")?.dataset.body).toBe( + "current body" + ); + }); +}); diff --git a/src/modules/MainApp/TeamInbox/useTeamInboxWorkItemBody.ts b/src/modules/MainApp/TeamInbox/useTeamInboxWorkItemBody.ts index 374fa03c7..b48211e2b 100644 --- a/src/modules/MainApp/TeamInbox/useTeamInboxWorkItemBody.ts +++ b/src/modules/MainApp/TeamInbox/useTeamInboxWorkItemBody.ts @@ -13,6 +13,10 @@ export interface TeamInboxWorkItemBodyState { loading: boolean; } +interface KeyedTeamInboxWorkItemBodyState extends TeamInboxWorkItemBodyState { + requestKey: string; +} + /** * Lazily loads the full Work Item body for the selected assigned inbox item so * the detail preview can render the real content instead of the short list @@ -24,14 +28,15 @@ export function useTeamInboxWorkItemBody( target: WorkItemTarget ): TeamInboxWorkItemBodyState { const { projectId, workItemId } = target; - const [state, setState] = useState({ + const requestKey = `${projectId ?? "standalone"}:${workItemId}`; + const [state, setState] = useState({ + requestKey, body: null, loading: true, }); useEffect(() => { let cancelled = false; - setState({ body: null, loading: true }); const request = projectId ? projectApi.readWorkItem(projectId, workItemId) @@ -41,18 +46,25 @@ export function useTeamInboxWorkItemBody( .then((workItem) => { if (cancelled) return; const body = workItem.body.trim(); - setState({ body: body.length > 0 ? body : null, loading: false }); + setState({ + requestKey, + body: body.length > 0 ? body : null, + loading: false, + }); }) .catch((error: unknown) => { if (cancelled) return; log.warn("Failed to load Team Inbox Work Item body", error); - setState({ body: null, loading: false }); + setState({ requestKey, body: null, loading: false }); }); return () => { cancelled = true; }; - }, [projectId, workItemId]); + }, [projectId, requestKey, workItemId]); - return state; + if (state.requestKey !== requestKey) { + return { body: null, loading: true }; + } + return { body: state.body, loading: state.loading }; }