Skip to content

refactor: extract generic EntityTableList component#529

Merged
need4deed merged 12 commits into
developfrom
ivannissimrch/526/refactor/extract-entity-table-list
May 21, 2026
Merged

refactor: extract generic EntityTableList component#529
need4deed merged 12 commits into
developfrom
ivannissimrch/526/refactor/extract-entity-table-list

Conversation

@ivannissimrch
Copy link
Copy Markdown
Collaborator

Description

Extracts a generic EntityTableList<T> component to Dashboard/common/EntityTableList/ so opportunities (#477) and agents (#478) list views can reuse it instead of duplicating the table/header/pagination boilerplate

Also:

  • Replaces selectedTabIndex prop on VolunteerListController with viewMode: ViewMode , controller no longer knows about tab indices
  • Adds VOLUNTEER_COL_WIDTHS as single source of truth for column widths

Layout note: With filters open the filter panel overflows off screen (table takes full width). Tried flex: 1 on the container so filters sit side by side but some columns get clipped and also tried an overlay approach neither is clean. Figma has no spec for this state. What's the preferred behavior?

Related Issues

Closes #526 Part of #441

Changes

  • Add EntityTableList<T> to Dashboard/common/EntityTableList/
  • Add ViewMode type to Dashboard/common/types.ts
  • Add volunteerTableColumns.ts with createVolunteerTableColumns(t)
  • Migrate VolunteerTableList to use EntityTableList
  • Replace selectedTabIndex with viewMode: ViewMode on VolunteerListController

Screenshots / Demos

If UI-related, add before/after or GIFs.

Checklist

  • WITHIN THE SCOPE OF AN ISSUE; No unnecessary files included
  • Tests added/updated
  • Documentation updated
  • CI passes

@ivannissimrch ivannissimrch requested a review from nadavosa May 12, 2026 12:10
@need4deed need4deed requested a review from DarrellRoberts May 18, 2026 12:45
@nadavosa
Copy link
Copy Markdown
Collaborator

Two small things worth addressing:

CSS variable in a generic component
styles.ts uses var(--opportunities-container-gap) inside EntityTableList, which lives in common/. If that variable isn't declared in non-opportunity contexts this will silently collapse to zero. Either introduce a generic variable (e.g. --entity-table-gap), add a fallback value (var(--opportunities-container-gap, 16px)), or accept a gap prop.

viewMode === "list" repeated in VolunteerListController
It appears twice — once for the limit, once for the conditional render. Extracting const isListView = viewMode === "list" (same as the old isListView) keeps the intent clear and avoids the duplication.

@ivannissimrch
Copy link
Copy Markdown
Collaborator Author

Thank you for the feedback. I implemented the following changes.

  • I added --entity-table-gap as a generic CSS variable in the base: root of globals.css and replaced the dependency on --opportunities-container-gap in the shared component.
  • I added const isListView = viewMode === "list" at the top of VolunteerListController to replace the two repeated checks.

Copy link
Copy Markdown
Collaborator

@arturasmckwcz arturasmckwcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
pls check comments and address if you recon they are to the point

@@ -0,0 +1 @@
export type ViewMode = "cards" | "list" | "map";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here I'd suggest to use enum:

export enum ViewMode {
  LIST = "list",
  CARDS = "cards",
  MAP = "map",
};

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done I converted ViewMode to an enum

const router = useRouter();
const tabs = [t("dashboard.volunteers.tabs.tab1"), t("dashboard.volunteers.tabs.tab2")];

const viewMode: ViewMode = selectedTabIndex === 0 ? "list" : "cards";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is totally okay for 2 tabs, but I'd define mapping object, in this case just an array: [...Object.values(ViewMode)][selectedTabIndex]
ViewMode has to be an enum in such a case of course.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or you may list values in an order you need instead of using Object.values()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and no need for spreading as Object.values() creates a new array anyway, so it'd look: Object.values(ViewMode)[selectedTabIndex]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done , I used Object.values(ViewMode)[selectedTabIndex].
Also updated VolunteerListController to use ViewMode.LIST instead of the string literal.

@ivannissimrch ivannissimrch force-pushed the ivannissimrch/526/refactor/extract-entity-table-list branch from 2b84b00 to 8f3b120 Compare May 20, 2026 21:20
@need4deed need4deed merged commit 5af1b3c into develop May 21, 2026
1 check passed
@need4deed need4deed deleted the ivannissimrch/526/refactor/extract-entity-table-list branch May 21, 2026 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: extract generic EntityTableList component before #477 and #478

4 participants