refactor(RangePicker): centralize interaction flow - #994
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughRangePicker 与 SinglePicker 改用统一的焦点事件和字段值变更流程，支持弹层关闭、键盘、清空、确认及 ChangesPicker 交互状态重构
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
React Doctor found 2 new issues in 2 files · 2 warnings · score 69 / 100 (Needs work) · 2 fixed · vs 2 warnings
Reviewed by React Doctor for commit |
❌ Deploy failed
📋 Build log (last lines)🤖 Powered by surge-preview |
|||||||||
![gemini-code-assist[bot]](https://cdn.github.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors focus and value change handling in the RangePicker component by introducing the useFocusControl and useRangeValueChange hooks. These hooks streamline the coordination of calendar value updates, partial/final submissions, and focus/blur states across multiple fields. The feedback suggests a critical fix in useRangeValueChange to allow smooth field switching when needConfirm is false and the previous field is empty, as well as adding optional chaining to selectorRef.current accesses in RangePicker.tsx to prevent potential runtime errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| // ============================= Utils ============================= | ||
| /** Check whether the target is the container itself or inside it. / 判断目标是否为容器自身或其子元素。 */ | ||
| function containsElement(container: Element | null, target: EventTarget | null) { |
There was a problem hiding this comment.
这个函数直接在 isTargetInContainers 里内联掉，不要额外写个函数
| case 'switchPrevious': { | ||
| if (!needConfirm) { | ||
| const currentValue = getCalendarValue()[actionIndex]; | ||
| const currentEmpty = currentValue === null || currentValue === undefined; | ||
|
|
||
| if (!currentEmpty || allowEmpty[actionIndex]) { | ||
| // Going back may part-submit the current field, but must never | ||
| // finish the whole round before the previous field is edited. | ||
| // 返回上一个 field 时可以局部提交当前 field，但不能在用户修改 | ||
| // 上一个 field 前结束整轮提交。 | ||
| flushSubmit(actionIndex, false); | ||
| } else { | ||
| resetValue(actionIndex); | ||
| } | ||
| } | ||
|
|
||
| const previousPosition = triggeredFieldsRef.current.findIndex( | ||
| (field) => field.index === index, | ||
| ); | ||
| triggeredFieldsRef.current = triggeredFieldsRef.current.slice(0, previousPosition + 1); | ||
| recordTriggeredField(index, false); | ||
| setCurrentIndex(index); | ||
| break; | ||
| } |
There was a problem hiding this comment.
needConfirm 场景切回 previous field 时，这里不会 reset 当前未确认值，但后面会移除对应的 tracking。这样 CalendarValue 仍然保留，状态机却不再记录，后续 blur 是否可能直接走 finish，导致未确认值残留？
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/util/commonUtil.tsx (1)
123-128: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
closePicker的index参数语义已被弱化。现在
index只用于定位getRootNode()，实际 blur 的是当前activeElement，因此closePicker(container, 1)与closePicker(container, 0)行为等价。建议补一行注释说明，避免调用方误以为可以指定失焦的输入框。♻️ 建议补充注释
export function closePicker(container: HTMLElement | ShadowRoot, index = 0) { + // `index` is only used to resolve the root node. The blur always applies to + // the currently focused element to mimic a real focus-leaving behavior. const input = container.querySelectorAll('input')[index];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/util/commonUtil.tsx` around lines 123 - 128, Add a concise comment above or beside the index handling in closePicker explaining that index selects the input’s root for active-element lookup, not the element that receives blur, so callers do not interpret it as choosing a specific input to blur.docs/examples/debug.tsx (1)
12-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value调试示例被裁剪为单一验证场景。
原有的多组 Picker 配置被替换成本次修复对应的复现场景。若该文件仍作为通用调试入口，建议合并前确认是否需要恢复其他示例配置。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/examples/debug.tsx` around lines 12 - 36, 恢复 docs/examples/debug.tsx 中被替换的其他 Picker 调试配置，保留现有通用调试入口场景；将当前基于 RangePicker、changeCount 的复现用例作为新增或并列场景，而不是删除原有示例。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/PickerInput/hooks/useRangeValue.ts`:
- Around line 307-316: Update resetValue in the indexed single-field reset path
to normalize a missing mergedValue[index] to null before passing it to fillIndex
and setSubmitValue. Keep triggerCalendarChange and the existing behavior for
defined values unchanged, ensuring submitValue uses null rather than undefined
for empty range fields.
In `@src/PickerInput/hooks/useRangeValueChange.ts`:
- Around line 264-299: 修正 field-switch 分支中 nextIndex 的计算：不要用 enabledFieldCount
对绝对 currentIndex 取模，而应基于可用字段的绝对 index 列表循环推进并跳过 disabled 字段。处理没有可用字段的情况，避免生成 NaN
或非法 activeIndex/currentIndex；同时保持后续 nextFieldTriggered、确认和切换结果逻辑不变。
In `@src/PickerInput/RangePicker.tsx`:
- Around line 326-342: 更新 RangePicker 中传给 useRangeValueChange 的字段数量：当
enabledFieldCount 为 0 时改用 disabled.length，确保全禁用场景不会让 hook 的索引计算以 0
为除数；非全禁用场景继续使用 enabledFieldCount。
In `@tests/range.spec.tsx`:
- Line 599: Restore the default non-allowEmpty coverage in tests/range.spec.tsx
at lines 599 and 1002, and add separate allowEmpty variants rather than
modifying the existing cases. Apply the same pattern to tests/new-range.spec.tsx
lines 50-53 and 87: preserve the original “defaultPickerValue should reset every
time when opened” and “onPickerValueChange” cases, then add explicit allowEmpty
variants.
---
Nitpick comments:
In `@docs/examples/debug.tsx`:
- Around line 12-36: 恢复 docs/examples/debug.tsx 中被替换的其他 Picker
调试配置，保留现有通用调试入口场景；将当前基于 RangePicker、changeCount 的复现用例作为新增或并列场景，而不是删除原有示例。
In `@tests/util/commonUtil.tsx`:
- Around line 123-128: Add a concise comment above or beside the index handling
in closePicker explaining that index selects the input’s root for active-element
lookup, not the element that receives blur, so callers do not interpret it as
choosing a specific input to blur.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 48e70e4e-6273-4b86-92ba-9e7206537086
📒 Files selected for processing (23)
.github/workflows/react-doctor.ymldocs/examples/debug.tsxsrc/PickerInput/Popup/index.tsxsrc/PickerInput/RangePicker.tsxsrc/PickerInput/Selector/RangeSelector.tsxsrc/PickerInput/Selector/SingleSelector/index.tsxsrc/PickerInput/SinglePicker.tsxsrc/PickerInput/hooks/useFocusEvents.tssrc/PickerInput/hooks/useFocusLock.tssrc/PickerInput/hooks/usePickerRef.tssrc/PickerInput/hooks/useRangeActive.tssrc/PickerInput/hooks/useRangeDisabledDate.tssrc/PickerInput/hooks/useRangePickerValue.tssrc/PickerInput/hooks/useRangeValue.tssrc/PickerInput/hooks/useRangeValueChange.tssrc/utils/miscUtil.tstests/disabledTime.spec.tsxtests/keyboard.spec.tsxtests/multiple.spec.tsxtests/new-range.spec.tsxtests/picker.spec.tsxtests/range.spec.tsxtests/util/commonUtil.tsx
💤 Files with no reviewable changes (1)
- src/PickerInput/hooks/useRangeActive.ts
| const resetValue = useEvent((index?: number) => { | ||
| if (index === undefined) { | ||
| triggerCalendarChange(mergedValue); | ||
| syncWithValue(); | ||
| return; | ||
| } | ||
|
|
||
| // Sync with value anyway | ||
| syncWithValue(); | ||
| } | ||
| }, | ||
| 2, | ||
| ); | ||
| triggerCalendarChange(fillIndex(getCalendarValue(), index, mergedValue[index])); | ||
| setSubmitValue(fillIndex(submitValue(), index, mergedValue[index])); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
单字段重置时 mergedValue[index] 可能是 undefined 而非 null。
mergedValue 在无值时是 EMPTY_VALUE（空数组），mergedValue[index] 取到 undefined。triggerCalendarChange 在 range 模式下会把 undefined 归一成 null，但 setSubmitValue(fillIndex(submitValue(), index, mergedValue[index])) 直接把 undefined 写进了 submitValue，后续 triggerSubmit 会把它原样带进 onChange 的入参，与 range 场景下一贯的 null 约定不一致。
🐛 建议归一为 null
- triggerCalendarChange(fillIndex(getCalendarValue(), index, mergedValue[index]));
- setSubmitValue(fillIndex(submitValue(), index, mergedValue[index]));
+ const resetFieldValue = mergedValue[index] ?? null;
+ triggerCalendarChange(fillIndex(getCalendarValue(), index, resetFieldValue));
+ setSubmitValue(fillIndex(submitValue(), index, resetFieldValue));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const resetValue = useEvent((index?: number) => { | |
| if (index === undefined) { | |
| triggerCalendarChange(mergedValue); | |
| syncWithValue(); | |
| return; | |
| } | |
| // Sync with value anyway | |
| syncWithValue(); | |
| } | |
| }, | |
| 2, | |
| ); | |
| triggerCalendarChange(fillIndex(getCalendarValue(), index, mergedValue[index])); | |
| setSubmitValue(fillIndex(submitValue(), index, mergedValue[index])); | |
| }); | |
| const resetValue = useEvent((index?: number) => { | |
| if (index === undefined) { | |
| triggerCalendarChange(mergedValue); | |
| syncWithValue(); | |
| return; | |
| } | |
| const resetFieldValue = mergedValue[index] ?? null; | |
| triggerCalendarChange(fillIndex(getCalendarValue(), index, resetFieldValue)); | |
| setSubmitValue(fillIndex(submitValue(), index, resetFieldValue)); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/PickerInput/hooks/useRangeValue.ts` around lines 307 - 316, Update
resetValue in the indexed single-field reset path to normalize a missing
mergedValue[index] to null before passing it to fillIndex and setSubmitValue.
Keep triggerCalendarChange and the existing behavior for defined values
unchanged, ensuring submitValue uses null rather than undefined for empty range
fields.
| if (source === 'field-switch') { | ||
| if (index === currentIndex) { | ||
| return 'abort'; | ||
| } | ||
|
|
||
| const nextIndex = (currentIndex + 1) % fieldCount; | ||
| if (index !== nextIndex) { | ||
| return 'abort'; | ||
| } | ||
|
|
||
| const nextFieldTriggered = triggeredFieldsRef.current.some( | ||
| (field) => field.index === nextIndex, | ||
| ); | ||
|
|
||
| if (needConfirm) { | ||
| if (confirmedIndexRef.current === currentIndex) { | ||
| return 'switchNext'; | ||
| } | ||
|
|
||
| // An allowEmpty field may be left without confirmation. Discard any | ||
| // unconfirmed CalendarValue before moving to the next field. | ||
| // allowEmpty field 可以在未确认时离开；切换前需要丢弃未确认的 | ||
| // CalendarValue，再进入下一个 field。 | ||
| return allowEmpty[currentIndex] ? 'resetCurrentAndSwitchNext' : 'abort'; | ||
| } | ||
|
|
||
| if (canSwitch) { | ||
| return 'switchNext'; | ||
| } | ||
|
|
||
| // Revisiting the next field starts another circular round. Discard the | ||
| // invalid current field and finish the old round before entering it. | ||
| // 再次进入已触发的 next field 表示开始新一轮循环。进入前先丢弃当前 | ||
| // 无效 field，并结束旧的一轮。 | ||
| return nextFieldTriggered ? 'resetCurrentAndSwitchNext' : 'resetCurrent'; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 确认 enabledFieldCount 的定义与取值范围，以及 disabled 字段是否会参与 field-switch
rg -nP -C6 'enabledFieldCount' src/PickerInput/RangePicker.tsxRepository: react-component/picker
Length of output: 737
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
fd -a 'useRangeValueChange\.ts|RangePicker\.tsx' src | sed 's#^\./##'
echo
echo "useRangeValueChange outline:"
ast-grep outline src/PickerInput/hooks/useRangeValueChange.ts --view expanded | sed -n '1,220p'
echo
echo "Relevant useRangeValueChange sections:"
sed -n '1,360p' src/PickerInput/hooks/useRangeValueChange.ts
echo
sed -n '360,470p' src/PickerInput/hooks/useRangeValueChange.ts
echo
echo "RangePicker relevant sections around hooks call:"
sed -n '1,80p' src/PickerInput/RangePicker.tsx
echo
sed -n '260,360p' src/PickerInput/RangePicker.tsx
echo
echo "Search field-switch related symbols:"
rg -n -C4 'field-switch|setTriggeredFields|triggeredFields|enabledFieldCount|index ===|currentIndex' src/PickerInput src > /tmp/search_range_picker_output.txt
sed -n '1,220p' /tmp/search_range_picker_output.txtRepository: react-component/picker
Length of output: 40031
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "useRangeValue.ts relevant disable/index handling:"
sed -n '1,380p' src/PickerInput/hooks/useRangeValue.ts
echo
echo "RangeSelector relevant field-switch calls:"
rg -n -C8 'onFocus|field-switch|index' src/PickerInput/Selector/RangeSelector.tsx
echo
echo "Behavioral probe for modulo/index mapping modeled on current hook:"
node - <<'JS'
function resolveFieldSwitch({fieldCount, currentIndex, index, needConfirm, allowEmpty, triggeredAbsolute, confirmedIndex}) {
if (index === currentIndex) return 'abort';
const nextIndex = Number.isFinite(fieldCount) ? (currentIndex + 1) % fieldCount : NaN;
if (index !== nextIndex) return 'abort';
const nextFieldTriggered = triggeredAbsolute.includes(nextIndex);
if (needConfirm) {
if (confirmedIndex === currentIndex) return 'switchNext';
return allowEmpty[currentIndex] ? 'resetCurrentAndSwitchNext' : 'abort';
}
return 'switchNext';
}
for (const [disabled, actions] of Object.entries({
'[true,false]': [{enabledFieldCount:1}, ['switchNext']],
'[false,true]': [{enabledFieldCount:1}, ['abort']],
'[true,true]': [{enabledFieldCount:0}, ['NaN currentIndex']],
})) {
for (const [from, to] of [
['start(0)', 'end(1)'],
['end(1)', 'start(0)'],
['start(0)', 'start(0)'],
]) {
const [aIndex, bIndex] = from.split('(')[1].slice(0,-1),
[nIndex] = to.split('(')[1].slice(0,-1);
// In current hook, field-switch is called with index=aIndex/NaN, currentIndex=last active absolute.
const cases = [
{desc:`disabled=${disabled} from ${from} to ${to} currentIndex=${aIndex} index=${nIndex}`,
result: (() => {
const current = Number(aIndex), index = parseInt(nIndex,10);
return JSON.stringify(resolveFieldSplit = resolveFieldSwitch({
fieldCount: Number(aIndex) === 0 && Number(nIndex) === 0 && true ? 0 : Number(aIndex)+1,
currentIndex: current,
index: index,
needConfirm: false,
allowEmpty: [false,false],
triggeredAbsolute: [],
confirmedIndex: null,
})
))})()}
];
}
}
console.log(JSON.stringify({
disabledStartTrue: resolveFieldSwitch({fieldCount:1,currentIndex:1,index:0,needConfirm:false,allowEmpty:[false,false],triggeredAbsolute:[],confirmedIndex:null}),
disabledStartFalse: resolveFieldSwitch({fieldCount:1,currentIndex:0,index:1,needConfirm:false,allowEmpty:[false,false],triggeredAbsolute:[],confirmedIndex:null}),
disabledAllTrue: {currentIndex:1,index:0,fieldCount:0,nextIndex:NaN},
}, null, 2));
JS
echo
echo "Current hook execution modulo paths (read-only static extraction):"
python3 - <<'PY'
from pathlib import Path
p=Path('src/PickerInput/hooks/useRangeValueChange.ts')
text=p.read_text()
for word in ['fieldCount', '% fieldCount', 'setCurrentIndex(nextIndex)', 'setCurrentIndex(index)', 'setCurrentIndex(NaN)', 'disabled', 'enabledFieldCount']:
print(f'{word}: {text.count(word)}')
PYRepository: react-component/picker
Length of output: 13267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "RangeSelector lines 160-300:"
sed -n '160,300p' src/PickerInput/Selector/RangeSelector.tsx
echo
echo "focused/onFocus handling around selector activeIndex:"
rg -n -C6 'focused|onFocus|activeIndex|onFieldFocus|triggerRangeValueChange|disabled' src/PickerInput/RangePicker.tsx src/PickerInput/Selector/RangeSelector.tsx src/PickerInput/hooks/useFocusEvents.ts
echo
echo "Clean behavioral probe for modulo/index mapping:"
node - <<'JS'
function resolveFieldSwitch({fieldCount, currentIndex, index, needConfirm, allowEmpty, triggeredAbsolute, confirmedIndex}) {
if (Number.isNaN(fieldCount) || fieldCount <= 0) return {nextIndex: NaN};
if (index === currentIndex) return {action: 'abort'};
const nextIndex = (currentIndex + 1) % fieldCount;
if (index !== nextIndex) return {action: 'abort'};
const nextFieldTriggered = triggeredAbsolute.includes(nextIndex);
if (needConfirm) {
if (confirmedIndex === currentIndex) return {action: 'switchNext'};
return allowEmpty[currentIndex] ? {action: 'resetCurrentAndSwitchNext'} : {action: 'abort'};
}
return {action: 'switchNext', nextIndex};
}
console.log(JSON.stringify({
disabledStartTrue_endActive: resolveFieldSwitch({fieldCount:1,currentIndex:1,index:0,needConfirm:false,allowEmpty:[false,false],triggeredAbsolute:[],confirmedIndex:null}),
disabledStartFalse_endActive: resolveFieldSwitch({fieldCount:1,currentIndex:1,index:1,needConfirm:false,allowEmpty:[false,false],triggeredAbsolute:[],confirmedIndex:null}),
disabledAllTrue_startActive: {currentIndex:1,index:0,fieldCount:0},
disabledAllTrue_endActive: {currentIndex:1,index:1,fieldCount:0},
}, null, 2));
JSRepository: react-component/picker
Length of output: 45371
避免在 disabled 字段的绝对 index 上按 enabledFieldCount 环形推进。
调用方传入的是可用字段数量，但 field-switch / switchNext 仍用 (currentIndex + 1) % fieldCount 得到 0/1 绝对 index。例如 disabled = [true, false] 时，end field 切换到被禁用的 start field 会被当做 switchNext；disabled = [true, true] 且 enabledFieldCount === 0 时取模产生 NaN，会把 currentIndex/activeIndex 染成非法焦点。建议循环推进时使用可用字段的绝对 index 列表，或先跳过 disabled 字段。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/PickerInput/hooks/useRangeValueChange.ts` around lines 264 - 299, 修正
field-switch 分支中 nextIndex 的计算：不要用 enabledFieldCount 对绝对 currentIndex
取模，而应基于可用字段的绝对 index 列表循环推进并跳过 disabled 字段。处理没有可用字段的情况，避免生成 NaN 或非法
activeIndex/currentIndex；同时保持后续 nextFieldTriggered、确认和切换结果逻辑不变。
| const enabledFieldCount = disabled.filter((fieldDisabled) => !fieldDisabled).length; | ||
| const [ | ||
| rangeValueIndex, | ||
| activeIndex, | ||
| setActiveIndex, | ||
| nextActiveIndex, | ||
| activeIndexList, | ||
| updateSubmitIndex, | ||
| hasActiveSubmitValue, | ||
| ] = useRangeActive(disabled, allowEmpty, mergedOpen); | ||
|
|
||
| const onSharedFocus = (event: React.FocusEvent<HTMLElement>, index?: number) => { | ||
| triggerFocus(true); | ||
|
|
||
| onFocus?.(event, { | ||
| range: getActiveRange(index ?? activeIndex), | ||
| }); | ||
| }; | ||
| forceFocus, | ||
| triggeredFields, | ||
| triggerRangeValueChange, | ||
| resetRangeValueChange, | ||
| ] = useRangeValueChange( | ||
| enabledFieldCount, | ||
| needConfirm, | ||
| allowEmpty, | ||
| getCalendarValue, | ||
| triggerFieldCalendarChange, | ||
| flushFieldSubmit, | ||
| resetValue, | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C5 'fieldCount' src/PickerInput/hooks/useRangeValueChange.ts
rg -nP -C3 'enabledFieldCount' src/PickerInputRepository: react-component/picker
Length of output: 3215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- locate files ---\n'
git ls-files | rg '(^|/)RangePicker\.tsx$|(^|/)useRangeValueChange\.ts$' || true
printf '\n--- RangePicker relevant sections ---\n'
if [ -f src/PickerInput/RangePicker.tsx ]; then
sed -n '280,380p' src/PickerInput/RangePicker.tsx | cat -n
fi
printf '\n--- useRangeValueChange outline ---\n'
if [ -f src/PickerInput/hooks/useRangeValueChange.ts ]; then
ast-grep outline src/PickerInput/hooks/useRangeValueChange.ts || true
fi
printf '\n--- useRangeValueChange relevant sections ---\n'
if [ -f src/PickerInput/hooks/useRangeValueChange.ts ]; then
sed -n '1,380p' src/PickerInput/hooks/useRangeValueChange.ts | cat -n
fiRepository: react-component/picker
Length of output: 251
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- locate files ---'
git ls-files | rg '(^|/)RangePicker\.tsx$|(^|/)useRangeValueChange\.ts$' || true
echo
echo '--- RangePicker relevant sections ---'
if [ -f src/PickerInput/RangePicker.tsx ]; then
sed -n '280,380p' src/PickerInput/RangePicker.tsx | cat -n
fi
echo
echo '--- useRangeValueChange outline ---'
if [ -f src/PickerInput/hooks/useRangeValueChange.ts ]; then
ast-grep outline src/PickerInput/hooks/useRangeValueChange.ts || true
fi
echo
echo '--- useRangeValueChange relevant sections ---'
if [ -f src/PickerInput/hooks/useRangeValueChange.ts ]; then
sed -n '1,380p' src/PickerInput/hooks/useRangeValueChange.ts | cat -n
fiRepository: react-component/picker
Length of output: 20869
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- useRangeValueChange execute/action call sites ---'
sed -n '360,560p' src/PickerInput/hooks/useRangeValueChange.ts | cat -n
echo
echo '--- selector callbacks definitions/usages ---'
rg -n -C4 'onSelectorFocus|onSelectorBlur|onSelectorClick|onSelectorKeyDown|selectorProps|triggerOpen\(\s*false' src/PickerInput/RangePicker.tsx
echo
echo '--- all triggerRangeValueChange usages in RangePicker ---'
rg -n -C3 'triggerRangeValueChange|resetRangeValueChange' src/PickerInput/RangePicker.tsxRepository: react-component/picker
Length of output: 12401
避免在全禁用时向 useRangeValueChange 传入 0。
当 disabled=[true, true] 时，enabledFieldCount=0 会让 useRangeValueChange 中的 currentIndex + 1 % fieldCount 变为 NaN；虽然 onSelectorFocus 会先 triggerOpen(true, { disabled: [true, true] }) 即不打开，但焦点切换仍已进入 hook 流程，建议用 0 替换为 disabled.length，或在调用前返回默认 activeIndex，避免分母为 0。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/PickerInput/RangePicker.tsx` around lines 326 - 342, 更新 RangePicker 中传给
useRangeValueChange 的字段数量：当 enabledFieldCount 为 0 时改用 disabled.length，确保全禁用场景不会让
hook 的索引计算以 0 为除数；非全禁用场景继续使用 enabledFieldCount。
|
|
||
| it('mode is array', () => { | ||
| const { container } = render(<DayRangePicker mode={['year', 'month']} />); | ||
| const { container } = render(<DayRangePicker allowEmpty mode={['year', 'month']} />); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
多处既有回归用例被直接加上 allowEmpty，导致默认（非 allowEmpty）路径失去覆盖。 共同根因是新的字段切换规则要求字段有值或 allowEmpty 才能切换，测试通过放开 allowEmpty 来适配，而不是新增变体用例。
tests/range.spec.tsx#L599-L599：mode is array保留无allowEmpty的原始断言，另加allowEmpty变体。tests/range.spec.tsx#L1002-L1002：defaultPickerValue用例同上，保留原路径覆盖。tests/new-range.spec.tsx#L50-L53：defaultPickerValue should reset every time when opened保留原用例，另加allowEmpty变体。tests/new-range.spec.tsx#L87-L87：onPickerValueChange用例同上。
📍 Affects 2 files
tests/range.spec.tsx#L599-L599(this comment)tests/range.spec.tsx#L1002-L1002tests/new-range.spec.tsx#L50-L53tests/new-range.spec.tsx#L87-L87
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/range.spec.tsx` at line 599, Restore the default non-allowEmpty
coverage in tests/range.spec.tsx at lines 599 and 1002, and add separate
allowEmpty variants rather than modifying the existing cases. Apply the same
pattern to tests/new-range.spec.tsx lines 50-53 and 87: preserve the original
“defaultPickerValue should reset every time when opened” and
“onPickerValueChange” cases, then add explicit allowEmpty variants.
![github-actions[bot]](https://cdn.github.com/in/15368?s=60&v=4){kind=small}
| triggerRangeValueChange(rangeValueIndex ?? activeIndex, 'popupClose'); | ||
| } | ||
| }, | ||
| [mergedOpen], |
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useLayoutEffect can run with a stale triggerRangeValueChange, rangeValueIndex, activeIndex & show your users old data.
Fix → Don't blindly add missing dependencies. Read the hook callback first.
Bad:
useEffect(() => {
setCount(count + 1);
}, [count]);
Better:
useEffect(() => {
setCount((currentCount) => currentCount + 1);
}, []);
If the missing value is recreated every render, move it inside the hook or stabilize it before adding it to deps.
| triggerSingleValueChange(0, 'popupClose'); | ||
| } | ||
| }, | ||
| [mergedOpen], |
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useLayoutEffect can run with a stale triggerSingleValueChange & show your users old data.
Fix → Don't blindly add missing dependencies. Read the hook callback first.
Bad:
useEffect(() => {
setCount(count + 1);
}, [count]);
Better:
useEffect(() => {
setCount((currentCount) => currentCount + 1);
}, []);
If the missing value is recreated every render, move it inside the hook or stabilize it before adding it to deps.
2 participants
Summary
PickerTrigger.onCloseresponsible only for popup visibilityMotivation
RangePicker currently spreads part-submit and reset behavior across event handlers and an effect. This refactor prepares a single event-driven decision point for confirm and non-confirm flows, following the behavior discussed around #966.
Impact
This changes RangePicker's internal interaction handling only. The existing
useRangeValuehook remains responsible for value validation and final submission.Validation
ut lint:tscut lint— 0 errorsInteraction flow
canSwitch = 当前值非空 || allowEmpty[currentIndex]allFieldsTriggered = triggeredFields.length >= fieldCountmodifyswitchNextswitchPreviousfinishabortresetCurrentresetCurrentAndSwitchNextresetAllflowchart TB START["① 入口与 field-switch<br/>triggerChange(index, source, value?)"] --> ESC{"🔎 source === esc?"} ESC -- "是" --> ESC_RESET["🗑️ action = resetAll"] ESC -- "否" --> IDLE{"🔎 currentIndex === null?"} IDLE -- "是，source = blur" --> IDLE_RESET["🗑️ action = resetAll"] IDLE -- "是，其他事件" --> INIT["📌 初始化 currentIndex = index<br/>记录 triggered field"] IDLE -- "否" --> ROUTE{"🔎 source === field-switch?"} INIT --> ROUTE ROUTE -- "否" --> CURRENT_ENTRY["进入 ② 当前 field 事件"] ROUTE -- "是" --> FS_SAME{"🔎 index === currentIndex?"} FS_SAME -- "是" --> FS_ABORT_SAME["⛔ action = abort"] FS_SAME -- "否" --> FS_PREVIOUS{"🔎 是否为已访问的上一个 field?"} FS_PREVIOUS -- "是" --> FS_PREVIOUS_LOCK{"🔎 needConfirm<br/>且当前值非空、未确认<br/>且不允许空?"} FS_PREVIOUS_LOCK -- "是" --> FS_ABORT_PREVIOUS["⛔ action = abort"] FS_PREVIOUS_LOCK -- "否" --> FS_SWITCH_PREVIOUS["⬅️ action = switchPrevious"] FS_PREVIOUS -- "否" --> FS_NEXT{"🔎 index === nextIndex?"} FS_NEXT -- "否" --> FS_ABORT_INVALID["⛔ action = abort"] FS_NEXT -- "是" --> FS_CONFIRM{"🔎 needConfirm?"} FS_CONFIRM -- "否" --> FS_CAN_SWITCH{"🔎 canSwitch?"} FS_CAN_SWITCH -- "是" --> FS_SWITCH_NEXT["➡️ action = switchNext"] FS_CAN_SWITCH -- "否" --> FS_RESET_CURRENT["↩️ action = resetCurrent"] FS_CONFIRM -- "是" --> FS_CONFIRMED{"🔎 当前 field 已确认?"} FS_CONFIRMED -- "是" --> FS_SWITCH_CONFIRMED["➡️ action = switchNext"] FS_CONFIRMED -- "否" --> FS_ALLOW_EMPTY{"🔎 allowEmpty[currentIndex]?"} FS_ALLOW_EMPTY -- "是" --> FS_RESET_SWITCH["🔄 action = resetCurrentAndSwitchNext"] FS_ALLOW_EMPTY -- "否" --> FS_ABORT_LOCKED["⛔ action = abort"]flowchart TB START["② 当前 field 事件<br/>非 field-switch"] --> CURRENT_INDEX{"🔎 index === currentIndex?"} CURRENT_INDEX -- "否" --> CURRENT_ABORT["⛔ action = abort"] CURRENT_INDEX -- "是" --> SOURCE{"🔎 source 类型"} SOURCE -- "input / panel-intermediate" --> MODIFY["✏️ action = modify"] SOURCE -- "panel-final" --> PANEL_CONFIRM{"🔎 needConfirm?"} PANEL_CONFIRM -- "是" --> PANEL_MODIFY["✏️ action = modify"] PANEL_CONFIRM -- "否" --> PANEL_SWITCH["➡️ action = switchNext"] SOURCE -- "keyboard-submit / confirm" --> SUBMIT_VALID{"🔎 canSwitch?"} SUBMIT_VALID -- "是" --> SUBMIT_SWITCH["➡️ action = switchNext"] SUBMIT_VALID -- "否" --> SUBMIT_ABORT["⛔ action = abort"] SOURCE -- "blur" --> BLUR_ENTRY["进入 ③ blur 分支"]flowchart TB START["③ blur 分支"] --> BLUR_MODIFIED{"🔎 本轮是否修改过任意 field?"} BLUR_MODIFIED -- "否" --> BLUR_FINISH["✅ action = finish"] BLUR_MODIFIED -- "是" --> BLUR_CONFIRM{"🔎 needConfirm?"} BLUR_CONFIRM -- "否" --> BLUR_CAN_SWITCH{"🔎 canSwitch?"} BLUR_CAN_SWITCH -- "是" --> BLUR_SWITCH["➡️ action = switchNext"] BLUR_CAN_SWITCH -- "否" --> BLUR_RESET_INVALID["🗑️ action = resetAll"] BLUR_CONFIRM -- "是" --> BLUR_READY{"🔎 allFieldsTriggered<br/>且 canSwitch?"} BLUR_READY -- "否" --> BLUR_RESET_ALL["🗑️ action = resetAll"] BLUR_READY -- "是" --> BLUR_CURRENT_MODIFIED{"🔎 当前 field 是否修改过?"} BLUR_CURRENT_MODIFIED -- "否" --> BLUR_FINAL_SWITCH["➡️ action = switchNext"] BLUR_CURRENT_MODIFIED -- "是" --> BLUR_ALLOW_EMPTY{"🔎 allowEmpty[currentIndex]?"} BLUR_ALLOW_EMPTY -- "是" --> BLUR_RESET_SWITCH["🔄 action = resetCurrentAndSwitchNext"] BLUR_ALLOW_EMPTY -- "否" --> BLUR_RESET_UNCONFIRMED["🗑️ action = resetAll"]Summary by CodeRabbit
containerRef，便于外部获取/控制容器 DOM 引用。RangeSelector的 ref 现可直接访问起止输入元素。onChange现在可区分触发来源（如输入/移除）。