Skip to content

feat(MonthPicker,YearPicker): add month and year pickers#2434

Open
onmax wants to merge 12 commits intov2from
feat/month-year-pickers
Open

feat(MonthPicker,YearPicker): add month and year pickers#2434
onmax wants to merge 12 commits intov2from
feat/month-year-pickers

Conversation

@onmax
Copy link
Collaborator

@onmax onmax commented Feb 3, 2026

Adds 4 new picker components: MonthPicker, MonthRangePicker, YearPicker, YearRangePicker. Follows existing Calendar/RangeCalendar patterns with Header, Heading, Grid, Cell, CellTrigger, Next, Prev sub-components.

Cap.2026-02-04.at.13.48.41-compressed.mp4

In essence, these changes enable view-switching date pickers à la Mantine. See the new Date Picker View Switching example showing Calendar + MonthPicker + YearPicker composed together.

Related: nuxt/ui#5981
Closes #1933, #2389, #2191, #1730

Summary by CodeRabbit

  • New Features

    • Added MonthPicker, MonthRangePicker, YearPicker, YearRangePicker and a Date Picker view‑switching example.
  • Documentation

    • New comprehensive docs for each picker (API, anatomy, accessibility, usage, props/slots) and meta pages.
  • Examples / Stories

    • Added CSS and Tailwind demo pages plus Storybook stories demonstrating composition and view switching.
  • Tests

    • New extensive test suites covering accessibility, navigation, selection, ranges, and keyboard interactions.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Added four new picker feature sets — MonthPicker, MonthRangePicker, YearPicker, YearRangePicker — including root/context components, grid/cell/trigger primitives, navigation controls, composables (useMonthPicker/useYearPicker + range variants), tests, stories, docs, demos (CSS & Tailwind), registry exports, and new date utilities for month/year grids and comparisons.

Changes

Cohort / File(s) Summary
Docs config & nav
docs/.vitepress/config.ts
Added navigation entries for Month/MonthRange/Year/YearRange pickers and an Examples > Date entry.
Documentation pages & meta
docs/content/docs/*, docs/content/examples/*, docs/content/meta/*
Many new component docs, example (date-picker view switching), and autogenerated meta prop/slot pages for all pickers.
Demos (CSS & Tailwind)
docs/components/demo/*/css/*, docs/components/demo/*/tailwind/*, .../tailwind.config.js
Added demo SFCs, stylesheets, and Tailwind configs for Month/MonthRange/Year/YearRange pickers.
Component registry & package exports
packages/core/constant/components.ts, packages/core/src/index.ts
Registered new component groups and exported Month/MonthRange/Year/YearRange picker modules from core entry.
MonthPicker feature
packages/core/src/MonthPicker/*, .../useMonthPicker.ts, .../MonthPicker.test.ts, .../story/*, .../index.ts
Full MonthPicker implementation: Root/context, Grid/Body/Row, Cell, CellTrigger, Header/Heading, Prev/Next, composable state, tests, stories, demos, and exports.
MonthRangePicker feature
packages/core/src/MonthRangePicker/*, .../useRangeMonthPicker.ts, .../MonthRangePicker.test.ts, .../story/*, .../index.ts
Full MonthRangePicker suite: root/context, range state/highlighting, grid components, navigation, tests, stories, demos, and exports.
YearPicker feature
packages/core/src/YearPicker/*, .../useYearPicker.ts, .../YearPicker.test.ts, .../story/*, .../index.ts
YearPicker implementation with paging, root/context, grid/cell/trigger components, composables, tests, stories, and exports.
YearRangePicker feature
packages/core/src/YearRangePicker/*, .../useRangeYearPicker.ts, .../YearRangePicker.test.ts, .../story/*, .../index.ts
YearRangePicker range behavior mirror: root/context, components, hooks, tests, stories, and exports.
Date utilities
packages/core/src/date/calendar.ts, packages/core/src/date/comparators.ts
Added createMonthGrid/createYearGrid and many month/year comparator & range helper functions used by pickers.
RangeCalendar & Calendar tweaks
packages/core/src/RangeCalendar/*, packages/core/src/Calendar/CalendarRoot.vue
Normalized nullable modelValue, added mouseleave focus reset, and changed Calendar fixedWeeks default to true.
Stories & examples
packages/core/src/*/story/*, docs/components/demo/*/...
Storybook stories and documentation demos for each picker feature added.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant CT as CellTrigger
    participant R as PickerRoot
    participant S as PickerState
    participant D as DateUtils
    rect rgba(200,220,255,0.5)
    U->>CT: click / keyboard action
    CT->>R: request selection / navigation
    R->>S: update modelValue / placeholder / grid (useMonth/useYear)
    S-->>D: validate ranges & bounds (comparators, grids)
    D-->>S: validation & candidate cells
    S-->>R: computed flags (selected, disabled, highlighted)
    R-->>CT: updated context/props
    CT-->>U: UI updates (aria/data attrs, focus)
    end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Poem

🐇 I hopped through months and years today,

Tables, grids, and headings in tidy array,
Roots whispered context, cells learned to sing,
Clicks turned to ranges — what joy they bring!
Docs and demos sparkle under VitePress ray.

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change—adding four new picker components (MonthPicker, YearPicker, and their range variants), which is the primary focus of this changeset.
Linked Issues check ✅ Passed The PR implements all coding requirements from linked issue #1933: dedicated MonthPicker and YearPicker components with both single and range variants, consistent API with existing Calendar patterns, and support for composable view-switching.
Out of Scope Changes check ✅ Passed All changes directly support the new picker components (documentation, demos, tests, utilities) or enable view-switching (RangeCalendar normalization, Calendar fixedWeeks default). Minor updates to existing Calendar/RangeCalendar components support the new feature without introducing unrelated functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/month-year-pickers

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 3, 2026

Open in StackBlitz

npm i https://pkg.pr.new/reka-ui@2434

commit: 55c1f69

@caiotarifa
Copy link

Awesome work, @onmax! I hope to see these PR merged soon.

@caiotarifa
Copy link

@onmax I was testing the components and was wondering whether we should create these components:

  • MonthField
  • MonthRangeField
  • YearField
  • YearRangeField

What do you think?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 11

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🤖 Fix all issues with AI agents
In `@packages/core/src/MonthPicker/MonthPickerPrev.vue`:
- Around line 38-40: The click handler currently calls
rootContext.prevPage(props.prevPage) unconditionally, which can fire when the
rendered element is not a native disabled control (asChild cases); change the
template to invoke a guarded handler (e.g., handleClick) and implement
handleClick to first return early when props.disabled (or disabled) is truthy,
otherwise call rootContext.prevPage(props.prevPage). Update MonthPickerPrev.vue
to replace the inline `@click` with `@click`="handleClick" and add the handleClick
method that checks disabled before invoking rootContext.prevPage.

In `@packages/core/src/MonthPicker/MonthPickerRoot.vue`:
- Around line 178-189: The useMonthPicker call is receiving raw function values
for isMonthDisabled and isMonthUnavailable which breaks reactivity; update the
UseMonthPickerProps interface to accept Ref<Matcher | undefined> for
isMonthDisabled and isMonthUnavailable, and change the call site to pass
propsIsMonthDisabled and propsIsMonthUnavailable (remove the .value extraction)
so the composable receives refs and will react to parent updates; ensure the
useMonthPicker signature and any internal usages dereference those refs where
needed.

In `@packages/core/src/MonthPicker/useMonthPicker.ts`:
- Around line 64-65: The formatter returned from useMonthPicker is non-reactive
to locale changes, causing consumers like MonthPickerCellTrigger
(shortMonthValue, labelText) and MonthRangePickerCellTrigger to render stale
labels; fix by exposing a locale-aware formatter that depends on props.locale:
create a computedWrapper (e.g., computedFormatter = computed(() => {
formatter.setLocale(props.locale.value); return formatter; })) and replace
usages/returns of formatter with computedFormatter (and update headingValue to
use computedFormatter as well) so consumers declare props.locale as an implicit
dependency and re-evaluate when locale changes; keep the original formatter
variable and use its setLocale inside the computed to sync locale.

In `@packages/core/src/MonthRangePicker/MonthRangePickerNext.vue`:
- Around line 36-41: The click handler currently calls
rootContext.nextPage(props.nextPage) unconditionally, so when the component is
rendered as a non-button (asChild or other) clicks still fire even if disabled;
update MonthRangePickerNext.vue to guard the handler: replace the inline `@click`
call with a local method (e.g., onNextClick) that checks the disabled state (or
an isDisabled ref/value consistent with MonthPickerCellTrigger) and returns
early when true, otherwise calls rootContext.nextPage(props.nextPage); apply the
same pattern to the sibling components (CalendarNext, CalendarPrev,
MonthPickerNext/Prev, RangeCalendarNext/Prev, YearPickerNext/Prev and their
range variants).

In `@packages/core/src/MonthRangePicker/useRangeMonthPicker.ts`:
- Around line 96-128: The capping logic in highlightedRange is inverted and can
produce start > end; instead of always computing cappedEnd from start, branch by
direction: when isStartBeforeFocused is true (props.start < props.focused)
compute cappedEnd = the lesser of props.focused.value and start.add({ months:
props.maximumMonths.value - 1 }) and return { start, end: cappedEnd }; when
isStartBeforeFocused is false compute cappedStart = the greater of
props.focused.value and start.subtract({ months: props.maximumMonths.value - 1
}) and return { start: cappedStart, end } (use props.maximumMonths, start, end,
and props.focused.value to locate and implement the fix).
- Around line 35-41: The computed isInvalid in useRangeMonthPicker (and the
analogous block in useRangeYearPicker) has its first condition inverted — change
the logic so if isStartInvalid.value || isEndInvalid.value is true the computed
returns true, otherwise proceed to compareYearMonth(props.end.value,
props.start.value) < 0; fix highlightedRange capping: when !isStartBeforeFocused
the end bound should be capped to start.add(...) or otherwise ensure the capped
end is >= start to avoid producing end < start (adjust the subtract to an add or
flip the cap logic in the highlightedRange computation where
isStartBeforeFocused is used); remove the unreachable branch in
rangeIsMonthDisabled (the condition that always returns false) and simplify the
function to only evaluate the meaningful enable/disable checks. Ensure you
update both useRangeMonthPicker and useRangeYearPicker where mentioned
(isInvalid, highlightedRange, rangeIsMonthDisabled and compareYearMonth usages).

In `@packages/core/src/YearPicker/YearPickerNext.vue`:
- Line 40: The click handler currently calls
rootContext.nextPage(props.nextPage) unconditionally, which allows clicks to
fire when the control is rendered as a non-<button> via asChild; change the
handler to guard against the disabled state (e.g. check props.disabled or a
relevant disabled prop) and return early when disabled so the action isn't
invoked. Update the `@click` in YearPickerNext.vue to call a small method or use a
conditional expression that only calls rootContext.nextPage(props.nextPage) when
not disabled; apply the same guarded pattern used for CalendarNext and
RangeCalendarNext to keep behavior consistent.

In `@packages/core/src/YearRangePicker/useRangeYearPicker.ts`:
- Around line 35-41: The computed property isInvalid currently returns false
when either isStartInvalid.value or isEndInvalid.value is true; change the logic
so that if isStartInvalid.value || isEndInvalid.value the computed returns true
(i.e., mark range invalid when either boundary is disabled). Update the same
inversion in the analogous computed named isInvalid inside
useRangeMonthPicker.ts and useRangeCalendar.ts (adjust the early return for
isStartInvalid/isEndInvalid there to return true), and keep the existing
year-order check (end.year < start.year) intact.
- Around line 96-128: The capping logic in the highlightedRange computed uses
isStartBeforeFocused (which reflects original prop order) after swapping start
and end so it can choose subtract and produce an inverted range; update the
capping in useRangeYearPicker's highlightedRange so that once start is set to
the earlier year you always use start.add({ years: props.maximumYears.value - 1
}) for cappedEnd (do not subtract), and only replace end with cappedEnd when the
computed span actually exceeds props.maximumYears.value (e.g., when end.year -
start.year + 1 > props.maximumYears.value); apply the identical fix in
useRangeMonthPicker (use add months and check month span against
props.maximumMonths.value) and keep references to highlightedRange,
isStartBeforeFocused, start, end, cappedEnd, and
props.maximumYears/maximumMonths to locate the changes.

In `@packages/core/src/YearRangePicker/YearRangePickerCellTrigger.vue`:
- Around line 193-225: The shiftFocus function can recurse indefinitely; add a
guarded attempt counter (e.g., a new optional parameter like attempts = 0 and a
MAX_ATTEMPTS constant) to both recursion sites in shiftFocus to abort after N
tries; increment attempts when calling shiftFocus(candidateYearValue, add) and
when scheduling nextTick(() => shiftFocus(currentYear, add, attempts + 1)), and
return (or focus fallback) when attempts >= MAX_ATTEMPTS or when pagination no
longer changes the DOM (detect by comparing a page token or visible years
snapshot from parentElement) to prevent infinite loops; update any
rootContext.nextPage()/prevPage() flow to pass the updated attempts through the
nextTick callback and ensure disabled-element skipping stops after the cap.

In `@packages/core/src/YearRangePicker/YearRangePickerRoot.vue`:
- Around line 311-317: The keydown listener currently uses
useEventListener('keydown', ...) which attaches to window and reverts edits
whenever Escape is pressed anywhere; change it to scope the listener to the
picker root (e.g., useEventListener('keydown', handler, { target: parentElement
}) or pass parentElement as the second arg) so it only triggers when the picker
or its descendants have focus; inside the handler (still referencing kbd.ESCAPE,
isEditing, startValue, endValue, validModelValue) optionally guard by checking
that parentElement.value contains(ev.target) or document.activeElement is inside
parentElement before resetting startValue/endValue to validModelValue copies.
🟡 Minor comments (12)
docs/components/demo/MonthPicker/css/styles.css-117-134 (1)

117-134: ⚠️ Potential issue | 🟡 Minor

Today indicator dot is not horizontally centered.

The ::before pseudo-element has position: absolute and top: 0.25rem but no horizontal positioning. It will render near the left edge of the cell instead of centered above the text.

🐛 Proposed fix
 .MonthPickerCellTrigger::before {
   content: '';
   position: absolute;
   top: 0.25rem;
+  left: 50%;
+  transform: translateX(-50%);
   width: 0.25rem;
   height: 0.25rem;
   border-radius: 9999px;
   display: none;
 }
packages/core/src/YearRangePicker/useRangeYearPicker.ts-88-94 (1)

88-94: ⚠️ Potential issue | 🟡 Minor

Remove dead code: lines 90–93 always return false.

The condition on line 90 is meaningless—both the if branch (line 91) and the fallthrough (line 93) return false. This appears in both useRangeYearPicker.ts and useRangeMonthPicker.ts, suggesting incomplete implementation. Simplify to a single return false or implement the intended logic for handling non-contiguous range restrictions.

packages/core/src/YearPicker/YearPickerCellTrigger.vue-115-147 (1)

115-147: ⚠️ Potential issue | 🟡 Minor

Potential unbounded recursion in shiftFocus when many consecutive years are disabled.

When the candidate year isn't in the DOM (lines 125-139), shiftFocus paginates and recurses via nextTick. After pagination, if the candidate is found but disabled (line 142-143), it recurses synchronously with add incremented by one year. In scenarios without minValue/maxValue bounds and a long stretch of disabled years, this could produce a deep call stack.

The bounds check at lines 118-121 prevents infinite recursion only when min/max are set. Consider either:

  1. Adding a recursion depth limit, or
  2. Documenting that minValue/maxValue should always be set when using isYearDisabled/isYearUnavailable.

This is a narrow edge case, so flagging for awareness.

packages/core/src/YearPicker/YearPickerCell.vue-24-26 (1)

24-26: ⚠️ Potential issue | 🟡 Minor

aria-disabled may render "false" instead of being omitted.

Line 25 evaluates to false when neither disabled nor unavailable, causing aria-disabled="false" to be rendered. The sibling YearPickerCellTrigger (line 187) correctly normalizes to true | undefined using a ternary. Apply the same pattern here for consistency.

Proposed fix
-    :aria-disabled="rootContext.isYearDisabled(date) || rootContext.isYearUnavailable?.(date)"
+    :aria-disabled="rootContext.isYearDisabled(date) || rootContext.isYearUnavailable?.(date) ? true : undefined"
packages/core/src/MonthRangePicker/MonthRangePickerRoot.vue-217-218 (1)

217-218: ⚠️ Potential issue | 🟡 Minor

Matcher props lose reactivity due to architectural difference from Calendar/RangeCalendar.

Lines 217-218 pass unwrapped .value to useMonthPicker, which wraps them in local functions and returns them. These wrapped functions are then passed to useRangeMonthPickerState (lines 224-232). Since useMonthPicker is called once at mount, the wrapped functions capture the matcher at that moment. If a parent changes the isMonthDisabled or isMonthUnavailable prop later, the change won't propagate.

This pattern differs from Calendar and RangeCalendar, which pass original refs directly to their state composables (useCalendarState and useRangeCalendarState), allowing those composables to read dynamic updates. To align with the codebase pattern and support reactive matcher changes, useRangeMonthPickerState should receive the original refs from toRefs(props) rather than the wrapped output from useMonthPicker.

docs/content/meta/YearRangePickerRoot.md-106-110 (1)

106-110: ⚠️ Potential issue | 🟡 Minor

Fix misleading "checked state" wording in date picker JSDoc comments.

The modelValue prop description in YearRangePickerRoot.vue (line 91-92) says "The controlled checked state of the calendar," which is inappropriate for a date picker. The term "checked state" is semantically correct only for checkbox-like controls, not date/time pickers. This wording appears to be systematically copied across multiple date/time picker components including DatePickerRoot, DateRangePickerRoot, DateFieldRoot, RangeCalendarRoot, MonthRangePickerRoot, MonthPickerRoot, TimeFieldRoot, and others.

Update the JSDoc to use "selected value" or "selected date range" instead:

/** The controlled selected date range of the calendar. Can be bound as `v-model`. */
docs/content/meta/MonthRangePickerRoot.md-1-1 (1)

1-1: ⚠️ Potential issue | 🟡 Minor

Minor typo in auto-generation template: "automatic generated" → "automatically generated".

This appears in the auto-generation template and likely affects all generated meta files.

packages/core/src/YearRangePicker/YearRangePickerPrev.vue-31-45 (1)

31-45: ⚠️ Potential issue | 🟡 Minor

Guard the click handler to prevent execution when disabled and rendered as a non-button element.

When the component is rendered as a non-button element (e.g., <div>), the native disabled attribute has no effect. The click handler on line 40 will still invoke rootContext.prevPage even when disabled is true, unless CSS prevents it via pointer-events: none. Add an explicit guard to prevent this:

♻️ Guard the click handler
-    `@click`="rootContext.prevPage(props.prevPage)"
+    `@click`="!disabled && rootContext.prevPage(props.prevPage)"

This pattern is already implemented in PaginationPrev.vue and should be consistent across all similar components.

packages/core/src/YearRangePicker/YearRangePickerRoot.vue-216-228 (1)

216-228: ⚠️ Potential issue | 🟡 Minor

Same reactivity concern as MonthPickerRoot: isYearDisabled/isYearUnavailable passed as unwrapped values.

Lines 223-224 pass .value from the refs, which means dynamic changes to these props won't propagate into the composable.

packages/core/src/MonthPicker/MonthPickerRoot.vue-82-87 (1)

82-87: ⚠️ Potential issue | 🟡 Minor

Emit type for update:modelValue doesn't cover DateValue[].

The modelValue prop accepts DateValue | DateValue[] | undefined, but the emit signature only declares DateValue | undefined. When multiple is true, the component will emit DateValue[], which won't match.

Proposed fix
 export type MonthPickerRootEmits = {
   /** Event handler called whenever the model value changes */
-  'update:modelValue': [date: DateValue | undefined]
+  'update:modelValue': [date: DateValue | DateValue[] | undefined]
   /** Event handler called whenever the placeholder value changes */
   'update:placeholder': [date: DateValue]
 }
packages/core/src/MonthPicker/MonthPickerRoot.vue-226-230 (1)

226-230: ⚠️ Potential issue | 🟡 Minor

Missing .copy() when appending to the multi-select array.

Line 229 spreads the existing array and appends value without calling .copy(). Other paths in this function consistently copy date values (e.g., Line 224: value.copy(), Line 238: date.copy()). This inconsistency risks shared-reference mutations if value is later modified externally.

Proposed fix
-      modelValue.value = [...modelValue.value, value]
+      modelValue.value = [...modelValue.value, value.copy()]
packages/core/src/YearRangePicker/YearRangePickerCellTrigger.vue-258-284 (1)

258-284: ⚠️ Potential issue | 🟡 Minor

Replace v-bind="props" with explicit prop bindings to prevent the year object from being rendered as a DOM attribute.

The component spreads all props including the year (DateValue) object via v-bind="props". Since Primitive only accepts as and asChild as props, the year property becomes a fallthrough attribute and is passed to the rendered DOM element, converting the complex object to a string representation (likely [object Object]). This pattern affects multiple CellTrigger components throughout the codebase.

Proposed fix
   <Primitive
     ref="primitiveElement"
-    v-bind="props"
+    :as="as"
+    :as-child="asChild"
     role="button"
🧹 Nitpick comments (23)
packages/core/src/MonthRangePicker/MonthRangePickerCell.vue (1)

19-29: aria-disabled binding is inconsistent with aria-selected across all picker Cell components, not just this one.

Line 24 uses rootContext.isSelected(date) ? true : undefined to omit aria-selected when false. However, line 25 uses a bare || expression for aria-disabled, which renders aria-disabled="false" when both isMonthDisabled and isMonthUnavailable are falsy. This inconsistency exists across all picker Cell components (MonthPickerCell, YearPickerCell, YearRangePickerCell, CalendarCell, RangeCalendarCell), where aria-disabled uses the bare || pattern while both aria-selected and data-disabled use ternary guards. While aria-disabled="false" is semantically valid ARIA, applying the ternary pattern consistently would align with the codebase conventions:

-    :aria-disabled="rootContext.isMonthDisabled(date) || rootContext.isMonthUnavailable?.(date)"
+    :aria-disabled="(rootContext.isMonthDisabled(date) || rootContext.isMonthUnavailable?.(date)) || undefined"

Consider addressing this across all affected Cell components for consistency.

packages/core/src/MonthPicker/MonthPickerNext.vue (1)

32-45: Minor inconsistency: props.as vs bare as in template.

Line 33 uses props.as but Line 36 uses bare as. Both work in Vue 3 templates, but mixing styles reduces readability.

🔧 Suggested fix
-    :type="as === 'button' ? 'button' : undefined"
+    :type="props.as === 'button' ? 'button' : undefined"
packages/core/src/YearPicker/YearPickerPrev.vue (1)

33-44: Inconsistent aria-label and slot text compared to sibling components.

MonthPickerNext uses aria-label="Next year" and slot text "Next year", while this component uses aria-label="Previous page" and "Prev page". For better accessibility and consistency, consider using a more descriptive label (e.g., "Previous decade" or "Previous years") that aligns with the component's purpose.

packages/core/src/date/calendar.ts (1)

200-218: JSDoc slightly misleading: "3x4" depends on yearsPerPage.

The comment says "Creates a 3x4 grid" but that's only true for the default yearsPerPage = 12. A caller passing, say, 16 would get a 4×4 grid.

📝 Suggested doc fix
 /**
- * Creates a 3x4 grid of years (decade-aligned).
- * The grid starts from the decade that contains the given date.
+ * Creates a grid of years (default 3x4, decade-aligned).
+ * Rows are chunked into groups of 4 columns.
  */
docs/components/examples/DatePickerViewSwitching/index.vue (1)

92-94: Hardcoded English month labels won't adapt to locale.

This is a documentation example, so hardcoded labels may be intentional. However, if locale-awareness is desired, the placeholder date can be formatted using Intl.DateTimeFormat or similar.

packages/core/src/MonthRangePicker/useRangeMonthPicker.ts (1)

88-94: Dead code: both branches of lines 90–93 unconditionally return false.

After the maximumMonths block, the remaining logic always evaluates to false regardless of the condition on line 90. This looks like an incomplete non-contiguous-range or unavailability check was intended but left as a no-op.

♻️ Simplify by collapsing to a single return
     }
 
-    if (!props.start.value || props.end.value || isSameYearMonth(props.start.value, date))
-      return false
-
     return false
   }
packages/core/src/MonthRangePicker/MonthRangePickerCellTrigger.vue (1)

193-225: Note: shiftFocus can recurse unboundedly if many consecutive months are disabled with no min/max constraint.

Line 221 recurses directly when a candidate month has data-disabled, and lines 214–216 recurse via nextTick after page navigation. Without minValue/maxValue bounds, a long sequence of disabled months could exhaust the call stack. This matches the existing Calendar/RangeCalendar behavior and may be an accepted trade-off in the codebase.

packages/core/src/YearPicker/useYearPicker.ts (1)

94-98: Nit: isYearUnavailable can be simplified.

The if (true) return true; return false pattern can be reduced.

♻️ Simplify
-  const isYearUnavailable = (date: DateValue) => {
-    if (props.isYearUnavailable?.(date))
-      return true
-    return false
-  }
+  const isYearUnavailable = (date: DateValue) => props.isYearUnavailable?.(date) ?? false
packages/core/src/YearPicker/YearPicker.test.ts (2)

38-38: Unnecessary async on describe callback.

The async modifier on the describe callback at Line 38 serves no purpose since describe doesn't await its callback. While vitest tolerates it, it's a minor inconsistency — none of the other describe blocks in this file use it.

Proposed fix
-describe('year picker', async () => {
+describe('year picker', () => {

25-27: Return type hides possible null.

querySelector returns HTMLElement | null, but the as HTMLElement cast strips the null. This is contradicted by Line 92 which explicitly asserts the result is null. Consider returning HTMLElement | null for clarity.

Proposed fix
 function getSelectedYear(picker: HTMLElement) {
-  return picker.querySelector<HTMLElement>('[data-selected]') as HTMLElement
+  return picker.querySelector<HTMLElement>('[data-selected]')
 }
packages/core/src/MonthPicker/useMonthPicker.ts (1)

158-162: Grid watch short-circuits on same year — consider calendar identifier.

The watch only compares value.year === grid.value.value.year. If the calendar system changes (e.g., switching between Gregorian and non-Gregorian calendars while the numeric year happens to stay the same), the grid won't regenerate. This is unlikely in practice but worth noting.

packages/core/src/MonthRangePicker/MonthRangePickerRoot.vue (1)

305-311: ESC-key handler is globally scoped — may interfere with other components.

useEventListener('keydown', ...) without a target element listens on window by default. If multiple MonthRangePickers are mounted, or if other components also handle ESC, this could cause unintended reverts. Consider scoping the listener to parentElement or checking document.activeElement is within the picker.

docs/.vitepress/config.ts (1)

195-210: Consider inserting the new picker items in alphabetical order within the Dates section.

The four new entries are appended after "Time Field", but alphabetically "Month Picker" and "Month Range Picker" should appear before "Range Calendar", and "Year Picker" / "Year Range Picker" after "Time Field". This would keep the sidebar consistent and easier to scan as the component list grows.

Suggested ordering
               {
                 text: `Range Calendar ${BadgeHTML('Alpha', true)}`,
                 link: '/docs/components/range-calendar',
               },
               {
                 text: `Time Field ${BadgeHTML('Alpha', true)}`,
                 link: '/docs/components/time-field',
               },
-              {
-                text: `Month Picker ${BadgeHTML('Alpha', true)}`,
-                link: '/docs/components/month-picker',
-              },
-              {
-                text: `Month Range Picker ${BadgeHTML('Alpha', true)}`,
-                link: '/docs/components/month-range-picker',
-              },
-              {
-                text: `Year Picker ${BadgeHTML('Alpha', true)}`,
-                link: '/docs/components/year-picker',
-              },
-              {
-                text: `Year Range Picker ${BadgeHTML('Alpha', true)}`,
-                link: '/docs/components/year-range-picker',
-              },

Then insert Month pickers before Range Calendar, and Year pickers after Time Field:

              ...Date Range Picker...
+             { Month Picker (Alpha) }
+             { Month Range Picker (Alpha) }
              { Range Calendar (Alpha) }
              { Time Field (Alpha) }
+             { Year Picker (Alpha) }
+             { Year Range Picker (Alpha) }
packages/core/src/MonthPicker/MonthPickerPrev.vue (1)

31-45: Minor: inconsistent prop access between props.as and bare as in the template.

Lines 34–35 reference props.as and props.asChild, but line 36 uses bare as. While both work in Vue <script setup> templates (props are auto-exposed), mixing the two styles in the same element is inconsistent.

♻️ Suggested fix for consistency
-    :type="as === 'button' ? 'button' : undefined"
+    :type="props.as === 'button' ? 'button' : undefined"
packages/core/src/YearRangePicker/YearRangePickerNext.vue (1)

31-45: Minor inconsistency in prop referencing style.

Lines 33–34 reference props.as and props.asChild, but Line 36 uses bare as. Both work in Vue templates, but mixing styles in the same block reduces readability.

♻️ Suggested consistency fix
-    :type="as === 'button' ? 'button' : undefined"
+    :type="props.as === 'button' ? 'button' : undefined"
docs/components/demo/YearRangePicker/css/styles.css (1)

1-2: Unused black-alpha import.

@radix-ui/colors/black-alpha.css is imported but no --black-a* CSS variables appear to be referenced in this stylesheet. If it's included only for consistency with other demo CSS files, that's fine, but otherwise it adds unnecessary weight.

packages/core/src/MonthPicker/MonthPickerCell.vue (1)

24-26: aria-disabled binding is inconsistent with the aria-selected pattern.

Line 24 correctly uses a ternary (? true : undefined) to avoid rendering aria-selected="false" in the DOM. However, Line 25 uses a bare boolean expression, which will render aria-disabled="false" when both checks return false—adding a redundant attribute. Consider normalizing:

♻️ Normalize the aria-disabled binding
-    :aria-disabled="rootContext.isMonthDisabled(date) || rootContext.isMonthUnavailable?.(date)"
+    :aria-disabled="rootContext.isMonthDisabled(date) || rootContext.isMonthUnavailable?.(date) || undefined"
packages/core/src/MonthPicker/MonthPicker.test.ts (1)

25-27: as HTMLElement cast masks nullable return.

querySelector returns HTMLElement | null, but the cast to HTMLElement hides the null case from TypeScript. Line 92 asserts toBe(null), which works at runtime but the type lies. Consider returning HTMLElement | null instead to keep the types honest.

 function getSelectedMonth(picker: HTMLElement) {
-  return picker.querySelector<HTMLElement>('[data-selected]') as HTMLElement
+  return picker.querySelector<HTMLElement>('[data-selected]')
 }
packages/core/src/YearRangePicker/YearRangePicker.test.ts (1)

7-7: mount from @vue/test-utils is only used for the axe test.

The rest of the suite uses render from @testing-library/vue (via setup()). The MonthPicker tests use setup() for the axe test too. Consider using setup() here as well for consistency and dropping the mount import.

Align axe test with other suites
 it('should pass axe accessibility tests', async () => {
-  const wrapper = mount(YearRangePicker)
-  expect(await axe(wrapper.element)).toHaveNoViolations()
+  const { picker } = setup()
+  expect(await axe(picker)).toHaveNoViolations()
 })
packages/core/src/MonthPicker/MonthPickerCellTrigger.vue (2)

116-148: shiftFocus recursion skipping disabled months has no depth guard.

Line 144 recurses synchronously for each consecutive disabled month. In a 12-month page, this is bounded to ~11 calls, but if the recursion crosses page boundaries (via the async nextTick path at line 137-139 chaining back into sync recursion), a long stretch of disabled months across many years could produce deep async recursion chains.

A simple iteration limit would make this defensive:

Suggested defensive guard
-  function shiftFocus(currentMonth: DateValue, add: number) {
+  function shiftFocus(currentMonth: DateValue, add: number, depth = 0) {
+    if (depth > 24)
+      return
     const candidateMonthValue = currentMonth.add({ months: add })
     ...
     if (!candidateMonth) {
       ...
       nextTick(() => {
-        shiftFocus(currentMonth, add)
+        shiftFocus(currentMonth, add, depth + 1)
       })
       return
     }

     if (candidateMonth && candidateMonth.hasAttribute('data-disabled'))
-      return shiftFocus(candidateMonthValue, add)
+      return shiftFocus(candidateMonthValue, add, depth + 1)

34-34: Consider defaulting as to 'button' for improved accessibility consistency.

The component implements role="button" with comprehensive keyboard handling (arrows, Enter, Space, Page Up/Down at lines 195-196) and manual focus management at line 194. A native <button> element would simplify the implementation by providing built-in focusability and semantic meaning without the need for custom tabindex logic.

Note: Similar cell trigger components across Calendar, YearPicker, and RangeCalendar follow the same as: 'div' pattern, so this change would ideally be applied consistently across the library rather than to this component alone.

packages/core/src/YearRangePicker/YearRangePickerRoot.vue (1)

366-395: Minor template ordering inconsistency with MonthPickerRoot.

Here the visually-hidden heading is rendered before the slot (Lines 377-386 then 388-393), while in MonthPickerRoot.vue it's rendered after the slot. Both are functionally equivalent, but consistent ordering across the picker family would improve maintainability.

packages/core/src/YearRangePicker/YearRangePickerCellTrigger.vue (1)

84-103: Redundant condition on Line 100 — always evaluates to true.

lastPressedDateValue was just set to date.copy() on Line 90, so isSameYear(rootContext.lastPressedDateValue.value, date) is always true when this branch is reached. The if guard can be removed for clarity, or the intent may have been to check the previous value before the assignment on Line 90.

@onmax
Copy link
Collaborator Author

onmax commented Feb 6, 2026

@caiotarifa Good question. I think MonthField/YearField (and the range variants) are a separate feature from this PR. MonthPicker/YearPicker are Calendar-like grid pickers; fields would be DateField-like segmented inputs, which needs some design work first.

Today DateField only commits once all segments are filled, and our Granularity doesn’t support month-only/year-only inputs, so we’d likely need a small refactor (required segments, commit criteria, defaults like day=1, etc.). The range variants also need clear semantics (e.g. does a “month range” map to first-day..last-day?).

I’d prefer landing the pickers first, then opening a follow-up issue to design/implement month/year-only field semantics; once that’s in place, wrappers like MonthField/YearField become straightforward.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In `@packages/core/src/MonthPicker/MonthPickerRoot.vue`:
- Around line 222-240: The interactive path in onMonthChange silently no-ops
when multiple.value is true but modelValue.value is a single DateValue; add a
normalization step at the start of the multi-select branch to convert a
non-array modelValue into an array (e.g., if multiple.value && modelValue.value
&& !Array.isArray(modelValue.value) then set modelValue.value =
[modelValue.value.copy() or modelValue.value] ), then proceed with the existing
Array.isArray(modelValue.value) logic (references: onMonthChange, multiple,
modelValue, isSameYearMonth, placeholder, preventDeselect); apply the same
normalization pattern in CalendarRoot.vue to keep behavior consistent.

In `@packages/core/src/MonthRangePicker/MonthRangePickerCellTrigger.vue`:
- Around line 231-257: shiftFocusYear currently bails when the target month cell
is disabled instead of skipping to the next enabled cell; update shiftFocusYear
(the function using candidateMonthValue, parentElement, nextTick,
rootContext.onPlaceholderChange and candidateMonth?.focus()) to behave like
shiftFocus by either: (A) invoking the existing shiftFocus with ±12 months (e.g.
shiftFocus(12) / shiftFocus(-12)) to reuse its recursive skip-over-disabled
logic, or (B) implementing a loop that increments/decrements candidateMonthValue
by one month within the target year and searches parentElement for the next
non-disabled `[data-value='...']` element, then calls
rootContext.onPlaceholderChange and focuses it; ensure you still respect min/max
checks and next/prev page navigation (rootContext.nextPage()/prevPage()) before
calling nextTick.
- Around line 92-105: The check comparing
isSameYearMonth(rootContext.lastPressedDateValue.value, date) is redundant
because rootContext.lastPressedDateValue.value is set to date.copy() just
before; either remove the inner isSameYearMonth(...) condition and always set
rootContext.startValue.value = date.copy() in that branch, or move the
assignment rootContext.lastPressedDateValue.value = date.copy() to after the
conditional so the comparison actually checks the previous press; update the
same pattern in RangeCalendarCellTrigger.vue and YearRangePickerCellTrigger.vue
to keep behavior consistent.

In `@packages/core/src/MonthRangePicker/useRangeMonthPicker.ts`:
- Around line 66-91: In rangeIsMonthDisabled, when props.fixedDate is set and
both props.start and props.end exist but diff > props.maximumMonths, the
function currently falls through and returns false, leaving months
unconstrained; update the branch in rangeIsMonthDisabled (symbols:
props.fixedDate, props.start, props.end, props.maximumMonths, getMonthsBetween,
compareYearMonth) to enforce limits even when diff > maximumMonths by computing
allowed startLimit and endLimit anchored to the fixed endpoint (e.g., derive
monthsLeft = props.maximumMonths.value and set startLimit =
fixedEndpoint.subtract({ months: monthsLeft }) and endLimit =
fixedEndpoint.add({ months: monthsLeft }) or otherwise clamp around the
fixedDate) and return compareYearMonth(date, startLimit) < 0 ||
compareYearMonth(date, endLimit) > 0 so the maximumMonths constraint is applied
instead of returning false.

In `@packages/core/src/YearPicker/YearPickerCellTrigger.vue`:
- Line 193: Change the data-selected attribute to use an empty string when the
year is selected, matching the sibling attributes; specifically update the
expression that uses isSelectedYear in YearPickerCellTrigger.vue (the
:data-selected binding) to return '' when isSelectedYear is truthy and undefined
otherwise so the DOM renders data-selected as an empty string like
data-disabled/data-today/data-focused.

In `@packages/core/src/YearRangePicker/YearRangePickerCellTrigger.vue`:
- Around line 231-258: shiftFocusPage currently returns silently when the target
candidateYear on the new page is disabled, leaving no focused element; update
shiftFocusPage to behave like shiftFocus by locating the nearest non-disabled
year after pagination (or restore previous focus) instead of doing nothing:
after nextTick finds candidateYear, if it has data-disabled then search the
parentElement grid for the nearest sibling/date cell that does not have
data-disabled (e.g., scan incrementally forward and backward from the
candidateYearValue or iterate sibling elements in parentElement) and when an
enabled cell is found call rootContext.onPlaceholderChange(with that cell's
DateValue) and focus it; if no enabled cell exists on the page, restore focus to
the previously focused element (or fall back to a safe tabbable) to avoid
leaving focus lost.
🧹 Nitpick comments (8)
packages/core/src/MonthRangePicker/MonthRangePickerCellTrigger.vue (1)

53-61: Reactive locale tracking via side-effect read — works but is subtle.

Accessing rootContext.locale.value as a standalone statement (lines 54 and 59) is a valid Vue pattern to register the reactive dependency so the computed recomputes when locale changes. A brief inline comment like // track locale for reactivity would help future readers understand why a seemingly unused expression exists.

packages/core/src/YearPicker/YearPickerCellTrigger.vue (1)

100-104: Arrow up/down hardcodes a 4-column grid assumption.

The ±4 shift assumes the year grid always renders in 4 columns. If yearsPerPage changes (e.g., to 9 with a 3×3 layout), vertical navigation will land on unexpected cells. Consider deriving the column count from the grid or making it configurable.

This likely mirrors the existing Calendar component pattern, so it may be acceptable for now.

packages/core/src/YearPicker/useYearPicker.ts (1)

65-66: useDateFormatter receives a raw string, not the reactive ref.

useDateFormatter(props.locale.value) initializes the formatter with the current locale string. The watcher on line 173 keeps it in sync via formatter.setLocale(...), and the headingValue computed on line 179 also guards against drift. This works but is somewhat fragile — if any new computed is added that reads formatter without the locale guard, it could use a stale locale.

This mirrors the existing pattern in the codebase, so just flagging for awareness.

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

305-343: Consider adding edge-case tests for min/max boundaries and RTL navigation.

The current keyboard navigation tests cover the happy path. Tests for arrow navigation near minValue/maxValue boundaries (where navigation should stop) and RTL direction (where arrow left/right swap) would strengthen confidence.

packages/core/src/YearRangePicker/YearRangePickerRoot.vue (1)

250-270: Model-to-internal sync watcher could miss edge cases with null transitions.

When modelValue transitions from a valid range to null (line 255: || !_modelValue), both startValue and endValue are set to undefined (via next.start?.copy?.() and next.end?.copy?.() where next is { start: undefined, end: undefined }). This works correctly.

However, the condition on line 257 (startValue.value && !isSameYear(next.start, startValue.value)) will call isSameYear with undefined as the first argument when next.start is undefined but startValue.value is still set. The isSameYear implementation (a.year === b.year) would throw on undefined.year.

This is guarded by line 256 (|| !next.start) which returns true and enters the branch before reaching line 257. But the || chain means line 257 is only reached when next.start is truthy. So it's actually safe — just hard to follow.

A comment clarifying the guard chain would help future readers, but functionally correct as-is.

packages/core/src/YearRangePicker/YearRangePickerCellTrigger.vue (1)

92-105: lastPressedDateValue check on line 102 is always true.

lastPressedDateValue is unconditionally set to date.copy() on line 92, so the isSameYear(rootContext.lastPressedDateValue.value, date) guard on line 102 can never be false within the same synchronous call. The conditional is effectively dead code.

If this is intentionally mirroring the RangeCalendar pattern for consistency, a brief comment would help future readers.

packages/core/src/MonthPicker/MonthPickerCellTrigger.vue (1)

43-54: Add a brief comment explaining the bare locale.value access.

Lines 44 and 49 reference rootContext.locale.value as a standalone statement solely to register it as a reactive dependency so the computed re-evaluates when the locale changes. Without context, this looks like dead code and could be removed by a future contributor or linter.

📝 Suggested inline comment
 const shortMonthValue = computed(() => {
-  rootContext.locale.value
+  rootContext.locale.value // track locale for reactivity — formatter is not inherently reactive
   return rootContext.formatter.custom(toDate(props.month), { month: 'short' })
 })
 
 const labelText = computed(() => {
-  rootContext.locale.value
+  rootContext.locale.value // track locale for reactivity — formatter is not inherently reactive
   return rootContext.formatter.custom(toDate(props.month), {
packages/core/src/MonthPicker/MonthPicker.test.ts (1)

353-386: Consider adding a test for preventDeselect and non-array initial value in multi-select mode.

The multi-select tests cover adding and removing selections, but a few edge cases aren't exercised:

  1. preventDeselect: true with multiple: true — verifying that clicking a selected month in multi-select mode doesn't deselect it.
  2. Passing a single DateValue (non-array) as modelValue with multiple: true — this would surface the silent fallthrough in onMonthChange flagged in MonthPickerRoot.vue.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/core/src/RangeCalendar/RangeCalendarRoot.vue`:
- Line 157: The default for fixedWeeks was unintentionally flipped to true in
RangeCalendarRoot (property fixedWeeks), which breaks existing consumers; either
revert the default to false on the fixedWeeks prop in RangeCalendarRoot to
preserve previous behavior or, if keeping true is intentional, add a clear
breaking-change entry to the release notes/changelog and update documentation
for RangeCalendar to state the new default and visual impact, plus add a
unit/integration test covering both fixedWeeks=true and fixedWeeks=false to
prevent regressions.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/core/src/RangeCalendar/useRangeCalendar.ts (1)

93-97: ⚠️ Potential issue | 🟠 Major

Off-by-one: rangeIsDateDisabled allows too many days compared to highlightedRange.

The disabled-date window uses maximumDays directly (lines 94-95), while highlightedRange uses maximumDays - 1 (lines 131, 135). Since isBetween is exclusive on both bounds, this creates an inconsistency: a range of N days should allow dates strictly between N-day offsets, not 2N-1 days. Both functions should use maximumDays - 1 for consistency.

Proposed fix
       if (props.start.value) {
-        const maxDate = props.start.value.add({ days: props.maximumDays.value })
-        const minDate = props.start.value.subtract({ days: props.maximumDays.value })
+        const maxDate = props.start.value.add({ days: props.maximumDays.value - 1 })
+        const minDate = props.start.value.subtract({ days: props.maximumDays.value - 1 })
         return !isBetween(date, minDate, maxDate)
       }
🤖 Fix all issues with AI agents
In `@packages/core/src/MonthRangePicker/MonthRangePickerRoot.vue`:
- Line 180: The bug is that validModelValue (defined as const validModelValue =
ref(normalizeRange(modelValue.value)) in MonthRangePickerRoot.vue) is not
refreshed when modelValue is externally replaced with a complete range, so ESC
reverts to a stale value; to fix, inside the watcher that observes modelValue
(the modelValue watcher that synchronizes startValue/endValue), detect when the
incoming modelValue is a complete normalized DateRange and assign
validModelValue.value = normalizeRange(modelValue.value) (or the normalized
incoming range) after updating startValue/endValue so validModelValue always
reflects the latest external model; ensure this update runs before any
early-return logic in the startValue/endValue watcher and before the ESC handler
uses validModelValue.

In `@packages/core/src/YearPicker/YearPickerCellTrigger.vue`:
- Around line 153-180: shiftFocusPage currently returns without moving focus
when the paged candidate year element is disabled; modify the nextTick callback
so if candidateYear exists but hasAttribute('data-disabled') you fall back to
the per-cell skipping logic instead of doing nothing: call the same shiftFocus
helper used for per-cell navigation (referencing shiftFocus and
rootContext.onPlaceholderChange and candidateYearValue) to start from the
candidateYearValue and move in the same direction (use direction > 0 ? 1 : -1)
so the next non-disabled year receives the placeholder and focus; keep the
existing behavior when candidateYear is enabled.
🧹 Nitpick comments (9)
packages/core/src/MonthRangePicker/MonthRangePickerRoot.vue (1)

53-61: Reactive dependency on rootContext.locale.value is accessed as a bare statement — clarify intent.

Lines 54 and 59 reference rootContext.locale.value as standalone expressions purely to register a reactive dependency so the computed re-evaluates when locale changes. This works but is non-obvious. A brief comment would help future readers understand the intent.

Suggested clarification
 const shortMonthValue = computed(() => {
-  rootContext.locale.value
+  rootContext.locale.value // track locale for reactivity
   return rootContext.formatter.custom(toDate(props.month), { month: 'short' })
 })
packages/core/src/MonthRangePicker/MonthRangePickerCellTrigger.vue (1)

53-61: Bare rootContext.locale.value statements for reactive tracking — same note as in Root.

These standalone property accesses exist solely to register a reactive dependency. Already noted in the Root file review; consider adding a clarifying comment here as well.

packages/core/src/YearRangePicker/YearRangePickerRoot.vue (1)

56-99: Minor: JSDoc uses "checked state" terminology instead of "selected/value".

Line 61 says "The controlled checked state of the calendar" — this is a picker, not a checkbox. Consider updating to something like "The controlled selected range" for clarity.

Proposed fix
-  /** The controlled checked state of the calendar. Can be bound as `v-model`. */
+  /** The controlled selected date range. Can be bound as `v-model`. */
   modelValue?: DateRange | null
packages/core/src/YearRangePicker/YearRangePickerCellTrigger.vue (1)

53-61: Side-effect locale access for reactivity tracking could use a brief comment.

Lines 54 and 59 access rootContext.locale.value purely to register a reactive dependency so the computed re-evaluates when locale changes. This is a known Vue pattern but can confuse future readers. A one-line comment would help.

Proposed clarification
 const yearValue = computed(() => {
+  // Access locale to register reactive dependency for re-computation on locale change
   rootContext.locale.value
   return rootContext.formatter.fullYear(toDate(props.year))
 })
packages/core/src/YearRangePicker/YearRangePicker.test.ts (2)

42-45: Axe accessibility test is a good baseline check.

Note that mount (from @vue/test-utils) is used only here while the rest of the file uses render (from @testing-library/vue). Both work, but for consistency you could use render here too and pass container.firstChild to axe. Not a blocker.


338-376: Keyboard navigation tests cover arrow keys and page navigation.

Consider adding a test case for keyboard navigation at grid boundaries (e.g., arrow right from the last year on a page triggering pagination) to ensure the shiftFocus pagination path is exercised.

packages/core/src/YearPicker/YearPickerCellTrigger.vue (2)

43-53: Consider adding a brief comment explaining the reactivity tracking pattern.

The standalone rootContext.locale.value access on lines 44 and 49 is a known Vue trick to register a reactive dependency so the computed re-evaluates when locale changes. This is non-obvious to contributors unfamiliar with the pattern; a one-line comment (e.g., // track locale for reactivity) would help.


100-104: Hardcoded column count of 4 for vertical arrow-key navigation.

shiftFocus(props.year, ±4) mirrors the hardcoded column count in createYearGrid() (line 217: chunk(years, 4)). While currently consistent, this tight coupling creates maintenance risk: any future change to grid layout (e.g., deriving columns from yearsPerPage) would require updating navigation in multiple places. Consider extracting the column count as a reactive value in the root context (similar to how Calendar uses indexIncrementation).

packages/core/src/RangeCalendar/RangeCalendarRoot.vue (1)

313-328: Sync guards look correct; minor defensive-chaining nit.

The isStartSynced/isEndSynced logic correctly avoids unnecessary updates. However, .copy?.() (optional chaining on copy) on lines 320 and 327 is unusual — DateValue always has a copy method when the object exists. The ?. before copy already guards the nullish case, so the second ?. on the method itself is redundant.

Proposed simplification
-    startValue.value = next.start?.copy?.()
+    startValue.value = next.start?.copy()
   }
   ...
-    endValue.value = next.end?.copy?.()
+    endValue.value = next.end?.copy()

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/core/src/YearPicker/YearPickerCellTrigger.vue`:
- Around line 153-184: shiftFocusPage currently does nothing when candidateYear
is null after the page change; add a fallback that mirrors the disabled-cell
handling: inside the nextTick callback, after checking if candidateYear exists
and is not disabled, if candidateYear is null (or exists but is disabled) call
shiftFocus(candidateYearValue, direction > 0 ? 1 : -1, 1) and ensure
rootContext.onPlaceholderChange(candidateYearValue) is invoked when appropriate
so the placeholder and focus move to the nearest enabled year; update the
nextTick block in function shiftFocusPage to reference candidateYear,
candidateYearValue, parentElement, rootContext.onPlaceholderChange and
shiftFocus accordingly.
🧹 Nitpick comments (6)
packages/core/src/MonthRangePicker/MonthRangePickerRoot.vue (1)

309-314: Consider preventing event propagation on ESC revert.

When ESC is pressed during an in-progress range edit, the handler reverts startValue/endValue but does not call ev.preventDefault() or ev.stopPropagation(). If this picker is nested inside a dialog or popover that also listens for ESC (e.g., to close itself), the ESC keypress will bubble up and potentially close the container before the user sees the reverted range.

Suggested fix
 useEventListener(parentElement, 'keydown', (ev) => {
   if (ev.key === kbd.ESCAPE && isEditing.value) {
+    ev.preventDefault()
+    ev.stopPropagation()
     startValue.value = validModelValue.value.start?.copy()
     endValue.value = validModelValue.value.end?.copy()
   }
 })
packages/core/src/MonthRangePicker/MonthRangePicker.test.ts (3)

256-297: Unused picker destructuring in fixedDate tests.

Both fixedDate test cases destructure picker from setup() but never use it — all assertions go through getByTestId. This is a minor nit.

Suggested cleanup
-    const { getByTestId, picker, user } = setup({
+    const { getByTestId, user } = setup({

275-297: fixedDate: 'end' test — verify the intended semantics are documented.

After clicking May (month 5) with fixedDate: 'end' and defaultValue: {start: Jan, end: Mar}, the test expects the end to become May (not the original March). Then clicking Feb expects end to remain May. This implies "fixedDate" anchors the most recently completed end, not the initial end. If this is intentional, it would be helpful to add a brief comment in the test or in the prop's JSDoc clarifying this nuance, since the name fixedDate: 'end' could be misread as "the end value never changes."


299-316: Non-contiguous range test could be more thorough.

The test verifies that month-4 (April) has data-selected when March is unavailable and the range spans Jan→May, confirming the range isn't broken by the unavailable month. Consider also asserting that month-3 (March) has data-unavailable to explicitly verify the unavailable matcher is applied, and that the overall selected count is as expected.

packages/core/src/YearPicker/YearPickerCellTrigger.vue (2)

43-46: Consider adding a brief comment explaining the bare locale.value access.

Lines 44 and 49 access rootContext.locale.value as standalone statements purely to register a reactive dependency so the computed re-evaluates when locale changes. This is a valid Vue pattern but can confuse future readers.

💡 Suggested comment
 const yearValue = computed(() => {
+  // Track locale changes to re-compute formatted value
   rootContext.locale.value
   return rootContext.formatter.fullYear(toDate(props.year))
 })

Also applies to: 48-53


100-104: Keyboard navigation hardcodes 4-column grid assumption.

Lines 101-104 hardcode -4 and 4 for up/down navigation, which assumes the year grid always has 4 columns. This assumption is baked into createYearGrid() in packages/core/src/date/calendar.ts (line 217: chunk(years, 4)). If this grid layout ever changes, keyboard navigation will silently break. Consider deriving the column count dynamically from rootContext.grid.rows[0]?.length to keep navigation in sync with grid layout.

@onmax
Copy link
Collaborator Author

onmax commented Feb 23, 2026

Hi @zernonia, quick status ping on this PR.

I just pushed the latest fixes (3c543e2), addressed the remaining CodeRabbit findings, and all current checks are passing.

When you have time, could you please share the status and let me know if you can review this one? Thank you!

# Conflicts:
#	docs/scripts/autogen.ts
#	packages/core/src/RangeCalendar/useRangeCalendar.ts
@zernonia
Copy link
Member

Thanks for the huge PR @onmax ! I will try to get to this as soon as I can 😁

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.

[Feature]: MonthPicker and YearPicker

3 participants