feat: 支持 disabled 数组和 onDisabledChange 回调#1069
Open
EmilyyyLiu wants to merge 13 commits intoreact-component:masterfrom
Open
feat: 支持 disabled 数组和 onDisabledChange 回调#1069EmilyyyLiu wants to merge 13 commits intoreact-component:masterfrom
EmilyyyLiu wants to merge 13 commits intoreact-component:masterfrom
Conversation
- Update README.md with disabled array and onDisabledChange documentation - Fix BasicDisabledHandle example to handle sparse disabled array Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Someone is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough将 Slider 的 Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Handle as Handle
participant Slider as Slider
participant useDrag as useDrag Hook
participant useOffset as useOffset Hook
User->>Handle: 交互（点击/拖动/键盘），携带 valueIndex
Handle->>Slider: 转发交互请求（valueIndex）
Slider->>Slider: 调用 isHandleDisabled(valueIndex)
alt 手柄已禁用
Slider->>Handle: 阻断交互，返回 aria/tabIndex 状态
else 手柄可用
Slider->>useDrag: 初始化拖动（含 valueIndex）
useDrag->>useOffset: 请求位移计算（含 isHandleDisabled）
useOffset->>useOffset: 计算左右禁用边界并约束/推送值
useOffset-->>useDrag: 返回受限的新值
useDrag->>Slider: 触发 onChange/onDrag 回调
Slider-->>Handle: 更新渲染与聚焦
end
Estimated code review effort🎯 4 (复杂) | ⏱️ ~45 分钟 Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1069 +/- ##
==========================================
- Coverage 99.39% 98.78% -0.61%
==========================================
Files 14 14
Lines 661 741 +80
Branches 191 218 +27
==========================================
+ Hits 657 732 +75
- Misses 4 9 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for disabling specific handles in range sliders by allowing the disabled prop to accept an array of booleans. Key changes include updates to handle interaction logic (mouse, keyboard, and track dragging), ARIA accessibility attributes, and the addition of an onDisabledChange callback to maintain state synchronization in editable mode. Review feedback highlighted a potential bug in the array synchronization logic when duplicate values are present and suggested a more consistent approach for determining the slider's overall disabled state in single-handle mode.
![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: 2
🧹 Nitpick comments (3)
README.md (1)
111-111: 建议微调说明文案可读性。Line 111 的第二句可补充主语让语义更自然，例如改为 “This prop can also be an array...”。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 111, The README description for the disabled prop is slightly awkward; update the second sentence for clarity by adding an explicit subject, e.g. change the clause "Can also be an array to disable specific handles..." to "This prop can also be an array to disable specific handles..." so the property description for disabled reads naturally and clearly (reference: property name "disabled" in the table).docs/examples/disabled-handle.tsx (2)
53-79: 稀疏数组可能导致示例行为不够清晰。
useState([true])创建的数组只有索引 0 有值，但滑块有 4 个手柄（值为[0, 30, 60, 100]）。虽然!!disabled[index]能正确处理undefined（转为false），但这种稀疏数组的初始化方式可能让用户困惑。建议初始化与
value数组长度一致的disabled数组：♻️ 建议的改进
const BasicDisabledHandle = () => { const [value, setValue] = useState<number[]>([0, 30, 60, 100]); - const [disabled, setDisabled] = useState([true]); + const [disabled, setDisabled] = useState([true, false, false, false]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/disabled-handle.tsx` around lines 53 - 79, The disabled array is initialized as a sparse array via useState([true]) while BasicDisabledHandle’s value has four handles, which can confuse readers; update the disabled state initialization to create an array matching value.length (e.g., using useState(() => value.map(() => true/false) or Array(value.length).fill(false)) so each handle index is explicitly defined, and ensure the existing onChange handler that uses setDisabled and !!disabled[index] continues to work with the fully populated array.
115-117: 移除空的 div 元素。这个空的
<div>元素没有任何内容或样式，建议移除。♻️ 建议的改进
<div> single handle disabled <SingleSlider /> </div> - <div> - - </div> <div style={style}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/disabled-handle.tsx` around lines 115 - 117, Remove the empty JSX element: delete the empty <div></div> fragment found in the component's render/return so there is no unused/blank wrapper element left; ensure any necessary layout or spacing is preserved by keeping surrounding elements intact and not altering component logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useOffset.ts`:
- Around line 198-222: The current adjustment logic around minBound/maxBound and
the subsequent push flow allows disabled handles to be moved under allowCross +
pushable because only the active handle is clamped; update the logic in
useOffset (symbols: isHandleDisabled, valueIndex, nextValues, offsetValue,
minBound, maxBound) so that when computing minBound/maxBound you skip disabled
handles (as you already try) and also ensure the push loop(s) that propagate
offsets explicitly ignore disabled handles (do not push into or from an index
where isHandleDisabled(index) is true); in practice, filter or short-circuit any
push iteration to treat disabled handles as immutable boundaries and clamp
nextValues[valueIndex] accordingly before and after calling offsetValue and
before performing any neighbor adjustments.
In `@src/Slider.tsx`:
- Around line 324-341: The current logic that finds the insert/remove index
using includes can fail when values repeat; instead compute the index by
scanning the arrays for the first position where the sequences diverge: when
cloneNextValues is longer, walk both arrays i and j and return the index in
cloneNextValues where an extra item appears; when cloneNextValues is shorter,
walk both arrays and return the index in rawValues where an item was removed.
Update the block that mutates rawDisabled (references: rawValues,
cloneNextValues, rawDisabled, newDisabled, onDisabledChange) to use this
deterministic sequence-compare approach (or a small helper like
getDiffIndex(existing, next)) and then splice at that returned index rather than
using findIndex/includes.
---
Nitpick comments:
In `@docs/examples/disabled-handle.tsx`:
- Around line 53-79: The disabled array is initialized as a sparse array via
useState([true]) while BasicDisabledHandle’s value has four handles, which can
confuse readers; update the disabled state initialization to create an array
matching value.length (e.g., using useState(() => value.map(() => true/false) or
Array(value.length).fill(false)) so each handle index is explicitly defined, and
ensure the existing onChange handler that uses setDisabled and !!disabled[index]
continues to work with the fully populated array.
- Around line 115-117: Remove the empty JSX element: delete the empty
<div></div> fragment found in the component's render/return so there is no
unused/blank wrapper element left; ensure any necessary layout or spacing is
preserved by keeping surrounding elements intact and not altering component
logic.
In `@README.md`:
- Line 111: The README description for the disabled prop is slightly awkward;
update the second sentence for clarity by adding an explicit subject, e.g.
change the clause "Can also be an array to disable specific handles..." to "This
prop can also be an array to disable specific handles..." so the property
description for disabled reads naturally and clearly (reference: property name
"disabled" in the table).
🪄 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
Run ID: df159bb5-73f9-4e53-a895-b3b282545dd3
📒 Files selected for processing (11)
README.mdassets/index.lessdocs/demo/disabled-handle.mddocs/examples/disabled-handle.tsxsrc/Handles/Handle.tsxsrc/Slider.tsxsrc/Tracks/index.tsxsrc/context.tssrc/hooks/useDrag.tssrc/hooks/useOffset.tstests/Range.test.tsx
EmilyyyLiu
changed the title
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/Slider.tsx (4)
193-203:value为undefined时的边界处理。当
value为undefined（未受控或初始状态）时，Array.isArray(value)为false，此时若rawDisabled为数组，disabled会返回false。这可能不符合预期——例如用户传入disabled={[true]}但未传value时，全局disabled仍为false。建议考虑是否应使用
rawValues（已处理后的值数组）来判断：💡 可选改进
const disabled = React.useMemo(() => { if (typeof rawDisabled === 'boolean') { return rawDisabled; } - if (Array.isArray(value)) { - return value.every((_, index) => rawDisabled[index]); + // 使用 rawValues 保证与内部状态一致 + if (rawValues.length > 0) { + return rawValues.every((_, index) => rawDisabled[index]); } return false; -}, [rawDisabled, value]); +}, [rawDisabled, rawValues]);注意：此改动需要调整变量声明顺序，将
rawValues的计算移到disabled之前。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Slider.tsx` around lines 193 - 203, The disabled computation currently checks Array.isArray(value) which returns false when value is undefined, causing cases like rawDisabled=[true] with no value to incorrectly yield disabled=false; update the logic in the disabled React.useMemo (symbols: disabled, rawDisabled, value) to use the processed array of values (rawValues) instead of raw value so array-shaped rawDisabled is honored even when value is undefined, and move the rawValues calculation above the disabled useMemo so rawValues is available when computing disabled.
205-213: 建议在开发环境添加长度不匹配警告。当
rawDisabled为数组但长度与rawValues不匹配时，越界索引默默返回false。这可能掩盖配置错误。💡 可选：添加开发环境警告
const isHandleDisabled = React.useCallback( (index: number) => { if (typeof rawDisabled === 'boolean') { return rawDisabled; } + if (process.env.NODE_ENV !== 'production' && index >= rawDisabled.length) { + warning(false, `[rc-slider] \`disabled\` array length (${rawDisabled.length}) is shorter than handle count.`); + } return rawDisabled[index] || false; }, [rawDisabled], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Slider.tsx` around lines 205 - 213, The isHandleDisabled callback should warn in development when rawDisabled is an array whose length doesn't match rawValues to surface configuration errors; update isHandleDisabled (or a small helper it calls) to detect Array.isArray(rawDisabled) and when process.env.NODE_ENV !== 'production' and rawDisabled.length !== rawValues.length emit a console.warn (or the component's logger) describing the mismatch and the expected length, then continue returning the current boolean behaviour (rawDisabled[index] || false) so runtime behavior is unchanged.
437-439: 越界访问rawValues[valueBeforeIndex + 1]可能返回undefined。当
valueBeforeIndex为最后一个索引时，rawValues[valueBeforeIndex + 1]返回undefined，此时newValue > undefined始终为false，虽然不会导致错误行为，但代码意图不够清晰。💡 建议增加边界检查以提高可读性
-if (rightDisabled && newValue > rawValues[valueBeforeIndex + 1]) { +if (rightDisabled && valueBeforeIndex + 1 < rawValues.length && newValue > rawValues[valueBeforeIndex + 1]) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Slider.tsx` around lines 437 - 439, The condition accesses rawValues[valueBeforeIndex + 1] without ensuring the index exists; update the conditional in the Slider logic so it first checks that valueBeforeIndex + 1 is within rawValues.length before comparing (e.g., ensure valueBeforeIndex + 1 < rawValues.length), then apply the existing check (rightDisabled && newValue > rawValues[valueBeforeIndex + 1]); reference the variables rightDisabled, newValue, rawValues, and valueBeforeIndex in the function handling value updates to locate and fix the code.
466-484: 逻辑正确，但与useOffset.ts中存在重复代码。查找 disabled 边界并 clamp 值的逻辑与
src/hooks/useOffset.ts中的实现（第 199-223 行）几乎相同。当前实现可以接受，但后续可考虑提取为共享工具函数以减少维护成本。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Slider.tsx` around lines 466 - 484, Duplicate logic: the code in Slider.tsx that finds the nearest disabled handle bounds and clamps newValue (using mergedMin/mergedMax, rawValues, valueIndex, isHandleDisabled, and writing into cloneNextValues) is the same as the implementation in useOffset.ts; extract this into a shared helper (e.g., clampWithinDisabledBounds or findDisabledBoundsAndClamp) that accepts rawValues, valueIndex, isHandleDisabled callback, newValue, mergedMin, mergedMax and returns the clamped value (or bounds + clamped value), then replace the duplicated block in both Slider.tsx (where cloneNextValues[valueIndex] is set and focusIndex updated) and in useOffset.ts to call the new helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Slider.tsx`:
- Around line 193-203: The disabled computation currently checks
Array.isArray(value) which returns false when value is undefined, causing cases
like rawDisabled=[true] with no value to incorrectly yield disabled=false;
update the logic in the disabled React.useMemo (symbols: disabled, rawDisabled,
value) to use the processed array of values (rawValues) instead of raw value so
array-shaped rawDisabled is honored even when value is undefined, and move the
rawValues calculation above the disabled useMemo so rawValues is available when
computing disabled.
- Around line 205-213: The isHandleDisabled callback should warn in development
when rawDisabled is an array whose length doesn't match rawValues to surface
configuration errors; update isHandleDisabled (or a small helper it calls) to
detect Array.isArray(rawDisabled) and when process.env.NODE_ENV !== 'production'
and rawDisabled.length !== rawValues.length emit a console.warn (or the
component's logger) describing the mismatch and the expected length, then
continue returning the current boolean behaviour (rawDisabled[index] || false)
so runtime behavior is unchanged.
- Around line 437-439: The condition accesses rawValues[valueBeforeIndex + 1]
without ensuring the index exists; update the conditional in the Slider logic so
it first checks that valueBeforeIndex + 1 is within rawValues.length before
comparing (e.g., ensure valueBeforeIndex + 1 < rawValues.length), then apply the
existing check (rightDisabled && newValue > rawValues[valueBeforeIndex + 1]);
reference the variables rightDisabled, newValue, rawValues, and valueBeforeIndex
in the function handling value updates to locate and fix the code.
- Around line 466-484: Duplicate logic: the code in Slider.tsx that finds the
nearest disabled handle bounds and clamps newValue (using mergedMin/mergedMax,
rawValues, valueIndex, isHandleDisabled, and writing into cloneNextValues) is
the same as the implementation in useOffset.ts; extract this into a shared
helper (e.g., clampWithinDisabledBounds or findDisabledBoundsAndClamp) that
accepts rawValues, valueIndex, isHandleDisabled callback, newValue, mergedMin,
mergedMax and returns the clamped value (or bounds + clamped value), then
replace the duplicated block in both Slider.tsx (where
cloneNextValues[valueIndex] is set and focusIndex updated) and in useOffset.ts
to call the new helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 725017db-e67b-40f6-836b-89b000fbe4b1
📒 Files selected for processing (2)
src/Slider.tsxtests/Range.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Range.test.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Slider.tsx (1)
333-344:findDiffIndex逻辑已改进，但存在潜在边界问题。当
arr1比arr2短时（如删除场景调用findDiffIndex(rawValues, cloneNextValues)），循环中arr1[i]在i >= arr1.length时会返回undefined，导致后续filter比较可能产生非预期结果。建议将循环边界限制为较短数组的长度加一：
♻️ 建议的优化
const findDiffIndex = (arr1: number[], arr2: number[]) => { - const maxLen = Math.max(arr1.length, arr2.length); + // Only need to check up to the longer array's length, but stop early when diff found + const maxLen = arr1.length; for (let i = 0; i < maxLen; i++) { // Count occurrences up to current index const count1 = arr1.slice(0, i + 1).filter((v) => v === arr1[i]).length; const count2 = arr2.filter((v) => v === arr1[i]).length; if (count1 > count2) { return i; } } - return -1; + // If no diff found in arr1, the new element is at the end of arr2 + return arr1.length; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Slider.tsx` around lines 333 - 344, The findDiffIndex function can access arr1[i] when i >= arr1.length causing undefined comparisons; fix by limiting the loop to Math.max(0, Math.min(arr1.length, arr2.length) + 1) (i.e., iterate only up to the shorter array length plus one) so you never read past arr1; update the loop bound inside findDiffIndex and keep the existing counting logic using arr1[i] safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Slider.tsx`:
- Around line 333-344: The findDiffIndex function can access arr1[i] when i >=
arr1.length causing undefined comparisons; fix by limiting the loop to
Math.max(0, Math.min(arr1.length, arr2.length) + 1) (i.e., iterate only up to
the shorter array length plus one) so you never read past arr1; update the loop
bound inside findDiffIndex and keep the existing counting logic using arr1[i]
safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 607a8c7b-bff6-43a1-bf76-20944b5c2b9c
📒 Files selected for processing (4)
README.mddocs/examples/disabled-handle.tsxsrc/Slider.tsxsrc/hooks/useOffset.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/useOffset.ts
- docs/examples/disabled-handle.tsx
EmilyyyLiu
force-pushed
the
feat/disabled-handle-array
branch
from
e9fd7e1 to
604d94e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/hooks/useOffset.ts (1)
198-221:⚠️ Potential issue | 🟠 Major
pushable间距在 disabled 边界处仍然会被破坏。这里把
minBound/maxBound直接钳到 disabled 句柄的值，但没有预留pushable距离。allowCross + pushable={10}下，启用句柄仍然可以被推到 disabled 句柄上或其禁区内，disabled 句柄并没有真正变成“不可推动的边界”。这一点在[20(disabled), 40, 60]这类场景里会直接破坏最小间距约束。建议修复
- let minBound = min; - let maxBound = max; + let minBound = min; + let maxBound = max; + const pushGap = typeof pushable === 'number' ? pushable : 0; if (isHandleDisabled) { for (let i = valueIndex - 1; i >= 0; i -= 1) { if (isHandleDisabled(i)) { - minBound = nextValues[i]; + minBound = nextValues[i] + pushGap; break; } } for (let i = valueIndex + 1; i < nextValues.length; i += 1) { if (isHandleDisabled(i)) { - maxBound = nextValues[i]; + maxBound = nextValues[i] - pushGap; break; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useOffset.ts` around lines 198 - 221, The code clamps nextValues[valueIndex] to disabled handle values but ignores the configured pushable distance, allowing enabled handles to be moved into disabled handles' push zones; inside useOffset, when you find a disabled handle (using isHandleDisabled and nextValues), adjust minBound to disabledValue + pushable for left-side disabled handles and adjust maxBound to disabledValue - pushable for right-side disabled handles (use the pushable variable from the surrounding scope), then use those adjusted bounds when clamping nextValues[valueIndex] after calling offsetValue; update the min/max search loops (the ones referencing minBound/maxBound and nextValues[i]) and the final clamp so disabled handles truly enforce the pushable spacing.src/Slider.tsx (1)
324-341:⚠️ Potential issue | 🟠 Major删除句柄时的
disabled同步在重复值场景下仍然会错位。新增分支已经改成了按位置比较，但删除分支还在用
includes推断“哪个值被删掉了”。只要rawValues里有重复值，这里就会返回-1或删错索引，最终把错误的 disabled 状态同步给外部。更稳妥的做法是把新增/删除的句柄索引从调用点传进来，而不是在这里根据排序后的值数组反推。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Slider.tsx` around lines 324 - 341, The deletion branch misidentifies the removed handle when rawValues contains duplicates by using includes; change the deletion logic in the block that handles cloneNextValues.length < rawValues.length so it finds the removed handle by positional comparison (mirror the insertion approach): compute index = rawValues.findIndex((item, i) => item !== cloneNextValues[i]) and set removeIndex = index === -1 ? rawValues.length - 1 : index, then call newDisabled.splice(removeIndex, 1) and call onDisabledChange?.(newDisabled); alternatively, modify the caller to pass the explicit add/remove index into this function and use that index instead of inferring it here (refer to rawDisabled, cloneNextValues, rawValues, newDisabled, onDisabledChange).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Slider.tsx`:
- Around line 437-459: The click-path can assign newValue directly to
cloneNextValues[valueIndex] after redirecting to the nearest enabled handle,
which skips the disabled-boundary clamping added in useOffset; update the
assignment to first clamp the candidate newValue against left/right disabled
neighbors (or call the existing offsetValues(..., 'dist') helper to compute the
adjusted value) before setting cloneNextValues[valueIndex] and focusIndex so
enabled handles cannot be moved outside disabled boundaries; adjust logic around
isHandleDisabled, rawValues, valueIndex, cloneNextValues and newValue
accordingly.
- Around line 193-203: The current disabled useMemo wrongly derives "all
disabled" from the controlled value, which breaks uncontrolled cases; update the
computation in the disabled memo (the block referencing rawDisabled and value)
to base the check on the actual rendered handles array instead of only value —
e.g., derive a valuesList that uses value when controlled but falls back to
defaultValue (or whatever internal mergedValues/handles array the component
renders) when uncontrolled, then return valuesList.every((_, i) =>
rawDisabled[i]) (and handle non-array rawDisabled/boolean cases as before).
---
Duplicate comments:
In `@src/hooks/useOffset.ts`:
- Around line 198-221: The code clamps nextValues[valueIndex] to disabled handle
values but ignores the configured pushable distance, allowing enabled handles to
be moved into disabled handles' push zones; inside useOffset, when you find a
disabled handle (using isHandleDisabled and nextValues), adjust minBound to
disabledValue + pushable for left-side disabled handles and adjust maxBound to
disabledValue - pushable for right-side disabled handles (use the pushable
variable from the surrounding scope), then use those adjusted bounds when
clamping nextValues[valueIndex] after calling offsetValue; update the min/max
search loops (the ones referencing minBound/maxBound and nextValues[i]) and the
final clamp so disabled handles truly enforce the pushable spacing.
In `@src/Slider.tsx`:
- Around line 324-341: The deletion branch misidentifies the removed handle when
rawValues contains duplicates by using includes; change the deletion logic in
the block that handles cloneNextValues.length < rawValues.length so it finds the
removed handle by positional comparison (mirror the insertion approach): compute
index = rawValues.findIndex((item, i) => item !== cloneNextValues[i]) and set
removeIndex = index === -1 ? rawValues.length - 1 : index, then call
newDisabled.splice(removeIndex, 1) and call onDisabledChange?.(newDisabled);
alternatively, modify the caller to pass the explicit add/remove index into this
function and use that index instead of inferring it here (refer to rawDisabled,
cloneNextValues, rawValues, newDisabled, onDisabledChange).
🪄 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
Run ID: 18679b8c-2503-4925-975e-60d949c0d40f
📒 Files selected for processing (5)
README.mddocs/examples/disabled-handle.tsxsrc/Slider.tsxsrc/hooks/useOffset.tstests/Range.test.tsx
✅ Files skipped from review due to trivial changes (1)
- docs/examples/disabled-handle.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Slider.tsx (1)
333-362: 同步逻辑改进良好，但初始对齐问题值得注意。
findDiffIndex的实现比之前的includes方案更健壮，能正确处理重复值场景。一个潜在问题：如果用户传入的
disabled数组长度与rawValues长度本就不一致，这里的 splice 操作会在已经错位的基础上继续操作。建议考虑在回调前校验长度一致性，或在文档中明确要求disabled数组长度必须与 handle 数量匹配。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Slider.tsx` around lines 333 - 362, 当前逻辑在根据 cloneNextValues/rawValues 差异插入或删除 disabled 元素时没有处理传入的 rawDisabled 与 rawValues 初始长度不一致的问题；请在使用 findDiffIndex 和对 newDisabled 执行 splice 前，校验并归一化 rawDisabled 长度（例如：如果 rawDisabled 长度大于 rawValues，先 slice 到 rawValues.length；如果小于，则用 false 或指定默认值填充到 rawValues.length），然后基于归一化后的数组执行插入/删除，最后再调用 onDisabledChange(newDisabled)；保留变量名 rawDisabled、rawValues、cloneNextValues、findDiffIndex、newDisabled 和 onDisabledChange 以便定位修改点，并在校验失败时可选地通过 console.warn 提醒调用方。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Slider.tsx`:
- Around line 333-362: 当前逻辑在根据 cloneNextValues/rawValues 差异插入或删除 disabled
元素时没有处理传入的 rawDisabled 与 rawValues 初始长度不一致的问题；请在使用 findDiffIndex 和对 newDisabled
执行 splice 前，校验并归一化 rawDisabled 长度（例如：如果 rawDisabled 长度大于 rawValues，先 slice 到
rawValues.length；如果小于，则用 false 或指定默认值填充到
rawValues.length），然后基于归一化后的数组执行插入/删除，最后再调用 onDisabledChange(newDisabled)；保留变量名
rawDisabled、rawValues、cloneNextValues、findDiffIndex、newDisabled 和
onDisabledChange 以便定位修改点，并在校验失败时可选地通过 console.warn 提醒调用方。
40985bc to
aca26f6
Compare
EmilyyyLiu
force-pushed
the
feat/disabled-handle-array
branch
from
aca26f6 to
15e473f
Compare
Member
|
@gemini-code-assist CR again |
There was a problem hiding this comment.
Code Review
This pull request introduces support for disabling individual handles in range mode by allowing the disabled prop to accept an array of booleans. The implementation includes updates to the slider logic to enforce boundary constraints around disabled handles, preventing them from being crossed or pushed during interaction. Additionally, a new onDisabledChange callback has been added to keep the disabled state in sync when handles are added or removed in editable mode. Review feedback highlights the need for a more robust index detection method when syncing disabled states, a fix for the global disabled state logic when no handles are present, and the removal of a redundant type check.
| } | ||
| if (Array.isArray(rawDisabled)) { | ||
| const values = Array.isArray(mergedValue) ? mergedValue : [mergedValue]; | ||
| return values.every((_, index) => rawDisabled[index]); |
There was a problem hiding this comment.
When mergedValue is an empty array (which can happen in editable mode), values.every() returns true. This causes the slider to appear visually disabled (via the rc-slider-disabled class) even if it's not. Consider ensuring there is at least one handle before concluding the slider is fully disabled.
Suggested change
| return values.every((_, index) => rawDisabled[index]); | |
| return values.length > 0 && values.every((_, index) => rawDisabled[index]); |
Contributor
Author
There was a problem hiding this comment.
values一定是数组没必要判断长度吧？
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
EmilyyyLiu
changed the title
zombieJ
reviewed
Apr 17, 2026
EmilyyyLiu
force-pushed
the
feat/disabled-handle-array
branch
from
f2bb03c to
056db7a
Compare
Add test cases to cover pushable behavior with disabled handles: - Test pushable maintains distance when pushing toward disabled handle - Test pushable with step=null and disabled handles Remove duplicate tests: - Remove 'pushable with boolean true' (covered by existing tests) - Remove 'pushable without disabled handles' (covered by 'pushable & allowCross') Consolidate similar tests to reduce duplication while maintaining coverage. Coverage: 98.59% statements, 90.29% branches, 100% functions, 100% lines
EmilyyyLiu
force-pushed
the
feat/disabled-handle-array
branch
from
056db7a to
eee70b9
Compare
zombieJ
reviewed
Apr 17, 2026
| newDisabled.splice(index, 1); | ||
| } | ||
|
|
||
| onDisabledChange?.(newDisabled); |
Member
There was a problem hiding this comment.
感觉有点怪异，这么看只要让 disabled 节点将线段切成两端，大家只能在自己的那段里处理就行了，这样比较通用。不需要 onDisabledChange。
然后 editable 那里，先做成存在一个 disabled 节点就禁止全部 editable，未来 onChange 加一个 info 属性支持告知添加是哪个 index 后再放开
- Remove onDisabledChange callback and related sync logic - Disable editable mode when any handle is disabled - Disabled handles act as fixed boundaries that cannot be crossed - Update examples and tests to reflect new behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
zombieJ
reviewed
Apr 17, 2026
| ({ value: nextValues[i + 1], changed } = offsetChangedValue(nextValues, 1, i + 1)); | ||
| } | ||
| // Apply boundary constraint to pushed handle | ||
| if (isHandleDisabled) { |
Member
There was a problem hiding this comment.
isHandleDisabled 很奇怪，有的地方当 func 有的地方比如这里直接 if。你在 Slider 里既然始终会传入 isHandleDisabled，那应该 if 都是不需要的
EmilyyyLiu
force-pushed
the
feat/disabled-handle-array
branch
from
a82de9b to
9584ac3
Compare
- Change isHandleDisabled from optional to required in SliderContextProps - Remove unnecessary null checks in Tracks and Handle components Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
React.createContext default value was missing isHandleDisabled after making it required in SliderContextProps Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
zombieJ
reviewed
Apr 17, 2026
| isHandleDisabled, | ||
| } = React.useContext(SliderContext); | ||
|
|
||
| const handleDisabled = globalDisabled || isHandleDisabled(valueIndex); |
Member
There was a problem hiding this comment.
这边已经不需要 globalDisabled 了，只需要 isHandleDisabled 来管，上游的 isHandleDisabled 如果遇到全 disabled，它返回的也是 disabled
Member
There was a problem hiding this comment.
handleDisabled 名字叫 mergedDisabled
zombieJ
reviewed
Apr 17, 2026
| }; | ||
|
|
||
| // Get the minimum boundary for a handle considering disabled handles | ||
| const getHandleMinBound = (values: number[], handleIndex: number): number => { |
Member
There was a problem hiding this comment.
参考一下，不一定代码完全一样：
更推荐的方式——将禁用 handle 统一抽象为"固定锚点"，在计算边界时把禁用 handle 和 min/max 一起纳入约束集合，而不是在遍历时散落处理：
const getEffectiveMin = (values: number[], handleIndex: number): number => {
const gap = typeof pushable === 'number' ? pushable : 0;
// 收集所有左侧禁用 handle 的位置作为候选下界
const candidates = [min];
for (let i = handleIndex - 1; i >= 0; i -= 1) {
if (isHandleDisabled(i)) {
candidates.push(values[i] + gap);
break; // 取最近的即可
}
}
return Math.max(...candidates);
};这样语义更清晰，也更容易扩展（比如将来支持 step 约束等）。
zombieJ
reviewed
Apr 17, 2026
| if (typeof rawDisabled === 'boolean') { | ||
| return rawDisabled; | ||
| } | ||
| return rawDisabled[index] || false; |
Member
There was a problem hiding this comment.
这里有单个 disabled 的返回单个，没有就 fallback 成全局的 disabled
zombieJ
reviewed
Apr 17, 2026
| focusIndex = valueBeforeIndex + 1; | ||
| } else { | ||
| cloneNextValues[valueIndex] = newValue; | ||
| if (isHandleDisabled(valueIndex)) { |
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
2 participants
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.
功能说明
本 PR 新增了两个功能：
关联issue
ant-design/ant-design#10143
使用示例
解决的问题
在 editable 模式下，当添加或删除点位时：
测试覆盖
Summary by CodeRabbit
新功能
disabled可为 boolean 或 boolean[]onDisabledChange回调，用于在句柄增删时同步禁用数组行为改进
文档
样式
测试