diff --git a/extension/src/popup/components/__tests__/item-list.test.ts b/extension/src/popup/components/__tests__/item-list.test.ts index 58cbb74..057bbaf 100644 --- a/extension/src/popup/components/__tests__/item-list.test.ts +++ b/extension/src/popup/components/__tests__/item-list.test.ts @@ -25,23 +25,27 @@ describe('popup/item-list in org context', () => { document.body.innerHTML = '
'; }); - // Task 3 — list loads via the context-aware message helper. In an org - // context the initial render must populate the list from `org_list_items` - // (the personal entries don't carry over across an org_switch). - it('loads org items (grant-filtered) when context is an org', async () => { - mockGetState.mockReturnValue({ orgContext: 'org-1', entries: [], searchQuery: '', selectedIndex: 0 }); - // org_list_items returns OrgManifestEntry[] (dedicated org type; collection is - // REQUIRED, not the personal ManifestEntry's optional one) — Dev-A mirrors - // OrgManifestEntry into shared/types.ts; the list consumes it at integration. - send.mockResolvedValue({ ok: true, data: [ - { id: 'a', type: 'login', title: 'db', tags: [], collection: 'prod-infra', modified: 1 }, - ]}); + // Task 3 (loading-design-v2) — the switcher loads org entries into state + // before navigating. renderItemList renders whatever is in state.entries and + // does NOT fire its own org_list_items load. This test asserts that: given + // pre-populated state.entries, the rows appear; and no org_list_items call + // is made by renderItemList itself. + it('renders org entries already in state.entries without re-fetching', () => { + mockGetState.mockReturnValue({ + orgContext: 'org-1', + entries: [['a', { + id: 'a', type: 'login', title: 'db', tags: [], collection: 'prod-infra', + modified: 1, favorite: false, attachment_summaries: [], + }]], + searchQuery: '', + selectedIndex: 0, + }); + const app = document.getElementById('app')!; - renderItemList(document.getElementById('app')!); - await Promise.resolve(); - await Promise.resolve(); + renderItemList(app); - expect(send).toHaveBeenCalledWith({ type: 'org_list_items' }); + expect(app.textContent).toContain('db'); + expect(send).not.toHaveBeenCalledWith({ type: 'org_list_items' }); }); // Task 3 — read-only this plan: no add affordance in org context. Dev-C's diff --git a/extension/src/popup/components/__tests__/org-switcher.test.ts b/extension/src/popup/components/__tests__/org-switcher.test.ts index 89b4069..b64205d 100644 --- a/extension/src/popup/components/__tests__/org-switcher.test.ts +++ b/extension/src/popup/components/__tests__/org-switcher.test.ts @@ -12,10 +12,12 @@ vi.mock('../../../shared/state', () => ({ })); import { renderOrgSwitcher } from '../org-switcher'; -import { sendMessage, navigate } from '../../../shared/state'; +import { sendMessage, navigate, getState, setState } from '../../../shared/state'; const send = sendMessage as ReturnType; const nav = navigate as ReturnType; +const mockGetState = getState as ReturnType; +const mockSetState = setState as ReturnType; describe('popup/org-switcher', () => { let host: HTMLElement; @@ -38,12 +40,51 @@ describe('popup/org-switcher', () => { const sel = host.querySelector('select') as HTMLSelectElement; sel.value = 'org-1'; sel.dispatchEvent(new Event('change')); + // Two awaits: one for org_switch, one for the subsequent list load. + // navigate('list') fires after both resolve. + await Promise.resolve(); await Promise.resolve(); expect(send).toHaveBeenCalledWith({ type: 'org_switch', context: 'org-1' }); expect(nav).toHaveBeenCalledWith('list', expect.anything()); }); + // loading-design-v2 — switching to an org loads entries into shared state via + // org_list_items, normalizes them (favorite:false, attachment_summaries:[]), + // and calls setState({ entries }) before navigating. Both popup renderItemList + // and vault renderListPane then render state.entries without a separate load. + it('loads org entries into state after switching to an org', async () => { + // First getState call: initial render needs 'personal' so the select shows + // Personal selected. Second call (inside messageForList after setState): + // needs 'org-1' so messageForList() returns { type: 'org_list_items' }. + mockGetState + .mockReturnValueOnce({ orgContext: 'personal', orgConfigs: [], orgOffline: false }) + .mockReturnValueOnce({ orgContext: 'org-1', orgConfigs: [], orgOffline: false }); + + send.mockImplementation(async (req: { type: string }) => { + if (req.type === 'org_list_configs') return { ok: true, data: [{ orgId: 'org-1', displayName: 'Acme' }] }; + if (req.type === 'org_switch') return { ok: true, data: { context: 'org-1', offline: false } }; + if (req.type === 'org_list_items') return { ok: true, data: [ + { id: 'x1', type: 'login', title: 'Org Login', tags: [], collection: 'eng', modified: 1 }, + ]}; + return { ok: true, data: [] }; + }); + + await renderOrgSwitcher(host); + const sel = host.querySelector('select') as HTMLSelectElement; + sel.value = 'org-1'; + sel.dispatchEvent(new Event('change')); + await Promise.resolve(); + await Promise.resolve(); + + expect(send).toHaveBeenCalledWith({ type: 'org_list_items' }); + expect(mockSetState).toHaveBeenCalledWith({ + entries: [['x1', expect.objectContaining({ + title: 'Org Login', favorite: false, attachment_summaries: [], + })]], + }); + }); + // Task 5 — offline read-only banner. it('offline org_switch renders the writes-disabled banner', async () => { send.mockImplementation(async (req: { type: string }) => diff --git a/extension/src/popup/components/item-list.ts b/extension/src/popup/components/item-list.ts index 6776a19..1a3c8cc 100644 --- a/extension/src/popup/components/item-list.ts +++ b/extension/src/popup/components/item-list.ts @@ -3,7 +3,7 @@ /// to the detail view. import { getState, setState, sendMessage, navigate, escapeHtml, openVaultTab } from '../../shared/state'; -import { currentContext, messageForList, messageForGet } from '../../shared/org-context'; +import { currentContext, messageForList, messageForGet, normalizeOrgEntries } from '../../shared/org-context'; import { showToast } from '../../shared/toast'; import { GLYPH_VAULT_TAB, @@ -33,38 +33,6 @@ function typeIcon(t: ItemType): string { } } -/// Normalize OrgManifestEntry[] (leaner org shape; no attachment_summaries / -/// favorite / icon_hint / group) into the Array<[ItemId, ManifestEntry]> that -/// buildRowsHtml and getFilteredEntries expect. Used by BOTH the sync handler -/// and the context-aware initial load. -function normalizeOrgEntries(data: OrgManifestEntry[]): Array<[ItemId, ManifestEntry]> { - return data.map(e => [e.id, { - id: e.id, - type: e.type, - title: e.title, - tags: e.tags, - modified: e.modified, - trashed_at: e.trashed_at, - favorite: false, - attachment_summaries: [], - // icon_hint, group: undefined (optional in ManifestEntry; omitted for org items) - }]); -} - -/// Context-aware item list loader. Sends list_items or org_list_items depending -/// on the current context, then normalizes the response into entries and calls -/// setState. Called once per context by renderItemList (loop-safe guard). -async function loadEntriesForContext(): Promise { - const resp = await sendMessage(messageForList()); - if (!resp.ok) { - setState({ error: resp.error ?? 'Failed to load items' }); - return; - } - const entries: Array<[ItemId, ManifestEntry]> = currentContext() === 'personal' - ? (resp.data as { items: Array<[ItemId, ManifestEntry]> }).items - : normalizeOrgEntries(resp.data as OrgManifestEntry[]); - setState({ entries }); -} function buildRowsHtml(): string { const state = getState(); @@ -192,16 +160,6 @@ export function renderItemList(app: HTMLElement): void { wireRowClicks(); document.addEventListener('keydown', handleListKeydown); - - // Context-aware initial load (org only; personal uses manual sync). - // Loop-safe guard: setState({loadedContext}) triggers a re-render that calls - // renderItemList again, but the guard now matches so no second load fires. - // When loadEntriesForContext() later resolves and calls setState({entries}), - // loadedContext still matches so still no loop. - if (isOrg && getState().loadedContext !== currentContext()) { - setState({ loadedContext: currentContext() }); - void loadEntriesForContext(); - } } async function openItem(id: ItemId): Promise { diff --git a/extension/src/popup/components/org-switcher.ts b/extension/src/popup/components/org-switcher.ts index 443059c..6d71a21 100644 --- a/extension/src/popup/components/org-switcher.ts +++ b/extension/src/popup/components/org-switcher.ts @@ -6,8 +6,9 @@ // org renders a read-only banner; Dev-C's write UI reads `orgOffline` to // disable writes. -import type { OrgConfigSummary } from '../../shared/types'; +import type { OrgConfigSummary, OrgManifestEntry, ManifestEntry, ItemId } from '../../shared/types'; import { sendMessage, setState, navigate, getState } from '../../shared/state'; +import { messageForList, normalizeOrgEntries } from '../../shared/org-context'; // Module-level teardown (one instance per browser context; popup and vault // each have their own module scope). @@ -59,6 +60,19 @@ export async function renderOrgSwitcher(host: HTMLElement): Promise { existing?.remove(); } + // Load the new context's entries into shared state before navigating. + // messageForList() reads the just-set orgContext so it picks the right SW + // message (list_items for personal, org_list_items for org). Both popup's + // renderItemList and vault's renderListPane read state.entries, so updating + // it here makes both surfaces work correctly on switch. + const listResp = await sendMessage(messageForList()); + if (listResp.ok) { + const entries: Array<[ItemId, ManifestEntry]> = data.context === 'personal' + ? (listResp.data as { items: Array<[ItemId, ManifestEntry]> }).items + : normalizeOrgEntries(listResp.data as OrgManifestEntry[]); + setState({ entries }); + } + navigate('list', {}); }; diff --git a/extension/src/shared/org-context.ts b/extension/src/shared/org-context.ts index c30daba..9947130 100644 --- a/extension/src/shared/org-context.ts +++ b/extension/src/shared/org-context.ts @@ -7,6 +7,7 @@ import { getState } from './state'; import type { Request } from './messages'; +import type { ItemId, ManifestEntry, OrgManifestEntry } from './types'; export function currentContext(): 'personal' | string { return getState().orgContext ?? 'personal'; @@ -19,3 +20,11 @@ export function messageForList(): Request { export function messageForGet(id: string): Request { return currentContext() === 'personal' ? { type: 'get_item', id } : { type: 'org_get_item', id }; } + +// Normalize OrgManifestEntry[] (leaner org shape; no attachment_summaries / +// favorite / icon_hint / group) into the Array<[ItemId, ManifestEntry]> that +// both list renderers (popup renderItemList + vault renderListPane) expect. +// Org rows show no attachment indicator (PM-accepted). +export function normalizeOrgEntries(entries: OrgManifestEntry[]): Array<[ItemId, ManifestEntry]> { + return entries.map(e => [e.id, { ...e, favorite: false, attachment_summaries: [] }]); +} diff --git a/extension/src/shared/popup-state.ts b/extension/src/shared/popup-state.ts index 6cb41b4..777d2bc 100644 --- a/extension/src/shared/popup-state.ts +++ b/extension/src/shared/popup-state.ts @@ -57,11 +57,6 @@ export interface PopupState { orgOffline?: boolean; orgConfigs?: OrgConfigSummary[]; orgCollections?: Collection[]; - // Tracks which context's items were last loaded into `entries`. Used by - // renderItemList to fire a context-aware load exactly ONCE per context - // switch (loop-safe guard: setState({loadedContext}) re-renders, but the - // guard then matches so no second load fires). - loadedContext?: string; // Vault-tab-only fields. The popup surface leaves these at their defaults // (unlocked=false implicit via separate lock-screen view, drawer/panel false). // Kept on the shared shape so VaultState satisfies StateHost.getState()