fix: wire Select all checkbox on Time Off multi-select tables by jeffredodd · Pull Request #1691 · Gusto/embedded-react-sdk

jeffredodd · 2026-05-06T01:59:53Z

Summary

DataTable already calls onSelectAll?.(checked, data) when its header checkbox is toggled, but every Time Off multi-select table dropped the prop on the floor — so toggling the header checkbox did nothing.
Plumbs onSelectAll through the prop chains for the four sites listed in the plan:
- Policy detail Employees tab — TimeOffPolicyDetail → EmployeeTable (via PolicyDetailLayout)
- SelectEmployees standalone (still used by tests/storybook) and SelectEmployeesPresentation (used by SelectEmployeesTimeOff + SelectEmployeesHoliday) → EmployeeTable
- HolidaySelectionFormPresentation → useDataView
Implementations mutate the existing selectedUuids Set with the visible-data argument DataTable hands back, so search-filtered-out rows stay in sync. useSelectEmployeesData exposes a shared handleSelectAll so both the time-off and holiday wizards share one implementation. SelectEmployeesTimeOff keeps its full-employee-record ref in sync the same way it does for single selects.
Adds a presentation-level regression test asserting the header checkbox calls onSelectAll(true, visibleItems).

This is PR 7 in the time-off bug-fix slate captured in ~/.claude/plans/ok-i-want-to-partitioned-balloon.md.

Test plan

npx tsc --noEmit — clean
npm run test -- --run src/components/UNSTABLE_TimeOff/ — 298/298 passing (1 new test)
Manual: at each site (policy detail Employees tab, both Add Employees flows, Holiday selection form) toggle the header checkbox; confirm all visible rows toggle, the indeterminate state appears when only some are selected, and downstream actions (bulk remove count, continue button) reflect the selection

🤖 Generated with Claude Code

Copilot

Pull request overview

Fixes the Time Off multi-select tables so their header “Select all” checkbox actually propagates through the component chain to the existing DataView/DataTable selection API.

Changes:

Added onSelectAll prop plumbing through Time Off employee-table wrappers and policy-detail layout types.
Implemented visible-row bulk selection handlers for policy detail, add-employees, and holiday selection flows.
Added one regression test covering the policy-detail presentation header checkbox behavior.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/components/UNSTABLE_TimeOff/TimeOffPolicyDetail/TimeOffPolicyDetailTypes.ts`	Extends policy-detail employee props to include `onSelectAll`.
`src/components/UNSTABLE_TimeOff/TimeOffPolicyDetail/TimeOffPolicyDetailPresentation.test.tsx`	Adds a presentation test for header-checkbox select-all behavior.
`src/components/UNSTABLE_TimeOff/TimeOffPolicyDetail/TimeOffPolicyDetail.tsx`	Implements bulk-selection state updates for visible employees.
`src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/useSelectEmployeesData.ts`	Adds shared `handleSelectAll` selection logic for employee flows.
`src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesTypes.ts`	Adds `onSelectAll` to standalone SelectEmployees props.
`src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesTimeOff.tsx`	Wires select-all through the time-off add-employees flow and capture ref.
`src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesPresentationTypes.ts`	Adds `onSelectAll` to presentation props.
`src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesPresentation.tsx`	Forwards `onSelectAll` into `EmployeeTable`.
`src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesHoliday.tsx`	Wires select-all through the holiday add-employees flow.
`src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployees.tsx`	Forwards `onSelectAll` in the standalone component.
`src/components/UNSTABLE_TimeOff/shared/PolicyDetailLayout/PolicyDetailLayoutTypes.ts`	Adds `onSelectAll` to layout employee prop typing.
`src/components/UNSTABLE_TimeOff/shared/PolicyDetailLayout/PolicyDetailLayout.tsx`	Forwards `onSelectAll` from layout to `EmployeeTable`.
`src/components/UNSTABLE_TimeOff/shared/EmployeeTable/EmployeeTableTypes.ts`	Adds `onSelectAll` to shared employee-table props.
`src/components/UNSTABLE_TimeOff/shared/EmployeeTable/EmployeeTable.tsx`	Passes `onSelectAll` into `useDataView`.
`src/components/UNSTABLE_TimeOff/HolidaySelectionForm/HolidaySelectionFormTypes.ts`	Adds select-all typing for holiday selection mode.
`src/components/UNSTABLE_TimeOff/HolidaySelectionForm/HolidaySelectionFormPresentation.tsx`	Forwards `onSelectAll` into holiday `useDataView` selection props.
`src/components/UNSTABLE_TimeOff/HolidaySelectionForm/HolidaySelectionForm.tsx`	Implements holiday bulk-selection state updates and wiring.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  selectedUuids: Set<string>
  searchValue: string
  onSelect: (item: EmployeeItem, checked: boolean) => void
+  onSelectAll?: (checked: boolean, visibleItems: EmployeeItem[]) => void


  mode?: 'select'
  selectedHolidayUuids: Set<string>
  onSelectionChange: (item: HolidayItem, selected: boolean) => void
+  onSelectAll?: (selected: boolean, visibleItems: HolidayItem[]) => void


        onSearchClear={onSearchClear}
        selectionMode="multiple"
        onSelect={onSelect}
+        onSelectAll={onSelectAll}


      holidays={holidays}
      selectedHolidayUuids={selectedUuids}
      onSelectionChange={handleSelectionChange}
+      onSelectAll={handleSelectAll}


gusto-fresh-eyes · 2026-05-06T04:15:36Z

Fresh Eyes Review

Found 1 issue in this PR.

Download findings.json — drag the file into Claude or use /add to propose fixes

Generated by Fresh Eyes Reviewer | Share feedback in #ai-code-reviews

DataTable already calls onSelectAll?.(checked, data) when its header checkbox is toggled, but every Time Off multi-select table was passing a no-op (the prop was never plumbed through). Toggling the header checkbox did nothing. Plumb onSelectAll from the four consumers (TimeOffPolicyDetail employees tab, SelectEmployees{Presentation,TimeOff,Holiday}, and HolidaySelectionForm) through their props chains. Implementations mutate the existing selectedUuids Set with the visible-data argument DataTable hands back — the right scope so search-filtered-out rows stay in sync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The Time Off select-all header checkbox lives inside a react-aria <Column> that installs a usePress handler stack. When the click bubbled to the Column, the checkbox's controlled DOM `checked` property desynced from React's _valueTracker — the row state and wrapper class toggled correctly but the input itself stayed visually inverted, so the header could not be unchecked. Stop click/pointerdown/mousedown propagation on the wrapper so the Column press tracker never sees the checkbox click. Keyboard activation (Tab + Space) is unaffected. Also addresses Copilot feedback on the original wiring PR: - onSelectAll is now required on SelectEmployeesProps and HolidaySelectionFormSelectModeProps so a future caller can't re-introduce a no-op header checkbox. - Add header-toggle tests in SelectEmployeesPresentation.test.tsx (both directions) and a full select-all/deselect-all integration test in HolidaySelectionForm.test.tsx. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jeffredodd force-pushed the fix/wire-select-all-time-off-tables branch from 71a018f to e659cdb Compare May 6, 2026 02:36

jeffredodd marked this pull request as ready for review May 6, 2026 02:45

jeffredodd requested a review from dmortal as a code owner May 6, 2026 02:45

Copilot AI review requested due to automatic review settings May 6, 2026 02:45

jeffredodd requested review from hukid, krisxcrash and serikjensen as code owners May 6, 2026 02:45

Copilot started reviewing on behalf of jeffredodd May 6, 2026 02:45 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

gusto-fresh-eyes Bot reviewed May 6, 2026

View reviewed changes

Comment thread src/components/UNSTABLE_TimeOff/TimeOffManagement/SelectEmployees/SelectEmployeesTimeOff.tsx

Copilot started work on behalf of jeffredodd May 6, 2026 04:22 View session

Copilot finished work on behalf of jeffredodd May 6, 2026 04:24

krisxcrash approved these changes May 6, 2026

View reviewed changes

jeffredodd and others added 2 commits May 6, 2026 09:08

jeffredodd enabled auto-merge (squash) May 6, 2026 16:08

jeffredodd force-pushed the fix/wire-select-all-time-off-tables branch from 5bb2761 to 432f045 Compare May 6, 2026 16:08

jeffredodd merged commit 0d70220 into main May 6, 2026
19 checks passed

jeffredodd deleted the fix/wire-select-all-time-off-tables branch May 6, 2026 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: wire Select all checkbox on Time Off multi-select tables#1691

fix: wire Select all checkbox on Time Off multi-select tables#1691
jeffredodd merged 2 commits intomainfrom
fix/wire-select-all-time-off-tables

jeffredodd commented May 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

gusto-fresh-eyes Bot commented May 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jeffredodd commented May 6, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

gusto-fresh-eyes Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fresh Eyes Review

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gusto-fresh-eyes Bot commented May 6, 2026 •

edited

Loading