packages/core/src/DateField/DateField.test.ts (1)

678-736: ⚡ Quick win

Consider adding explicit focus-out trigger and edge-case coverage.

The tests verify the core snapping behavior, but they don't explicitly trigger focusout after typing. While the model value update might implicitly handle this, explicitly blurring the field would make the test flow clearer and more closely match real user interaction.

Additionally, consider adding edge-case tests:

• Values already on step boundaries (0, 15, 30, 45) - should remain unchanged
• Boundary values that wrap (e.g., 59 → 0 of next hour when step is 15)
• Snapping with different step values (e.g., step: { minute: 10 })

🧪 Example: Explicit focusout trigger

 const minute = getByTestId('minute')
 await user.click(minute)
 await user.keyboard('{2}{3}')
+await minute.blur() // Explicitly trigger focusout

 expect(minute).toHaveTextContent('30')

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/DateField/DateField.test.ts` around lines 678 - 736, The
tests in DateField.test.ts for stepSnapping should explicitly trigger a
focus-out after typing and add edge-case specs; update the two existing it
blocks (the ones using setup, getByTestId('minute'), user.click and
user.keyboard) to perform an explicit blur/focusout on the minute field (e.g.,
call user.tab() or fireEvent.blur(minute) after typing) so snapping runs as on
real focus loss, and add new tests that assert: values already on step
boundaries remain unchanged, boundary wrap cases (e.g., typing 59 snaps to 00 of
next hour for step: { minute: 15 }), and a variant using a different step (e.g.,
step: { minute: 10 }) to verify different snapping intervals.

packages/core/src/DateField/DateField.test.ts (2)

678-828: ⚡ Quick win

Consider extracting shared setup to reduce duplication.

Each of the 5 tests in this suite contains nearly identical setup code (~18 lines), differing only in the step.minute value and stepSnapping boolean. This duplication makes maintenance more challenging if the setup pattern needs to change.

Consider extracting a helper function like:

function setupStepSnappingTest(options: { step: number, stepSnapping: boolean }) {
  // shared setup logic
}

This would reduce the ~90 lines of duplicated code while maintaining test clarity.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/DateField/DateField.test.ts` around lines 678 - 828, Tests
in the stepSnapping suite repeat the same setup block across five cases; extract
a helper (e.g., setupStepSnappingTest) that wraps the existing setup(...) call
and accepts parameters for step.minute and stepSnapping, return { user,
getByTestId, rerender } so each test calls setupStepSnappingTest({ step: 15,
stepSnapping: true }) (or other values) instead of duplicating the setup; update
each test to use the helper and keep the existing assertions (references: setup,
rerender, getByTestId, CalendarDateTime).

---

678-828: ⚖️ Poor tradeoff

Consider additional edge case coverage.

The current test suite provides solid coverage for minute segment snapping. Consider adding tests for:

• Equidistant values (e.g., '07' with step 15, which is 7 minutes from both 0 and 15)
• Edge case step: 1 (no effective snapping)
• Non-divisor steps (e.g., step: 7 where 60 % 7 ≠ 0)
• Hour and second segments if they also support snapping

These additions would strengthen confidence in edge case handling, though the current coverage validates the core functionality well.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/DateField/DateField.test.ts` around lines 678 - 828, Add
new test cases inside the existing describe('stepSnapping') suite to cover the
suggested edge cases: create tests that use setup(...) and the same rerender
emit pattern to assert (1) equidistant snapping e.g., type '{0}{7}' into
getByTestId('minute') with step: { minute: 15 } and verify expected
deterministic snap, (2) step: { minute: 1 } to confirm no snapping, (3)
non-divisor step like step: { minute: 7 } to assert correct nearest-step
behavior, and (4) analogous tests for getByTestId('hour') and
getByTestId('second') if those segments support stepSnapping; reuse the existing
test structure (user.click, user.keyboard, user.click to blur,
expect(...).toHaveTextContent(...)) and the same
modelValue/granularity/step/stepSnapping props so tests are consistent with the
current rerender handler.

packages/core/src/DateField/DateField.test.ts (1)

678-980: ⚡ Quick win

Consider adding truly equidistant minute test case.

The test suite includes an equidistant test for hours (line 889: value 10 with step 4, where 8 and 12 are both distance 2), which expects rounding up to 12. However, there's no equivalent truly equidistant test for minutes to confirm the rounding strategy is consistent across all time segments.

With step: { minute: 10 }, typing 25 would be equidistant between 20 and 30 (both at distance 5). Adding this test would verify that the rounding behavior matches the hour implementation (presumably rounding up to 30).

🧪 Proposed test case

it('snaps truly equidistant minute value up to the nearest step', async () => {
  const { user, getByTestId, rerender } = setup({
    dateFieldProps: {
      modelValue: new CalendarDateTime(1980, 1, 20, 12, 0, 0, 0),
      granularity: 'second',
      step: { minute: 10 },
      stepSnapping: true,
    },
    emits: {
      'onUpdate:modelValue': (data: DateValue) => {
        return rerender({
          dateFieldProps: {
            modelValue: data,
            granularity: 'second',
            step: { minute: 10 },
            stepSnapping: true,
          },
        })
      },
    },
  })

  const minute = getByTestId('minute')
  await user.click(minute)
  await user.keyboard('{2}{5}')
  await user.click(getByTestId('second'))

  expect(minute).toHaveTextContent('30')
})

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/DateField/DateField.test.ts` around lines 678 - 980, Add a
new equidistant minute test to the existing "stepSnapping" suite: create a test
(similar to the hour equidistant test) that uses setup with step: { minute: 10 }
and stepSnapping: true, clicks getByTestId('minute'), types '{2}{5}', clicks
getByTestId('second') and asserts the minute field text becomes '30'. Place it
alongside the other tests in DateField.test.ts so the behavior of minute
rounding matches the hour equidistant test.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes