Skip to content

fix: always show Create Policy button in page header#1905

Merged
krisxcrash merged 1 commit into
mainfrom
kw/timeoff-always-show-create-button
May 22, 2026
Merged

fix: always show Create Policy button in page header#1905
krisxcrash merged 1 commit into
mainfrom
kw/timeoff-always-show-create-button

Conversation

@krisxcrash
Copy link
Copy Markdown
Contributor

Summary

  • Removes the policies.length > 0 condition that hid the Create Policy button when the list was empty.
  • The empty-state illustration retains its own Create Policy button for visual emphasis.

Fixes bug: Create button disappears when there are no policies.

Test plan

  • Existing PolicyList.test.tsx passes (26/26)
  • Manually verify: with zero policies, both the header Create button and the empty-state button are visible

@krisxcrash krisxcrash self-assigned this May 21, 2026
@krisxcrash krisxcrash marked this pull request as ready for review May 21, 2026 23:12
@krisxcrash krisxcrash requested a review from a team as a code owner May 21, 2026 23:12
@krisxcrash krisxcrash enabled auto-merge (squash) May 21, 2026 23:46
The Create Policy button was hidden when the policy list was empty,
leaving only the button inside the empty-state illustration. Now the
header button renders regardless of list contents for consistent access.
@jeffredodd jeffredodd force-pushed the kw/timeoff-always-show-create-button branch from 4697284 to 5f85324 Compare May 22, 2026 02:21
@gusto-fresh-eyes
Copy link
Copy Markdown

Fresh Eyes Review

Found 2 issues in this PR.

Additional Findings (outside diff)

  • 🟡 Minor | [fresh_eyes]: test-coverage | src/components/TimeOff/PolicyList/PolicyList.test.tsx:L275
    No test verifies that clicking the header Create button in the empty state (zero policies) fires the TIME_OFF_CREATE_POLICY event. The existing navigation event test at line 279 only runs with the default (non-empty) policy list. Adding a click test for the empty-state scenario would cover the new code path end-to-end.

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


Please 👍🏽 👎🏽 if you found this useful. Generated by Fresh Eyes Reviewer. Get help in #ai-code-reviews

Comment on lines +272 to +273
const buttons = screen.getAllByRole('button', { name: 'Create policy' })
expect(buttons.length).toBeGreaterThanOrEqual(1)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Minor | [fresh_eyes]: test-coverage

The updated test asserts buttons.length >= 1 which doesn't distinguish the header button from the empty-state illustration button. Since the fix specifically makes the header button always visible, consider asserting exactly 2 buttons are present in the empty state (header + empty-state CTA) to pin the new behavior.


Please 👍🏽 👎🏽 if you found this useful. Generated by Fresh Eyes Reviewer. Get help in #ai-code-reviews

@krisxcrash krisxcrash merged commit 962b8a4 into main May 22, 2026
24 of 25 checks passed
@krisxcrash krisxcrash deleted the kw/timeoff-always-show-create-button branch May 22, 2026 02:28
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.

2 participants