feat(wds): date range calendar, date range picker 컴포넌트 추가#524
Open
feat(wds): date range calendar, date range picker 컴포넌트 추가#524
Conversation
- DateRangeCalendar: 멀티 캘린더 지원, 통합 containerRef 기반 cross-panel 키보드 네비게이션, day/month/year view range 선택, hover preview, 반응형 calendars prop (xs/sm/md/lg/xl) - DateRangePicker: Popper 기반 입력 래퍼, 섹션별 키보드 조작 (useDateRangeField), start↔end 섹션 간 ArrowLeft/Right 이동, paste 지원 - PickerActionArea 통합: mode 'single' | 'range' 지원, now variant + range 시 dev 경고 - 공통 로직 추출: getIncrementedSectionValue, getBoundSectionValue, processCharacterInput - useMedia hook을 hooks/internal/use-media.ts로 분리 (modal에서 추출, 재사용) - splitResponsiveProps를 utils/internal/responsive-props.ts에 추가 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughDateRangeCalendar 및 DateRangePicker 컴포넌트가 새로 추가되었고, 관련 타입·컨텍스트·헬퍼·훅·스타일·테스트가 포함되었습니다. 기존 date-picker 훅/헬퍼와 picker-action-area, time-picker, text-field 스타일, 유틸 훅들은 DateRangeType을 수용하도록 확장·조정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant DateRangePicker
participant TextField
participant DateRangeCalendar
participant ActionArea
User->>DateRangePicker: Click calendar icon
DateRangePicker->>TextField: focus/select input
DateRangePicker->>DateRangeCalendar: open popper
User->>DateRangeCalendar: Click start date
DateRangeCalendar->>DateRangeCalendar: set activePosition='end'
DateRangeCalendar->>DateRangePicker: emit interim rangeValue
User->>DateRangeCalendar: Click end date
DateRangeCalendar->>DateRangeCalendar: validate & order dates
DateRangeCalendar->>DateRangePicker: onChangeComplete(DateRangeType)
DateRangePicker->>TextField: update displayed input
DateRangePicker->>ActionArea: notify action-area
ActionArea->>User: close picker / finalize
sequenceDiagram
actor User
participant TextField as "TextField (useDateRangeField)"
participant InputProcessor as processCharacterInput
participant DateParser as parseFromFormat
participant DateRangePicker
User->>TextField: Type character in start section
TextField->>InputProcessor: processCharacterInput(key,...)
InputProcessor->>TextField: return {newInput,isFinished,newSectionRef}
alt section finished
TextField->>TextField: move focus to next section
end
User->>TextField: Complete both sections
TextField->>DateParser: parseFromFormat(fullInput)
DateParser->>TextField: parsed DateRangeType
TextField->>DateRangePicker: trigger onChange / onChangeComplete
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Contributor
size-limit report 📦
|
Contributor
🚀 Preview
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/wds/src/components/text-field/style.ts (1)
160-167:⚠️ Potential issue | 🟡 Minor
@supports not selector(:has(*))폴백 블록에date-range-picker-field셀렉터 누락
:has()지원 블록(lines 112-117)에는date-range-picker-field가 추가되었으나, 폴백 블록에는 누락되어 있습니다. 일관성을 위해 여기에도 동일한 셀렉터를 추가해야 합니다.🔧 수정 제안
`@supports` not selector(:has(*)) { &:where(:focus-within), &:where( :has( input[data-role='date-picker-field'][aria-expanded='true'] ), - :has(input[data-role='time-picker-field'][aria-expanded='true']) + :has(input[data-role='time-picker-field'][aria-expanded='true']), + :has( + input[data-role='date-range-picker-field'][aria-expanded='true'] + ) ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds/src/components/text-field/style.ts` around lines 160 - 167, The fallback `@supports` block using "not selector(:has(*))" is missing the date-range-picker-field selector; update the selector group inside that block (the &:where(...) / :has(...) entries used for focus/expanded handling) to include the same :has(input[data-role='date-range-picker-field'][aria-expanded='true']) clause you added in the supported :has() block so the behavior is consistent with the :has() support branch.
🧹 Nitpick comments (4)
packages/wds/src/components/date-range-calendar/index.test.tsx (1)
70-88: 날짜 스왑 검증 로직 개선 제안
getDate()만 비교하면 월이 다른 경우(예: 1월 31일 vs 2월 1일)에 오탐이 발생할 수 있습니다. 전체 타임스탬프 비교를 고려해 보세요.♻️ 더 강건한 검증 방법
const [start, end] = onChangeComplete.mock.calls[0]![0] as [Date, Date]; - expect(start.getDate()).toBeLessThanOrEqual(end.getDate()); + expect(start.getTime()).toBeLessThanOrEqual(end.getTime());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds/src/components/date-range-calendar/index.test.tsx` around lines 70 - 88, The test in the DateRangeCalendar spec uses start.getDate() and end.getDate() which can give false positives across months; update the assertion in the test case that clicks dates (where onChangeComplete is inspected via onChangeComplete.mock.calls[0]![0]) to compare full timestamps instead (e.g., use start.getTime() <= end.getTime() or Date.valueOf()) so the swap behavior is validated correctly across month/year boundaries; locate the test that references DateRangeCalendar and replace the getDate() comparison with a getTime()/valueOf() comparison.packages/wds/src/hooks/internal/use-media.ts (1)
14-21:useMemo의존성 배열 패턴에 대한 참고 사항
Object.values(queries)를 의존성 배열로 사용하는 것은 비표준적인 패턴입니다.queries가 이미Array<string>이므로queries자체를 스프레드하여 사용하는 것이 더 명확할 수 있습니다. 현재 구현도 동작하지만, eslint-disable 주석이 의도적인 선택임을 나타냅니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds/src/hooks/internal/use-media.ts` around lines 14 - 21, The useMemo dependency currently uses Object.values(queries) which is nonstandard for an Array<string>; update the dependency to use the queries array spread (e.g., [...queries]) or queries itself so React can correctly track changes, remove the // eslint-disable-next-line react-hooks/exhaustive-deps comment, and ensure the memo that computes mediaQueryLists (in the useMedia hook) depends on that normalized dependency list so window.matchMedia is re-run when any query changes.packages/wds/src/components/date-picker/helpers.ts (1)
840-843: 정적 분석 경고 검토: ReDoS 위험은 낮음정적 분석 도구가 변수로 생성된 정규식에 대해 ReDoS 경고를 발생시켰습니다. 그러나 이 경우:
am,pm값은getMeridiem(locale)에서 가져오며, 이는Intl.DateTimeFormat의 짧은 문자열입니다 (예: "AM", "PM", "오전")lowerKey는 키보드 입력의 단일 문자입니다따라서 실제 ReDoS 위험은 매우 낮습니다. 다만 방어적 코딩을 위해
am,pm값을 이스케이프하는 것을 고려해볼 수 있습니다.🛡️ (선택) 방어적 정규식 이스케이프
+const escapeRegex = (str: string) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return new RegExp( - `^${lowerKey === 'a' ? am : lowerKey === 'p' ? pm : '$^'}`, + `^${lowerKey === 'a' ? escapeRegex(am) : lowerKey === 'p' ? escapeRegex(pm) : '$^'}`, ).test(v);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds/src/components/date-picker/helpers.ts` around lines 840 - 843, The dynamic RegExp built using the variables am and pm can trigger static ReDoS warnings; defensively escape those values before constructing the RegExp to ensure any special regex metacharacters in the locale meridiem strings are treated literally. Update the code that builds the regex (the expression using lowerKey, am, pm in helpers.ts) to run am and pm through an escape function (e.g., escapeRegExp or a small replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&')) and then construct the RegExp from the escaped strings so lowerKey logic remains the same but the regex is safe.packages/wds/src/components/date-range-calendar/style.ts (1)
81-107: 반복되는addOpacity호출을 변수로 추출하면 DRY 원칙에 부합합니다.
addOpacity(theme.semantic.primary.normal, theme.opacity[8])가 3번 반복되고 있습니다. 로컬 변수로 추출하면 유지보수성이 향상됩니다.♻️ 제안된 리팩토링
const rangeCellBaseStyle = (theme: Theme) => css` position: relative; display: flex; align-items: center; justify-content: center; overflow: visible; + --range-band-bg: ${addOpacity( + theme.semantic.primary.normal, + theme.opacity[8], + )}; + &::before { content: ''; position: absolute; top: 2px; bottom: 2px; left: 0; right: 0; background-color: transparent; pointer-events: none; } &[data-in-range='true']::before { - background-color: ${addOpacity( - theme.semantic.primary.normal, - theme.opacity[8], - )}; + background-color: var(--range-band-bg); } &[data-range-start='true']::before { - background-color: ${addOpacity( - theme.semantic.primary.normal, - theme.opacity[8], - )}; + background-color: var(--range-band-bg); left: 50%; } &[data-range-end='true']::before { - background-color: ${addOpacity( - theme.semantic.primary.normal, - theme.opacity[8], - )}; + background-color: var(--range-band-bg); right: 50%; } &[data-range-start='true'][data-range-end='true']::before { display: none; } `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds/src/components/date-range-calendar/style.ts` around lines 81 - 107, Extract the repeated computed color into a local constant and reuse it in the three selector blocks instead of calling addOpacity(...) each time: compute const rangeBg = addOpacity(theme.semantic.primary.normal, theme.opacity[8]) near the top of the styled block and replace the three occurrences inside &[data-in-range='true']::before, &[data-range-start='true']::before, and &[data-range-end='true']::before with rangeBg (leave the special-case &[data-range-start='true'][data-range-end='true']::before unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wds/src/components/date-range-calendar/index.tsx`:
- Around line 87-90: The DateRangeCalendar component is exported using
forwardRef with DefaultComponentPropsInternal and does not follow the repo's
polymorphic Box contract; update the component to use the
PolymorphicPropsInternal pattern and wrap the root element with the Box
primitive from wds-engine. Specifically, change the generic/props type from
DefaultComponentPropsInternal<DateRangeCalendarProps, 'div'> to
PolymorphicPropsInternal<DateRangeCalendarProps, 'div'> (or the repo
equivalent), update the forwardRef signature to accept the polymorphic props,
and ensure the rendered root uses <Box ...> (preserving existing props like ref,
className, aria attributes) so the public DateRangeCalendar surface matches
other components' polymorphic API.
- Around line 917-922: The current ScrollArea uses radio semantics which breaks
in range mode because start and end months can both be active; change the ARIA
to non-radio semantics when the component is in range mode: set the ScrollArea
(where rangePanelWrapperStyle is applied / the current role="radiogroup") to
role="group" (or no radiogroup) when mode === "range", and update the selectable
month/year items (currently using role="radio" and aria-checked) to use
role="button" with aria-pressed or role="checkbox" with aria-checked so two
endpoints can be true simultaneously; locate the ScrollArea and the month/year
item render logic in this file and conditionally switch roles/attributes based
on the range mode.
- Around line 857-909: handleKeyDown currently computes newMonth and immediately
sets focusedIdx and focuses that month even if monthRange[newMonth] is disabled,
which breaks keyboard navigation; change the logic after computing newMonth to
clamp it to the nearest enabled month in monthRange (search backward/forward
from newMonth for the nearest item where v.disabled is false), then use that
clamped month index when calling setFocusedIdx, setHoveredDate (via
dateTypeToDateObject/dayjsTimezone/defaultSelectedDate/timezone) and when
building mv for focusRangeDate; reference handleKeyDown, monthRange,
setFocusedIdx, setHoveredDate, focusRangeDate, defaultSelectedDate,
containerRef, and timezone.
In `@packages/wds/src/components/date-range-picker/index.test.tsx`:
- Around line 1-8: Tests are mixing Vitest globals with explicit imports: remove
the explicit import of describe, it, expect, vi, beforeEach from 'vitest' so the
test uses Vitest globals (describe, it, expect, vi, beforeEach, afterEach)
consistently; leave imports from '@testing-library/react' (cleanup, fireEvent,
render, screen, waitFor) intact and ensure any uses of vi and lifecycle helpers
rely on the global symbols rather than the removed named import.
- Around line 17-364: Add a vitest-axe accessibility test to the DateRangePicker
suite: import and use axe from 'vitest-axe' and add a test (e.g., it('should
have no accessibility violations')) that renders <DateRangePicker
{...defaultProps} data-testid="range-picker" />, grabs the rendered container
(or screen.container), runs await axe(container) and asserts the result with
expect(...).toHaveNoViolations(); place this new test inside the existing
describe block so it runs with the other tests.
In `@packages/wds/src/components/date-range-picker/index.tsx`:
- Around line 34-37: The public component signature should follow the repo
polymorphic pattern: replace DefaultComponentPropsInternal<DateRangePickerProps,
'input'> with PolymorphicPropsInternal<DateRangePickerProps, 'input'> for
DateRangePicker, update imports to bring in PolymorphicPropsInternal and Box
from wds-engine, and change the implementation to render within a Box that
forwards the polymorphic ref and supports the as prop; ensure the forwarded ref
type is generic per the polymorphic pattern and update any related export/type
aliases (e.g., DateRangePickerProps usage) so the component surface matches
other components in the repo.
- Line 132: 현재 invalid 계산이 onChange 존재 여부로 uncontrolled/controlled를 판별하고
있어(defaultValue + onChange 조합의 내부 state 사용 케이스에서 aria-invalid가 무시됨), 계산 식에서
onChange 대신 value prop 유무로 판별하도록 수정하세요: locate the expression that sets the
invalid constant (const invalid = originInvalid || (!onChange &&
isInvalidDateRange(value));) and change the uncontrolled check to use value ===
undefined (or similar) so isInvalidDateRange(value) is evaluated for
uncontrolled cases; additionally add a regression test covering the defaultValue
+ onChange scenario to ensure aria-invalid is computed correctly when internal
state is used.
---
Outside diff comments:
In `@packages/wds/src/components/text-field/style.ts`:
- Around line 160-167: The fallback `@supports` block using "not
selector(:has(*))" is missing the date-range-picker-field selector; update the
selector group inside that block (the &:where(...) / :has(...) entries used for
focus/expanded handling) to include the same
:has(input[data-role='date-range-picker-field'][aria-expanded='true']) clause
you added in the supported :has() block so the behavior is consistent with the
:has() support branch.
---
Nitpick comments:
In `@packages/wds/src/components/date-picker/helpers.ts`:
- Around line 840-843: The dynamic RegExp built using the variables am and pm
can trigger static ReDoS warnings; defensively escape those values before
constructing the RegExp to ensure any special regex metacharacters in the locale
meridiem strings are treated literally. Update the code that builds the regex
(the expression using lowerKey, am, pm in helpers.ts) to run am and pm through
an escape function (e.g., escapeRegExp or a small
replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&')) and then construct the RegExp from
the escaped strings so lowerKey logic remains the same but the regex is safe.
In `@packages/wds/src/components/date-range-calendar/index.test.tsx`:
- Around line 70-88: The test in the DateRangeCalendar spec uses start.getDate()
and end.getDate() which can give false positives across months; update the
assertion in the test case that clicks dates (where onChangeComplete is
inspected via onChangeComplete.mock.calls[0]![0]) to compare full timestamps
instead (e.g., use start.getTime() <= end.getTime() or Date.valueOf()) so the
swap behavior is validated correctly across month/year boundaries; locate the
test that references DateRangeCalendar and replace the getDate() comparison with
a getTime()/valueOf() comparison.
In `@packages/wds/src/components/date-range-calendar/style.ts`:
- Around line 81-107: Extract the repeated computed color into a local constant
and reuse it in the three selector blocks instead of calling addOpacity(...)
each time: compute const rangeBg = addOpacity(theme.semantic.primary.normal,
theme.opacity[8]) near the top of the styled block and replace the three
occurrences inside &[data-in-range='true']::before,
&[data-range-start='true']::before, and &[data-range-end='true']::before with
rangeBg (leave the special-case
&[data-range-start='true'][data-range-end='true']::before unchanged).
In `@packages/wds/src/hooks/internal/use-media.ts`:
- Around line 14-21: The useMemo dependency currently uses
Object.values(queries) which is nonstandard for an Array<string>; update the
dependency to use the queries array spread (e.g., [...queries]) or queries
itself so React can correctly track changes, remove the //
eslint-disable-next-line react-hooks/exhaustive-deps comment, and ensure the
memo that computes mediaQueryLists (in the useMedia hook) depends on that
normalized dependency list so window.matchMedia is re-run when any query
changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b465ac8d-97c6-475b-ac03-5df9a18bebd9
📒 Files selected for processing (27)
docs/data/components/selection-and-input/date-picker/web.mdxpackages/wds-mcp/src/helpers/index.tspackages/wds/src/components/date-picker/helpers.tspackages/wds/src/components/date-picker/hooks.tspackages/wds/src/components/date-picker/index.tsxpackages/wds/src/components/date-range-calendar/constants.tspackages/wds/src/components/date-range-calendar/contexts.tspackages/wds/src/components/date-range-calendar/helpers.tspackages/wds/src/components/date-range-calendar/hooks.tspackages/wds/src/components/date-range-calendar/index.test.tsxpackages/wds/src/components/date-range-calendar/index.tsxpackages/wds/src/components/date-range-calendar/style.tspackages/wds/src/components/date-range-calendar/types.tspackages/wds/src/components/date-range-picker/helpers.tspackages/wds/src/components/date-range-picker/hooks.tspackages/wds/src/components/date-range-picker/index.test.tsxpackages/wds/src/components/date-range-picker/index.tsxpackages/wds/src/components/date-range-picker/style.tspackages/wds/src/components/date-range-picker/types.tspackages/wds/src/components/index.tspackages/wds/src/components/modal/hooks.tspackages/wds/src/components/picker-action-area/contexts.tspackages/wds/src/components/picker-action-area/index.tsxpackages/wds/src/components/text-field/style.tspackages/wds/src/components/time-picker/index.tsxpackages/wds/src/hooks/internal/use-media.tspackages/wds/src/utils/internal/responsive-props.ts
Comment on lines
+87
to
+90
| const DateRangeCalendar = forwardRef< | ||
| HTMLDivElement, | ||
| DefaultComponentPropsInternal<DateRangeCalendarProps, 'div'> | ||
| >( |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
공개 컴포넌트가 repo의 polymorphic/Box contract를 따르지 않습니다.
새 DateRangeCalendar는 공개 surface인데도 PolymorphicPropsInternal 기반이 아니고 루트도 Box 패턴을 쓰지 않습니다. 이 상태로 머지되면 date 계열 컴포넌트 API 일관성이 깨집니다.
As per coding guidelines, **/components/*/index.tsx: Implement components using the polymorphic component pattern with PolymorphicPropsInternal, wrapping content in Box from wds-engine
Also applies to: 164-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/wds/src/components/date-range-calendar/index.tsx` around lines 87 -
90, The DateRangeCalendar component is exported using forwardRef with
DefaultComponentPropsInternal and does not follow the repo's polymorphic Box
contract; update the component to use the PolymorphicPropsInternal pattern and
wrap the root element with the Box primitive from wds-engine. Specifically,
change the generic/props type from
DefaultComponentPropsInternal<DateRangeCalendarProps, 'div'> to
PolymorphicPropsInternal<DateRangeCalendarProps, 'div'> (or the repo
equivalent), update the forwardRef signature to accept the polymorphic props,
and ensure the rendered root uses <Box ...> (preserving existing props like ref,
className, aria attributes) so the public DateRangeCalendar surface matches
other components' polymorphic API.
Comment on lines
+17
to
+364
| describe('when given date range picker component', () => { | ||
| const defaultProps = { | ||
| defaultValue: [ | ||
| new Date('2025-01-15T00:00:00'), | ||
| new Date('2025-02-20T00:00:00'), | ||
| ] as [Date, Date], | ||
| onChange: vi.fn(), | ||
| locale: 'en-US', | ||
| format: 'YYYY.MM.DD', | ||
| }; | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| cleanup(); | ||
| }); | ||
|
|
||
| // --- Rendering --- | ||
|
|
||
| it('should render with default range value', () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| expect(input).toHaveValue('2025.01.15 - 2025.02.20'); | ||
| }); | ||
|
|
||
| it('should render empty when no value', () => { | ||
| render( | ||
| <DateRangePicker | ||
| locale="en-US" | ||
| format="YYYY.MM.DD" | ||
| data-testid="range-picker" | ||
| />, | ||
| ); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| expect(input).toHaveValue(''); | ||
| }); | ||
|
|
||
| it('should show format placeholder on focus when empty', () => { | ||
| render( | ||
| <DateRangePicker | ||
| locale="en-US" | ||
| format="YYYY.MM.DD" | ||
| data-testid="range-picker" | ||
| />, | ||
| ); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| expect(input).toHaveValue('YYYY.MM.DD - YYYY.MM.DD'); | ||
| }); | ||
|
|
||
| // --- Keyboard section navigation --- | ||
|
|
||
| it('should navigate between start date sections with arrow keys', async () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| await waitFor(() => { | ||
| expect(input.selectionStart).toBe(0); | ||
| expect(input.selectionEnd).toBe(4); | ||
| }); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(input.selectionStart).toBe(5); | ||
| expect(input.selectionEnd).toBe(7); | ||
| }); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(input.selectionStart).toBe(8); | ||
| expect(input.selectionEnd).toBe(10); | ||
| }); | ||
| }); | ||
|
|
||
| it('should navigate from start to end date sections', async () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
|
|
||
| // "2025.01.15 - 2025.02.20" → end YYYY starts at 13 | ||
| await waitFor(() => { | ||
| expect(input.selectionStart).toBe(13); | ||
| expect(input.selectionEnd).toBe(17); | ||
| }); | ||
| }); | ||
|
|
||
| it('should navigate from end to start date sections', async () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| // Navigate to end date YYYY | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
|
|
||
| // ArrowLeft from end YYYY → start DD | ||
| fireEvent.keyDown(input, { key: 'ArrowLeft' }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(input.selectionStart).toBe(8); | ||
| expect(input.selectionEnd).toBe(10); | ||
| }); | ||
| }); | ||
|
|
||
| // --- Value modification (start) --- | ||
|
|
||
| it('should modify start date year with ArrowUp/Down', () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowDown' }); | ||
|
|
||
| expect(input).toHaveValue('2024.01.15 - 2025.02.20'); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowUp' }); | ||
|
|
||
| expect(input).toHaveValue('2025.01.15 - 2025.02.20'); | ||
| }); | ||
|
|
||
| it('should modify start date year with Home/End', () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'End' }); | ||
|
|
||
| expect(input).toHaveValue('2100.01.15 - 2025.02.20'); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'Home' }); | ||
|
|
||
| expect(input).toHaveValue('1900.01.15 - 2025.02.20'); | ||
| }); | ||
|
|
||
| // --- Value modification (end) --- | ||
|
|
||
| it('should modify end date year with ArrowUp/Down', async () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| // Navigate to end YYYY | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(input.selectionStart).toBe(13); | ||
| }); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowDown' }); | ||
|
|
||
| expect(input).toHaveValue('2025.01.15 - 2024.02.20'); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowUp' }); | ||
|
|
||
| expect(input).toHaveValue('2025.01.15 - 2025.02.20'); | ||
| }); | ||
|
|
||
| // --- Backspace --- | ||
|
|
||
| it('should clear start section with Backspace', () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'Backspace' }); | ||
|
|
||
| expect(input).toHaveValue('YYYY.01.15 - 2025.02.20'); | ||
| }); | ||
|
|
||
| it('should clear end section with Backspace', async () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| // Navigate to end YYYY | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
| fireEvent.keyDown(input, { key: 'ArrowRight' }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(input.selectionStart).toBe(13); | ||
| }); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'Backspace' }); | ||
|
|
||
| expect(input).toHaveValue('2025.01.15 - YYYY.02.20'); | ||
| }); | ||
|
|
||
| // --- Paste --- | ||
|
|
||
| it('should handle paste for full range', () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| input.setSelectionRange(0, input.value.length); | ||
|
|
||
| fireEvent.paste(input, { | ||
| clipboardData: { | ||
| getData: () => '2024.06.01 - 2024.12.31', | ||
| }, | ||
| }); | ||
|
|
||
| expect(input).toHaveValue('2024.06.01 - 2024.12.31'); | ||
| }); | ||
|
|
||
| // --- Calendar popup --- | ||
|
|
||
| it('should open calendar when calendar icon is clicked', () => { | ||
| render(<DateRangePicker {...defaultProps} data-testid="range-picker" />); | ||
|
|
||
| fireEvent.click(screen.getByLabelText('Toggle date range picker')); | ||
|
|
||
| expect(screen.getByRole('dialog')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| // --- Invalid state --- | ||
|
|
||
| it('should be invalid when start > end in uncontrolled mode', () => { | ||
| render( | ||
| <DateRangePicker | ||
| defaultValue={[ | ||
| new Date('2025-03-01T00:00:00'), | ||
| new Date('2025-01-01T00:00:00'), | ||
| ]} | ||
| locale="en-US" | ||
| format="YYYY.MM.DD" | ||
| data-testid="range-picker" | ||
| />, | ||
| ); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| expect(input).toHaveAttribute('aria-invalid', 'true'); | ||
| }); | ||
|
|
||
| it('should not be invalid when start equals end', () => { | ||
| const sameDate = new Date('2025-01-15T00:00:00'); | ||
|
|
||
| render( | ||
| <DateRangePicker | ||
| defaultValue={[sameDate, sameDate]} | ||
| locale="en-US" | ||
| format="YYYY.MM.DD" | ||
| data-testid="range-picker" | ||
| />, | ||
| ); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| expect(input).not.toHaveAttribute('aria-invalid', 'true'); | ||
| }); | ||
|
|
||
| // --- Disabled / ReadOnly --- | ||
|
|
||
| it('should not modify value when disabled', () => { | ||
| render( | ||
| <DateRangePicker {...defaultProps} disabled data-testid="range-picker" />, | ||
| ); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| expect(input).toBeDisabled(); | ||
| }); | ||
|
|
||
| it('should not modify value when readOnly', () => { | ||
| render( | ||
| <DateRangePicker {...defaultProps} readOnly data-testid="range-picker" />, | ||
| ); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| fireEvent.focus(input); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowUp' }); | ||
|
|
||
| expect(input).toHaveValue('2025.01.15 - 2025.02.20'); | ||
| }); | ||
|
|
||
| // --- Custom format --- | ||
|
|
||
| it('should render with custom format', () => { | ||
| render( | ||
| <DateRangePicker | ||
| {...defaultProps} | ||
| format="DD/MM/YYYY" | ||
| data-testid="range-picker" | ||
| />, | ||
| ); | ||
|
|
||
| const input = screen.getByTestId<HTMLInputElement>('range-picker'); | ||
|
|
||
| expect(input).toHaveValue('15/01/2025 - 20/02/2025'); | ||
| }); | ||
|
|
||
| // --- View prop --- | ||
|
|
||
| it('should pass view prop to calendar', () => { | ||
| render( | ||
| <DateRangePicker | ||
| {...defaultProps} | ||
| view="month" | ||
| data-testid="range-picker" | ||
| />, | ||
| ); | ||
|
|
||
| fireEvent.click(screen.getByLabelText('Toggle date range picker')); | ||
|
|
||
| expect(screen.getByRole('radiogroup')).toBeInTheDocument(); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
index.test.tsx에는 기본 vitest-axe 검증이 하나 필요합니다.
키보드/ARIA 동작을 많이 추가한 PR인데, 이 suite에는 접근성 회귀를 잡아줄 axe assertion이 없습니다. 최소 1개는 넣어두는 게 안전합니다.
As per coding guidelines, **/components/**/*.test.{ts,tsx}: Write unit tests using Vitest with @testing-library/react and vitest-axe for accessibility in index.test.tsx
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/wds/src/components/date-range-picker/index.test.tsx` around lines 17
- 364, Add a vitest-axe accessibility test to the DateRangePicker suite: import
and use axe from 'vitest-axe' and add a test (e.g., it('should have no
accessibility violations')) that renders <DateRangePicker {...defaultProps}
data-testid="range-picker" />, grabs the rendered container (or
screen.container), runs await axe(container) and asserts the result with
expect(...).toHaveNoViolations(); place this new test inside the existing
describe block so it runs with the other tests.
Comment on lines
+34
to
+37
| const DateRangePicker = forwardRef< | ||
| HTMLDivElement, | ||
| DefaultComponentPropsInternal<DateRangePickerProps, 'input'> | ||
| >( |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
공개 컴포넌트 시그니처를 repo의 polymorphic 패턴으로 맞춰주세요.
새 DateRangePicker가 DefaultComponentPropsInternal 기반으로 추가돼서, 다른 wds 컴포넌트와 as/ref surface가 달라집니다. 이 파일은 PolymorphicPropsInternal + Box 패턴으로 맞추는 편이 API 일관성상 안전합니다.
As per coding guidelines, **/components/*/index.tsx: Implement components using the polymorphic component pattern with PolymorphicPropsInternal, wrapping content in Box from wds-engine
Also applies to: 174-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/wds/src/components/date-range-picker/index.tsx` around lines 34 -
37, The public component signature should follow the repo polymorphic pattern:
replace DefaultComponentPropsInternal<DateRangePickerProps, 'input'> with
PolymorphicPropsInternal<DateRangePickerProps, 'input'> for DateRangePicker,
update imports to bring in PolymorphicPropsInternal and Box from wds-engine, and
change the implementation to render within a Box that forwards the polymorphic
ref and supports the as prop; ensure the forwarded ref type is generic per the
polymorphic pattern and update any related export/type aliases (e.g.,
DateRangePickerProps usage) so the component surface matches other components in
the repo.
| disabled, | ||
| }); | ||
|
|
||
| const invalid = originInvalid || (!onChange && isInvalidDateRange(value)); |
Contributor
There was a problem hiding this comment.
onChange 유무로 uncontrolled 여부를 판단하면 invalid 계산이 꺼집니다.
Line 132는 제어 여부를 onChange 존재 여부로 보고 있어서, defaultValue + onChange 조합처럼 내부 state를 쓰는 경우에도 aria-invalid가 계산되지 않습니다. 여기서는 value prop 유무로 controlled/uncontrolled를 나눠야 합니다.
수정 예시
+ const isControlled = originValue !== undefined;
- const invalid = originInvalid || (!onChange && isInvalidDateRange(value));
+ const invalid =
+ originInvalid || (!isControlled && isInvalidDateRange(value));회귀 방지용으로 defaultValue + onChange 케이스 테스트도 같이 추가해두면 좋겠습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/wds/src/components/date-range-picker/index.tsx` at line 132, 현재
invalid 계산이 onChange 존재 여부로 uncontrolled/controlled를 판별하고 있어(defaultValue +
onChange 조합의 내부 state 사용 케이스에서 aria-invalid가 무시됨), 계산 식에서 onChange 대신 value prop
유무로 판별하도록 수정하세요: locate the expression that sets the invalid constant (const
invalid = originInvalid || (!onChange && isInvalidDateRange(value));) and change
the uncontrolled check to use value === undefined (or similar) so
isInvalidDateRange(value) is evaluated for uncontrolled cases; additionally add
a regression test covering the defaultValue + onChange scenario to ensure
aria-invalid is computed correctly when internal state is used.
- month/year panel ARIA: radiogroup/radio → grid/row/gridcell, aria-checked → aria-selected - month view 키보드: 화살표 경계 넘을 때 이전/다음 년도 이동, PageUp/PageDown 년도 단위 이동 - month view disabled 월 포커스 방지: findClosestEnableDate로 enabled 월로 clamp - 스타일 중복 제거: rangeCellBaseStyle, rangeItemBaseStyle 추출 - RangeDayItemProps/RangeDateItemProps → RangeItemProps 통합 - PanelHeaderButton → PanelHeaderLabel (Typography) 변경 - Grid/GridItem → FlexBox + role="row" 구조로 변경 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/wds/src/components/date-calendar/index.tsx (1)
1121-1122: 레이아웃 간격 조정이 적절합니다.
rowGap="2px"로 날짜 행 간 간격을 추가한 것은 시각적 일관성에 도움이 됩니다. 다만,columnGap="0px"는 기본값이 이미 0인 경우 불필요할 수 있습니다.♻️ 선택적 제안: columnGap 제거
columnGap="0px" + // columnGap="0px"는 기본값과 동일하다면 제거 가능 rowGap="2px"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds/src/components/date-calendar/index.tsx` around lines 1121 - 1122, Remove the redundant columnGap="0px" prop from the JSX where the date calendar layout props are set (leave rowGap="2px" in place); locate the element that currently includes columnGap and rowGap (the date calendar/grid component) and delete only the columnGap prop to rely on the default spacing, keeping rowGap for the vertical spacing consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wds/src/components/date-range-calendar/index.tsx`:
- Around line 1333-1337: RangeYearPanel's keyboard handling is missing the
disabled/readOnly guard before calling setHoveredDate; update the handleKeyDown
logic inside RangeYearPanel to mirror RangeMonthPanel by checking the component
props/state for disabled and readOnly and returning early if either is true
before computing yearDate via
dateTypeToDateObject(dayjs(defaultSelectedDate).set('year', clampedYear),
timezone) and calling setHoveredDate; ensure the same conditional gating used in
RangeMonthPanel (the checks around setHoveredDate in its handleKeyDown) is
applied to RangeYearPanel to keep behavior consistent.
---
Nitpick comments:
In `@packages/wds/src/components/date-calendar/index.tsx`:
- Around line 1121-1122: Remove the redundant columnGap="0px" prop from the JSX
where the date calendar layout props are set (leave rowGap="2px" in place);
locate the element that currently includes columnGap and rowGap (the date
calendar/grid component) and delete only the columnGap prop to rely on the
default spacing, keeping rowGap for the vertical spacing consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d22c60d1-f044-4e29-8e99-770e19f8d19b
📒 Files selected for processing (7)
packages/wds/src/components/date-calendar/index.tsxpackages/wds/src/components/date-calendar/style.tspackages/wds/src/components/date-range-calendar/index.test.tsxpackages/wds/src/components/date-range-calendar/index.tsxpackages/wds/src/components/date-range-calendar/style.tspackages/wds/src/components/date-range-calendar/types.tspackages/wds/src/components/date-range-picker/index.test.tsx
💤 Files with no reviewable changes (1)
- packages/wds/src/components/date-calendar/style.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/wds/src/components/date-range-calendar/index.test.tsx
- packages/wds/src/components/date-range-calendar/style.ts
- grid role을 ScrollArea로 이동, aria-multiselectable 추가 - aria-label을 "Select day/month/year range"로 변경 - year panel handleKeyDown에서 disabled/readOnly 가드 추가 - 테스트 셀렉터를 변경된 ARIA 구조에 맞게 업데이트 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/wds/src/components/date-range-calendar/index.test.tsx (1)
250-273: 접근성 테스트 범위 개선 제안현재 접근성 테스트가 개별 요소(
rowgroup,row)에 대해 실행되는데, 전체 컴포넌트에 대해 axe를 실행하면 구조적 접근성 문제도 함께 검증할 수 있습니다.♻️ 전체 컴포넌트 접근성 테스트 예시
it('should pass accessibility tests for day view', async () => { - render(<DateRangeCalendar {...defaultProps} />); + const { container } = render(<DateRangeCalendar {...defaultProps} />); - const rowgroup = screen.getByRole('rowgroup'); - expect(await axe(rowgroup)).toHaveNoViolations(); + expect(await axe(container)).toHaveNoViolations(); }); it('should pass accessibility tests for month view', async () => { - render(<DateRangeCalendar {...defaultProps} view="month" />); + const { container } = render(<DateRangeCalendar {...defaultProps} view="month" />); - const rows = screen.getAllByRole('row'); - for (const row of rows) { - expect(await axe(row)).toHaveNoViolations(); - } + expect(await axe(container)).toHaveNoViolations(); }); it('should pass accessibility tests for year view', async () => { - render(<DateRangeCalendar {...defaultProps} view="year" />); + const { container } = render(<DateRangeCalendar {...defaultProps} view="year" />); - const rows = screen.getAllByRole('row'); - for (const row of rows) { - expect(await axe(row)).toHaveNoViolations(); - } + expect(await axe(container)).toHaveNoViolations(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wds/src/components/date-range-calendar/index.test.tsx` around lines 250 - 273, Replace the per-element axe checks with a single full-component accessibility scan: in the DateRangeCalendar tests ("should pass accessibility tests for day view", "month view", "year view") use the render return value (container) to run axe against the entire rendered component instead of calling axe on individual elements like rowgroup or row; update each test to call axe on the render container (or document body from the render) and assert toHaveNoViolations once per test to validate structural and nested accessibility issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wds/src/components/date-range-calendar/index.tsx`:
- Around line 113-117: The useMemo for breakpoints is ineffective because
Object.values(theme) creates a new array each render; change the dependency to
the stable property instead (e.g., use theme.breakpoint) so memoization works:
update the useMemo call that computes breakpoints to depend on theme.breakpoint
(or another stable identifier) rather than Object.values(theme), and remove the
eslint-disable comment so the hook dependencies are correct; refer to the
breakpoints constant, useMemo call, and theme.breakpoint to locate and update
the code.
---
Nitpick comments:
In `@packages/wds/src/components/date-range-calendar/index.test.tsx`:
- Around line 250-273: Replace the per-element axe checks with a single
full-component accessibility scan: in the DateRangeCalendar tests ("should pass
accessibility tests for day view", "month view", "year view") use the render
return value (container) to run axe against the entire rendered component
instead of calling axe on individual elements like rowgroup or row; update each
test to call axe on the render container (or document body from the render) and
assert toHaveNoViolations once per test to validate structural and nested
accessibility issues.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2bb869ab-37f0-4d7a-8235-85c5c1da9a9d
📒 Files selected for processing (2)
packages/wds/src/components/date-range-calendar/index.test.tsxpackages/wds/src/components/date-range-calendar/index.tsx
Object.values(theme)는 매 렌더마다 새 배열을 생성하여 메모이제이션이 무의미했음. theme.breakpoint는 안정적인 참조이므로 이를 의존성으로 변경. Co-Authored-By: Claude Opus 4.6 (1M context) <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
1 participant
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
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
새로운 기능
문서화
테스트
스타일