Add agents list view (#478)#556
Conversation
…S/CARD_ROWS to match Volunteer
…feat/agents-table-list-view
nadavosa
left a comment
There was a problem hiding this comment.
Good feature, @ivannissimrch — the structure follows the volunteer table pattern cleanly, the district lookup fix is correct, and the shared COLUMN_WIDTH constants are a nice cleanup. A few things to address:
🔴 Hardcoded 0 for numberOfOpportunities — AgentTableRow.tsx
<TableCell $width={AGENT_COL_WIDTHS.numOpportunities} data-testid={`agent-opportunities-${id}`}>
0
</TableCell>I understand the BE doesn't send this yet, but displaying 0 for every agent is actively misleading — users will read it as "this agent has zero opportunities." Use "—" as a placeholder (same as you do for district) until the field arrives from the BE:
{activeVolunteers ?? "—"} // how district handles it
// →
{"—"} // for numOpportunities until BE supports itOr remove the column entirely until the data is available.
🟡 Fragile viewMode mapping — Agents.tsx
const viewMode = Object.values(ViewMode)[selectedTabIndex];This relies on JavaScript object key insertion order matching the tab order. If ViewMode values are ever reordered, or a tab is added/removed, this silently breaks — viewMode becomes the wrong value with no type error. Use an explicit array instead:
const VIEW_MODE_BY_TAB = [ViewMode.LIST, ViewMode.CARD, ViewMode.MAP] as const;
const viewMode = VIEW_MODE_BY_TAB[selectedTabIndex] ?? ViewMode.LIST;🟡 districtsList not coerced to undefined — AgentListController.tsx
districtsList={apiFilterOptions?.district}With SDK ≥ 0.0.91, district becomes Voidable<OptionItem[]> (i.e. can be null). AgentCard and AgentTableList type their prop as OptionItem[] | undefined, so passing null will be a type error after the SDK bump. Add ?? undefined here (same pattern as PR #561 does for the same field):
districtsList={apiFilterOptions?.district ?? undefined}Nit
The TABLE_LIMIT constant was moved from the middle of constants.ts to the bottom. Not a problem, but it's a pure noise change that adds diff noise in the file. Can be skipped.
Overall this is solid work — just needs the 0 → "—" fix and the viewMode mapping hardened before merging.
nadavosa
left a comment
There was a problem hiding this comment.
Two non-blocking issues worth addressing: misleading hardcoded '0' for opportunities column, and fragile tab→viewMode mapping. Details in the comment above.
…erce districtsList
|
Thank you for the feedback. I have addressed all three:
|
Description
Adds the list-view (table) variant for the agents dashboard.
Related Issues
Closes #478
Changes
AgentTableListandAgentTableRowwired intoAgentListControllerviaviewModedistrictsList(table and card paths) fixes empty district renderCOLUMN_WIDTHconstants (XS/SM/MD/LG), both agent and volunteer tables migratedWrapAnywhereCellfor long unbreakable text (used by agent email column)CARD_COLUMNS/CARD_ROWSmoved tosrc/config/constants.ts(touches Agent, Volunteer and Opportunity controllers)createAgentTypeMapmoved out oficon.tsxintoAgents/constants.tsdashboard.agents.*now uses BezirkSDK 0.0.91 note (relevant to email column)
Tried to bump to
need4deed-sdk@0.0.91to pick up the typedemailfield onApiAgentGetList.0.0.91 introduced
Voidable<T>(=T | null | undefined) on many fields where they were previouslyT | undefined, which broke 10 typecheck errors across 6 unrelated files. I stayed on0.0.90for this PR and used a cast workaround foremail(with a TODO comment explaining when to remove).numberOfOpportunitiescolumn currently renders0,but currently the BE is not sending the number of opportunities; we haveactiveVolunteers and numActiveVolunteers(which appear to be the same value, possibly duplicate metrics).Screenshots / Demos
Checklist