feat: HolidayPolicyDetail functional component (data + wire-up)#1601
Open
krisxcrash wants to merge 5 commits intofeat/sdk-569-holiday-policy-detail-presentationfrom
Open
Conversation
…removal Wire up the functional HolidayPolicyDetail container that fetches the holiday pay policy and employee list, handles tab state, employee search, and the remove-employee flow via useHolidayPayPoliciesRemoveEmployeesMutation. ViewHolidaySchedule and ViewHolidayEmployees stubs now delegate to HolidayPolicyDetail with the appropriate default tab. Includes unit tests and endpoint inventory updates. SDK-571
krisxcrash
requested review from
dmortal,
hukid,
jeffredodd and
serikjensen
as code owners
… pattern
Wrap the HolidaysTab DataView in a Box with BoxHeader and the EmployeeTable
in a Box with withPadding={false}, matching the card layout pattern used in
the Employee Dashboard views.
ActionsLayout used Children.toArray to count grid columns, but React fragments are counted as a single child, producing repeat(1, max-content) instead of the intended multi-column layout. Add countGridChildren helper that flattens fragments before counting. Also convert story PolicyActions components to hooks so the fragment is passed directly as the actions prop rather than wrapped in an opaque component element.
serikjensen
requested changes
Apr 24, 2026
Member
serikjensen
left a comment
serikjensen
left a comment
There was a problem hiding this comment.
Just a couple of things on this one! it looks like it is pulling in some hooks patterns from elsewhere in the codebase that we should avoid using here
|
|
||
| const removeEmployeesMutation = useHolidayPayPoliciesRemoveEmployeesMutation() | ||
|
|
||
| const errorHandling = composeErrorHandler([], { submitError, setSubmitError }) |
Member
There was a problem hiding this comment.
oh noes, it's getting confused with some hook patterns from elsewhere in the codebase. I think we can remove this one since errorHandling isn't actually doing anything here. This util will become more relevant when we're doing hooks down the line
|
|
||
| export function HolidayPolicyDetail({ FallbackComponent, ...props }: HolidayPolicyDetailProps) { | ||
| return ( | ||
| <BaseBoundaries |
Member
There was a problem hiding this comment.
Once again, it's getting confused with some hooks patterns. In your case you can just use the Base component here. BaseBoundaries separates some of the layout stuffs out because hooks are non suspense
| ) | ||
|
|
||
| return ( | ||
| <BaseLayout error={errorHandling.errors}> |
Member
There was a problem hiding this comment.
This would then go away once you update Root to use the Base component since that uses BaseLayout under the hood
Replaces BaseBoundaries + useBaseSubmit + composeErrorHandler + BaseLayout with BaseComponent and useBase(), matching the pattern used elsewhere in the codebase (e.g. HolidaySelectionForm). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HolidayPolicyDetail.tsxcontainer that fetches holiday pay policy and employee data viauseHolidayPayPoliciesGetSuspenseanduseEmployeesListSuspense, joins them to build the employee table, and wires up the remove-employee flow viauseHolidayPayPoliciesRemoveEmployeesMutation.ViewHolidayScheduleandViewHolidayEmployeesstubs now delegate toHolidayPolicyDetailwithdefaultTab="holidays"anddefaultTab="employees"respectively, maintaining the same public interface.TIME_OFF_BACK_TO_LIST.PUT /v1/companies/:companyUuid/holiday_pay_policy/remove.Depends on #1600 (SDK-569)
Resolves SDK-571
Test plan
npm run test -- --run src/components/UNSTABLE_TimeOff/HolidayPolicyDetail/HolidayPolicyDetail.test.tsxpasses (7 tests)tsc --noEmitpasses with no type errors