feat(SliderArea): add SliderArea component#2460
Conversation
… call expectations
…s for 2D slider functionality
📝 WalkthroughWalkthroughThis PR introduces the SliderArea component—a 2D slider for selecting (x, y) coordinates via dragging one or more thumbs in a rectangular area. It includes four main Vue components (Root, Area, Region, Thumb), comprehensive accessibility support, keyboard and pointer interaction handling, form integration, documentation, story variants, and utility functions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SliderAreaThumb as Thumb
participant SliderAreaRoot as Root
participant Context as Root Context
User->>SliderAreaThumb: Pointer Down on Thumb
SliderAreaThumb->>Root: handlePointerDown
Root->>Root: getClosestThumbIndex
Root->>Root: getPointFromPointerEvent
Root->>Context: Update valueIndexToChangeRef
Root->>User: Focus Thumb
User->>SliderAreaThumb: Pointer Move
SliderAreaThumb->>Root: handlePointerMove
Root->>Root: getPointFromPointerEvent
Root->>Root: clampAxis + updateValues
Root->>Context: Update modelValue (continuous)
User->>SliderAreaThumb: Pointer Up
SliderAreaThumb->>Root: handlePointerUp
Root->>Root: updateValues
Root->>Context: Update modelValue
Root->>User: Emit valueCommit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@docs/components/demo/SliderArea/tailwind/index.vue`:
- Around line 20-26: The aria-label on the SliderAreaThumb component is
incorrectly set to "Volume" for a generic 2D slider demo; update the aria-label
value on the SliderAreaThumb element (the attribute in the SliderAreaThumb
component instance) to a neutral, descriptive label such as "2D Slider" or
"Position" to accurately reflect its purpose in the demo and improve
accessibility.
In `@docs/content/docs/components/slider-area.md`:
- Around line 348-354: Add a short clarifying note in the Keyboard Interactions
section of the SliderArea docs explaining that SliderArea uses screen-coordinate
convention by default (Y increases downward) for its 2D use case, which is why
ArrowDown increases Y and ArrowUp decreases Y, and mention that the invertedY
prop on SliderArea can be set to true to invert the vertical axis to match the
WAI-ARIA vertical slider semantics if users need that behavior; update the text
near the ArrowDown/ArrowUp rows and/or add a single-sentence callout referencing
SliderArea and invertedY to keep the Accessibility/WAI-ARIA statement accurate.
In `@docs/content/meta/SliderAreaRoot.md`:
- Around line 3-10: Change the default for the "as" prop in the SliderAreaRoot
component from 'span' to a block-level element 'div' to avoid invalid HTML
nesting; update the prop default in the source component SliderAreaRoot
(packages/core/src/SliderArea/SliderAreaRoot.vue) so the props declaration for
"as" sets default: 'div' (the prop name is "as" in SliderAreaRoot) to match
other container components like Calendar/NumberField/Editable.
In `@packages/core/src/SliderArea/SliderAreaThumb.vue`:
- Around line 55-75: The ARIA attributes aria-roledescription and aria-label are
currently placed on SliderAreaThumb's wrapper Primitive (which has no role),
causing incorrect screen reader announcements; remove aria-label and
aria-roledescription from the Primitive in SliderAreaThumb and instead add
aria-label and aria-roledescription (or at least aria-label) to each focusable
child component SliderAreaThumbX and SliderAreaThumbY (the elements that set
role="slider"), ensuring aria-orientation remains on those child sliders; update
the binding that currently uses ($attrs['aria-label'] as string) || label so the
children receive the resolved label (or fallback) and do not rely on the parent
Primitive for ARIA attributes.
🧹 Nitpick comments (11)
packages/core/src/SliderArea/SliderArea.test.ts (3)
9-18: Duplicated global mocks could be extracted to a shared setup.The
ResizeObserverand pointer capture method mocks are duplicated verbatim between the two top-leveldescribeblocks (lines 10–18 and lines 377–385). Consider extracting them to a file-levelbeforeAllor a shared helper to reduce duplication.♻️ Proposed refactor: hoist mocks to file level
+// File-level setup — shared across all describe blocks +globalThis.ResizeObserver = class ResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} +window.HTMLElement.prototype.scrollIntoView = vi.fn() +window.HTMLElement.prototype.hasPointerCapture = vi.fn().mockImplementation(id => id) +window.HTMLElement.prototype.releasePointerCapture = vi.fn() +window.HTMLElement.prototype.setPointerCapture = vi.fn() + describe('given default SliderArea', () => { - globalThis.ResizeObserver = class ResizeObserver { - observe() {} - unobserve() {} - disconnect() {} - } - window.HTMLElement.prototype.scrollIntoView = vi.fn() - window.HTMLElement.prototype.hasPointerCapture = vi.fn().mockImplementation(id => id) - window.HTMLElement.prototype.releasePointerCapture = vi.fn() - window.HTMLElement.prototype.setPointerCapture = vi.fn() ... }) describe('given slider area in a form', () => { - globalThis.ResizeObserver = class ResizeObserver { - observe() {} - unobserve() {} - disconnect() {} - } - window.HTMLElement.prototype.scrollIntoView = vi.fn() - window.HTMLElement.prototype.hasPointerCapture = vi.fn().mockImplementation(id => id) - window.HTMLElement.prototype.releasePointerCapture = vi.fn() - window.HTMLElement.prototype.setPointerCapture = vi.fn() ... })Also applies to: 376-385
303-325: Consider asserting the emittedvalueCommitpayload, not just its presence.The pointer interaction test verifies that
valueCommitis emitted with one argument (line 321), but doesn't assert what value was committed. Adding a payload assertion would strengthen this test and guard against regressions in the committed value.
26-33: Note: axe test disables thelabelrule.The accessibility test disables the
labelrule. This is acceptable for a story/demo component, but please ensure that the documentation or real-world usage examples demonstrate proper labeling for the SliderArea (e.g., viaaria-labelor associated<label>elements) so consumers get it right.packages/core/src/SliderArea/SliderAreaRegion.vue (1)
53-67: Verify that the parent track element hasposition: relative.The region uses
position: absolutein its inline style, which requires a positioned ancestor. The demo CSS setsposition: relativeon.SliderAreaTrack, but consumers using custom styles or theas-childpattern on the track could lose this. Consider documenting this requirement or adding a note in the SliderAreaRegion docs.docs/components/demo/SliderArea/css/index.vue (1)
17-23: Consider a more descriptivearia-labelfor the demo."Volume" suggests a 1D control. Since this demonstrates a 2D slider area, a label like "Color picker" or "Position" would better illustrate the component's purpose to users reading the docs.
docs/content/docs/components/slider-area.md (1)
403-403: Minor wording nit.Per static analysis: "abstracts all of the" → "abstracts all the" for conciseness.
Proposed fix
-This example abstracts all of the `SliderArea` parts so it can be used as a self-closing element. +This example abstracts all the `SliderArea` parts so it can be used as a self-closing element.packages/core/src/SliderArea/story/_SliderArea.vue (1)
18-34:v-modeloverridesmodelValuefrom forwarded props.
v-bind="forwarded"spreads all props (includingmodelValueif passed by parent), butv-model="sliderValue"immediately overrides it with local state. This means any externally passedmodelValueis silently ignored. Since this is a story-internal component, it's likely intentional — just flagging for awareness.packages/core/src/SliderArea/SliderAreaImpl.vue (1)
51-54:PAGE_KEYSin the concat is redundant.
PageUpandPageDownare already handled by the earlierelse ifbranches (lines 43–50), so they can never reach this branch. ThePAGE_KEYS.concat(ARROW_KEYS)effectively matches only arrow keys here.Suggested simplification
- else if (PAGE_KEYS.concat(ARROW_KEYS).includes(event.key)) { + else if (ARROW_KEYS.includes(event.key)) {packages/core/src/SliderArea/SliderAreaRoot.vue (3)
132-136:isSlidingFromLeftlogic is correct but hard to parse.The double-negative style (
!== 'rtl',!== 'ltr') makes the truth table non-obvious at a glance. Consider simplifying to an XOR-style expression.Optional: clearer expression
const isSlidingFromLeft = computed(() => { - return (dir.value !== 'rtl' && !props.invertedX) || (dir.value !== 'ltr' && props.invertedX) + return (dir.value === 'ltr') !== props.invertedX })This reads as: "slide from left when LTR and not inverted, or RTL and inverted" — i.e., inversion flips the direction.
224-245: Deep-comparison viaJSON.stringifyis adequate here but worth noting.
JSON.stringifyfor change detection (line 236) works for small arrays of number pairs. For this use case it's fine, but if the component ever supports large numbers of thumbs, a manual comparison would be more efficient.
336-348: VerifyvaluesBeforeSlideStartRefcaptures a sufficiently deep snapshot.Line 337:
valuesBeforeSlideStartRef = currentModelValueassigns the shallow-copied array from the computed ([...modelValue.value]). The inner[x, y]arrays are shared references withmodelValue. This is safe only becauseupdateValues(line 232) replaces inner arrays with new[finalX, finalY]rather than mutating them in place. If that invariant is ever broken,handleSlideEnd's change detection (line 210) would silently fail.Consider a defensive deep copy to decouple from this implicit invariant:
Optional: defensive deep copy
`@pointerdown`="() => { - if (!disabled) valuesBeforeSlideStartRef = currentModelValue + if (!disabled) valuesBeforeSlideStartRef = currentModelValue.map(p => [...p]) }"
| <template> | ||
| <CollectionItem> | ||
| <Primitive | ||
| v-bind="$attrs" | ||
| :ref="forwardRef" | ||
| :aria-label="($attrs['aria-label'] as string) || label" | ||
| :data-disabled="rootContext.disabled.value ? '' : undefined" | ||
| aria-roledescription="2D slider" | ||
| :as-child="asChild" | ||
| :as="as" | ||
| :style="{ | ||
| transform: 'var(--reka-slider-area-thumb-transform)', | ||
| position: 'absolute', | ||
| [rootContext.isSlidingFromLeft.value ? 'left' : 'right']: `calc(${percentX}% + ${thumbInBoundsOffsetX}px)`, | ||
| [rootContext.isSlidingFromTop.value ? 'top' : 'bottom']: `calc(${percentY}% + ${thumbInBoundsOffsetY}px)`, | ||
| display: !isMounted && value === undefined ? 'none' : undefined, | ||
| }" | ||
| > | ||
| <slot /> | ||
| </Primitive> | ||
| </CollectionItem> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd 'SliderAreaThumbX\.vue|SliderAreaThumbY\.vue'Repository: unovue/reka-ui
Length of output: 157
🏁 Script executed:
cat packages/core/src/SliderArea/SliderAreaThumbX.vue
cat packages/core/src/SliderArea/SliderAreaThumbY.vueRepository: unovue/reka-ui
Length of output: 3717
Move ARIA attributes to the axis-specific child components.
The aria-roledescription and aria-label are currently on SliderAreaThumb (the parent wrapper), which has no role attribute. However, the actual focusable slider elements are SliderAreaThumbX and SliderAreaThumbY, which have role="slider" but no labels or role descriptions.
ARIA attributes like aria-roledescription should be on elements with semantic roles. This structure causes screen readers to announce the label on an invisible wrapper, then announce two separate sliders without context.
Move aria-label to both child components, or provide a single label on the parent if these are meant as a unified control. Add aria-roledescription to describe the 2D behavior on the child components or consider removing it entirely since each axis is already described as "horizontal"/"vertical" via aria-orientation.
🤖 Prompt for AI Agents
In `@packages/core/src/SliderArea/SliderAreaThumb.vue` around lines 55 - 75, The
ARIA attributes aria-roledescription and aria-label are currently placed on
SliderAreaThumb's wrapper Primitive (which has no role), causing incorrect
screen reader announcements; remove aria-label and aria-roledescription from the
Primitive in SliderAreaThumb and instead add aria-label and aria-roledescription
(or at least aria-label) to each focusable child component SliderAreaThumbX and
SliderAreaThumbY (the elements that set role="slider"), ensuring
aria-orientation remains on those child sliders; update the binding that
currently uses ($attrs['aria-label'] as string) || label so the children receive
the resolved label (or fallback) and do not rely on the parent Primitive for
ARIA attributes.
|
@GrandMagus02 can you check the comment from coderabbit? |
commit: |
Yeah, I see color components being added, but I think SliderArea still has a place. My implementation design is a bit different so I will update code to use single thumb for both X & Y |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/src/SliderArea/SliderArea.test.ts (1)
9-17: Extract and centralize global mock setup to reduce drift.Line 9 and Line 337 duplicate the same global/prototype stubs. Consolidating this into a shared helper (and restoring in teardown) will make the suite less brittle and easier to maintain.
Also applies to: 337-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/SliderArea/SliderArea.test.ts` around lines 9 - 17, Extract the duplicated global/prototype stubs (globalThis.ResizeObserver and window.HTMLElement.prototype methods: scrollIntoView, hasPointerCapture, releasePointerCapture, setPointerCapture) from SliderArea.test.ts into a centralized test helper (e.g., a shared setup/teardown utility used by tests); replace the duplicated blocks in SliderArea.test.ts (lines around the ResizeObserver and the prototype stubs) with calls to that helper, and ensure the helper registers the mocks in a beforeAll/beforeEach and restores the originals in an afterAll/afterEach so tests don’t leak mocked globals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/SliderArea/SliderAreaRoot.vue`:
- Around line 314-325: The pointerdown handler currently checks exact equality
with thumbElements via thumbElements.value.includes(target), which misses clicks
on nested elements inside a thumb and incorrectly calls handleSlideStart; change
the check to detect descendant hits (e.g., use Element.closest or check
thumbElements.value.some(thumb => thumb.contains(target))) so that pointer
events on thumb children focus the correct thumb and avoid calling
handleSlideStart; keep the setPointerCapture(event.pointerId),
valuesBeforeSlideStartRef.value = currentModelValue.value, and the focus
behavior for the resolved thumb element.
In `@packages/core/src/SliderArea/SliderAreaThumb.vue`:
- Around line 49-57: The cleanup in onUnmounted can fail because
currentElement.value may be null by unmount time; capture the element reference
during onMounted into a local variable (e.g., mountedEl) and push that captured
value into rootContext.thumbElements.value, then in onUnmounted use that
captured mountedEl (not currentElement.value) to findIndex and splice it out of
rootContext.thumbElements.value so removal is deterministic; update references
to currentElement in the mount/unmount pair accordingly.
---
Nitpick comments:
In `@packages/core/src/SliderArea/SliderArea.test.ts`:
- Around line 9-17: Extract the duplicated global/prototype stubs
(globalThis.ResizeObserver and window.HTMLElement.prototype methods:
scrollIntoView, hasPointerCapture, releasePointerCapture, setPointerCapture)
from SliderArea.test.ts into a centralized test helper (e.g., a shared
setup/teardown utility used by tests); replace the duplicated blocks in
SliderArea.test.ts (lines around the ResizeObserver and the prototype stubs)
with calls to that helper, and ensure the helper registers the mocks in a
beforeAll/beforeEach and restores the originals in an afterAll/afterEach so
tests don’t leak mocked globals.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88afc80e-3f40-4b2b-bc99-3d62bcdbeaf2
📒 Files selected for processing (15)
docs/components/demo/SliderArea/css/index.vuedocs/components/demo/SliderArea/css/styles.cssdocs/components/demo/SliderArea/tailwind/index.vuedocs/content/docs/components/slider-area.mdpackages/core/constant/components.tspackages/core/src/SliderArea/SliderArea.test.tspackages/core/src/SliderArea/SliderAreaArea.vuepackages/core/src/SliderArea/SliderAreaRoot.vuepackages/core/src/SliderArea/SliderAreaThumb.vuepackages/core/src/SliderArea/index.tspackages/core/src/SliderArea/story/SliderAreaChromatic.story.vuepackages/core/src/SliderArea/story/SliderAreaDemo.story.vuepackages/core/src/SliderArea/story/SliderAreaExtremes.story.vuepackages/core/src/SliderArea/story/_SliderArea.vuepackages/core/src/SliderArea/utils.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- docs/components/demo/SliderArea/css/index.vue
- packages/core/src/SliderArea/story/_SliderArea.vue
- packages/core/src/SliderArea/story/SliderAreaExtremes.story.vue
- docs/components/demo/SliderArea/css/styles.css
- packages/core/src/SliderArea/story/SliderAreaChromatic.story.vue
- packages/core/src/SliderArea/story/SliderAreaDemo.story.vue
- packages/core/src/SliderArea/utils.ts
- docs/components/demo/SliderArea/tailwind/index.vue
| const target = event.target as HTMLElement | ||
| target.setPointerCapture(event.pointerId) | ||
| event.preventDefault() | ||
|
|
||
| valuesBeforeSlideStartRef.value = currentModelValue.value | ||
|
|
||
| if (thumbElements.value.includes(target)) { | ||
| target.focus() | ||
| } | ||
| else { | ||
| handleSlideStart(event) | ||
| } |
There was a problem hiding this comment.
Handle pointerdown on thumb descendants, not only exact thumb elements.
At Line 320, using includes(target) misses cases where the pointer lands on nested content inside a thumb. That path falls into handleSlideStart, which can cause jumpy behavior instead of dragging the intended thumb.
Proposed fix
function handlePointerDown(event: PointerEvent) {
if (disabled.value)
return
- const target = event.target as HTMLElement
- target.setPointerCapture(event.pointerId)
+ const target = event.target as HTMLElement
+ const captureEl = event.currentTarget as HTMLElement
+ captureEl.setPointerCapture(event.pointerId)
event.preventDefault()
valuesBeforeSlideStartRef.value = currentModelValue.value
- if (thumbElements.value.includes(target)) {
- target.focus()
+ const thumbIndex = thumbElements.value.findIndex(el => el === target || el.contains(target))
+ if (thumbIndex >= 0) {
+ valueIndexToChangeRef.value = thumbIndex
+ thumbElements.value[thumbIndex]?.focus()
}
else {
handleSlideStart(event)
}
}📝 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 target = event.target as HTMLElement | |
| target.setPointerCapture(event.pointerId) | |
| event.preventDefault() | |
| valuesBeforeSlideStartRef.value = currentModelValue.value | |
| if (thumbElements.value.includes(target)) { | |
| target.focus() | |
| } | |
| else { | |
| handleSlideStart(event) | |
| } | |
| const target = event.target as HTMLElement | |
| const captureEl = event.currentTarget as HTMLElement | |
| captureEl.setPointerCapture(event.pointerId) | |
| event.preventDefault() | |
| valuesBeforeSlideStartRef.value = currentModelValue.value | |
| const thumbIndex = thumbElements.value.findIndex(el => el === target || el.contains(target)) | |
| if (thumbIndex >= 0) { | |
| valueIndexToChangeRef.value = thumbIndex | |
| thumbElements.value[thumbIndex]?.focus() | |
| } | |
| else { | |
| handleSlideStart(event) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/SliderArea/SliderAreaRoot.vue` around lines 314 - 325, The
pointerdown handler currently checks exact equality with thumbElements via
thumbElements.value.includes(target), which misses clicks on nested elements
inside a thumb and incorrectly calls handleSlideStart; change the check to
detect descendant hits (e.g., use Element.closest or check
thumbElements.value.some(thumb => thumb.contains(target))) so that pointer
events on thumb children focus the correct thumb and avoid calling
handleSlideStart; keep the setPointerCapture(event.pointerId),
valuesBeforeSlideStartRef.value = currentModelValue.value, and the focus
behavior for the resolved thumb element.
| onMounted(() => { | ||
| if (currentElement.value) | ||
| rootContext.thumbElements.value.push(currentElement.value) | ||
| }) | ||
| onUnmounted(() => { | ||
| const i = rootContext.thumbElements.value.findIndex(el => el === currentElement.value) | ||
| if (i >= 0) | ||
| rootContext.thumbElements.value.splice(i, 1) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vue 3, are template refs guaranteed to still reference the DOM node inside onUnmounted, or can they already be null by that hook?
💡 Result:
In Vue 3, template refs are not guaranteed to still point at a DOM node in onUnmounted(). By the time onUnmounted runs, the component is already unmounted, and template refs can already have been cleared to null (Vue explicitly notes refs can become null when the referenced element is unmounted, e.g. via v-if) [2]. The hook intended for “still fully functional” teardown work is onBeforeUnmount() [1].
Practical rule: if you must read the element (measurements, final DOM state, etc.), do it in onBeforeUnmount; in onUnmounted, assume the ref may be null [1][2].
🏁 Script executed:
cat -n packages/core/src/SliderArea/SliderAreaThumb.vue | sed -n '40,70p'Repository: unovue/reka-ui
Length of output: 1262
🏁 Script executed:
cat -n packages/core/src/SliderArea/SliderAreaThumb.vue | sed -n '1,48p'Repository: unovue/reka-ui
Length of output: 2443
🏁 Script executed:
rg -A 10 "export.*useForwardExpose" packages/core/src --type ts --type vueRepository: unovue/reka-ui
Length of output: 85
🏁 Script executed:
rg -A 10 "export.*useForwardExpose" packages/core/src --type tsRepository: unovue/reka-ui
Length of output: 2001
🏁 Script executed:
cat packages/core/src/shared/useForwardExpose.tsRepository: unovue/reka-ui
Length of output: 3316
🏁 Script executed:
rg "thumbElements" packages/core/src/SliderArea --type ts -B 2 -A 2Repository: unovue/reka-ui
Length of output: 40
🏁 Script executed:
rg "thumbElements" packages/core/src --type ts -B 2 -A 2Repository: unovue/reka-ui
Length of output: 40
🏁 Script executed:
find packages/core/src -name "*.ts" -o -name "*.vue" | xargs grep -l "thumbElements"Repository: unovue/reka-ui
Length of output: 369
🏁 Script executed:
cat packages/core/src/SliderArea/SliderAreaRoot.vue | grep -n "thumbElements" -B 5 -A 5Repository: unovue/reka-ui
Length of output: 1999
🏁 Script executed:
rg "currentElement\.value\s*=" packages/core/src/SliderArea/SliderAreaThumb.vueRepository: unovue/reka-ui
Length of output: 40
Capture the element reference during mount to ensure deterministic cleanup.
In Vue 3, template refs can become null by the time onUnmounted runs because the component is already unmounted. The current cleanup path at line 54 depends on currentElement.value still pointing to the mounted node, but if that ref has been cleared to null, the findIndex comparison fails and the element is never removed from rootContext.thumbElements. This causes stale entries to accumulate.
Proposed fix
+import { computed, onMounted, onUnmounted, ref } from 'vue'
...
+const mountedThumbEl = ref<HTMLElement | null>(null)
+
onMounted(() => {
- if (currentElement.value)
- rootContext.thumbElements.value.push(currentElement.value)
+ mountedThumbEl.value = currentElement.value ?? null
+ if (mountedThumbEl.value)
+ rootContext.thumbElements.value.push(mountedThumbEl.value)
})
onUnmounted(() => {
- const i = rootContext.thumbElements.value.findIndex(el => el === currentElement.value)
+ const i = rootContext.thumbElements.value.findIndex(el => el === mountedThumbEl.value)
if (i >= 0)
rootContext.thumbElements.value.splice(i, 1)
})📝 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.
| onMounted(() => { | |
| if (currentElement.value) | |
| rootContext.thumbElements.value.push(currentElement.value) | |
| }) | |
| onUnmounted(() => { | |
| const i = rootContext.thumbElements.value.findIndex(el => el === currentElement.value) | |
| if (i >= 0) | |
| rootContext.thumbElements.value.splice(i, 1) | |
| }) | |
| const mountedThumbEl = ref<HTMLElement | null>(null) | |
| onMounted(() => { | |
| mountedThumbEl.value = currentElement.value ?? null | |
| if (mountedThumbEl.value) | |
| rootContext.thumbElements.value.push(mountedThumbEl.value) | |
| }) | |
| onUnmounted(() => { | |
| const i = rootContext.thumbElements.value.findIndex(el => el === mountedThumbEl.value) | |
| if (i >= 0) | |
| rootContext.thumbElements.value.splice(i, 1) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/SliderArea/SliderAreaThumb.vue` around lines 49 - 57, The
cleanup in onUnmounted can fail because currentElement.value may be null by
unmount time; capture the element reference during onMounted into a local
variable (e.g., mountedEl) and push that captured value into
rootContext.thumbElements.value, then in onUnmounted use that captured mountedEl
(not currentElement.value) to findIndex and splice it out of
rootContext.thumbElements.value so removal is deterministic; update references
to currentElement in the mount/unmount pair accordingly.
2 participants
🔗 Linked issue
#2455
❓ Type of change
📚 Description
Implemented the new SliderArea component set, enabling a fully accessible 2D slider for selecting (x, y) values via one or multiple thumbs within a rectangular track.
This adds the core primitives (
SliderAreaRoot,SliderAreaTrack,SliderAreaRegion,SliderAreaThumb,SliderAreaThumbX,SliderAreaThumbY) with controlled and uncontrolled value support, keyboard interactions, pointer/touch dragging, RTL handling, disabled state, stepping, and optional axis inversion—following the existing Slider patterns and API conventions.Resolves #2455
📸 Screenshots (if appropriate)
📝 Checklist
Summary by CodeRabbit
Release Notes
New Features
Documentation