feat: support per-handle disabled for Range slider#1069
feat: support per-handle disabled for Range slider#1069EmilyyyLiu wants to merge 23 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. |
|
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.52% -0.88%
==========================================
Files 14 15 +1
Lines 661 745 +84
Branches 191 225 +34
==========================================
+ Hits 657 734 +77
- Misses 4 11 +7 ☔ 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}
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
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
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
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
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
|
@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.
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
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
- 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>
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>
| focusIndex = valueBeforeIndex + 1; | ||
| } else { | ||
| cloneNextValues[valueIndex] = newValue; | ||
| if (isHandleDisabled(valueIndex)) { |
There was a problem hiding this comment.
还是太啰嗦了，原来的逻辑是 cloneNextValues[valueIndex] = newValue。他做的就是把 valueIndex 匹配的换成对应的值。你这里即便有 disabled 的。也应该很简单的直接看这个范围内适合的 valueIndex。不应该这么麻烦。
…dback - Handle.tsx: rename handleDisabled to mergedDisabled, remove globalDisabled - Slider.tsx: use ?? operator for fallback to global disabled state - Slider.tsx: simplify boundary calculation with functional style - useOffset.ts: treat disabled handles as fixed anchors with candidates pattern - useOffset.ts: handle dead-lock case when minBound > maxBound Co-Authored-By: zombieJ <zombieJ@users.noreply.github.com>
EmilyyyLiu
changed the title
- Extract disabled logic into useDisabled hook - Add type annotations for TypeScript inference - Extract gap variable in useOffset to avoid duplication Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Simplify boundary calculation with for loop instead of map/filter chain - Unify disabled array handling in useDisabled hook to avoid repetitive checks - Remove duplicate disabledIsBoolean checks by converting upfront 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@gemini-code-assist, CR again |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to disable specific handles in a range slider by allowing the disabled prop to accept a boolean array. It updates handle interaction logic to treat disabled handles as fixed boundaries and restricts track interactions when handles are disabled. Feedback recommends using type guards instead of assertions in examples for better safety and optimizing the track component's disabled handle check for improved readability and dependency management.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request enables disabling specific handles in the Slider component by allowing the disabled prop to accept a boolean array. It introduces a useDisabled hook and updates interaction logic to treat disabled handles as fixed boundaries. Additionally, editable mode is now disabled if any handle is marked as disabled. Review feedback identifies a redundant check in the onDelete handler and suggests adding boundary validation to the click-to-move logic for consistency when handles are locked between disabled anchors.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
When a handle is positioned between two disabled handles with insufficient space for the required pushable gap, it's now kept at its original position instead of being forced to a boundary value. This ensures consistency with the drag behavior in useOffset.ts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@gemini-code-assist, CR again |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to disable specific handles in the Slider component by allowing the disabled prop to accept an array of booleans. The implementation includes a new useDisabled hook, updated interaction logic for handles and tracks, and boundary enforcement in the useOffset hook to treat disabled handles as fixed anchors. Review feedback highlights opportunities to optimize performance by stabilizing context references to prevent excessive re-renders during drag operations and suggests consolidating boundary calculation logic to reduce duplication.
|
@gemini-code-assist, CR again |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to disable specific handles in the Slider component by allowing the disabled prop to accept an array of booleans. It includes updates to the handle interaction logic, keyboard accessibility, and boundary calculations to treat disabled handles as fixed anchors. Additionally, it disables editable mode and track dragging when any handle is disabled. The changes are supported by new documentation, examples, and comprehensive test cases. I have no feedback to provide.
| mergedValue?: number | number[], | ||
| ): [boolean, boolean] => { | ||
|
|
||
| const values = React.useMemo( |
There was a problem hiding this comment.
Slider 里有聚合 rawValues，这里应该是直接消费 rawValues 而不是重新再算一次。
| const originValue = nextValues[valueIndex]; | ||
|
|
||
| const minBound = getHandleMinBound(nextValues, valueIndex); | ||
| const maxBound = getHandleMaxBound(nextValues, valueIndex); |
There was a problem hiding this comment.
getHandleXXXBound 逻辑太复杂了，你可以直接按照原来的逻辑进行 offset，然后比较一下新的 values 里是否有被 disabled 的节点变了 value。如果有那就略过就好了。现在先获取边界再去做处理逻辑绕太远了。
2 participants
功能说明
本 PR 新增了 per-handle disabled 功能，允许在 Range 滑块中单独禁用特定的 handle：
disabled支持数组类型，如[true, false, true]可禁用第1和第3个 handle使用示例
关联 issue
ant-design/ant-design#10143
测试覆盖