Skip to content

Conversation

@soniaboller
Copy link

PR Summary

  • This PR adds the scaffolding and spec for a new RangeSlider component
  • This is a separate control from current range-slider because it does not support multi values.

@github-actions
Copy link

github-actions bot commented Oct 31, 2025

📊 Bundle size report

✅ No changes found

@github-actions
Copy link

Pull request demo site: URL

@soniaboller soniaboller marked this pull request as ready for review November 10, 2025 19:13
@mainframev
Copy link
Contributor

mainframev commented Nov 25, 2025

@soniaboller

We also discussed this within the team and agreed that it would be better to add this to existing Slider component. So you better will be to create a new feature branch and change this PR to target that branch instead of master, locally reset this commit changes except the spec file, then run yarn create-component to add RangeSlider to react-slider to create a boilerplate code for new component

UPD: I forgot that create-component is intended for preview packages only. You can resolve the lint errors by adding the necessary exports to the react-components suite package and updating the story imports to pull from react-components

@mainframev mainframev changed the base branch from master to react-range-slider November 25, 2025 23:46
@soniaboller soniaboller requested a review from a team as a code owner December 1, 2025 17:13
@soniaboller soniaboller force-pushed the feat/react-range-slider-preview-scaffold branch from 9301b55 to 4065fac Compare December 1, 2025 18:46
@soniaboller soniaboller force-pushed the feat/react-range-slider-preview-scaffold branch from 4065fac to 9301b55 Compare December 1, 2025 18:50
@github-actions github-actions bot removed the NX: core label Dec 1, 2025
@mainframev mainframev deleted the branch microsoft:react-range-slider December 1, 2025 22:19
@mainframev mainframev closed this Dec 1, 2025
@mainframev mainframev reopened this Dec 1, 2025
@soniaboller soniaboller force-pushed the feat/react-range-slider-preview-scaffold branch from 9301b55 to 30ad97e Compare December 1, 2025 22:27
Copy link
Contributor

@mainframev mainframev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I'd suggest to revisit is the accessibility shape of the DOM. In the proposed markup, the input type="range" elements are aria-hidden and tabindex="-1", and the thumbs are exposed as div role="slider". Let's check this approach

Comment on lines +146 to +149
### Slider Open UI Research

- <https://open-ui.org/components/slider.research.parts>
- <https://open-ui.org/components/slider.research>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's common for both slider components, let's use separator and add it on the bottom of the document after both slider sections

Suggested change
### Slider Open UI Research
- <https://open-ui.org/components/slider.research.parts>
- <https://open-ui.org/components/slider.research>

- `pointermove` (or touch move) continues dragging the active thumb.
- Anti-crossing ensures thumbs never swap order. Collision is handled by clamping the active thumb to the other thumb's position.

#### Screen readers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should also mention here that when vertical is true we expose also aria-orientation:

Comment on lines +297 to +298
| `startInput` | none | Hidden (`aria-hidden="true"`, `tabindex="-1"`). Mirrors the start value for forms. |
| `endInput` | none | Hidden (`aria-hidden="true"`, `tabindex="-1"`). Mirrors the end value. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `startInput` | none | Hidden (`aria-hidden="true"`, `tabindex="-1"`). Mirrors the start value for forms. |
| `endInput` | none | Hidden (`aria-hidden="true"`, `tabindex="-1"`). Mirrors the end value. |
| `startInput` | none | Hidden (`aria-hidden="true"`, `tabindex="-1"`). Mirrors the start value for forms. |
| `endInput` | none | Hidden (`aria-hidden="true"`, `tabindex="-1"`). Mirrors the end value for forms. |

Comment on lines +231 to +232
<input type="range" aria-hidden="true" tabindex="-1" value="20" />
<input type="range" aria-hidden="true" tabindex="-1" value="80" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is no longer a control, hidden from AT, not focusable and visible

Suggested change
<input type="range" aria-hidden="true" tabindex="-1" value="20" />
<input type="range" aria-hidden="true" tabindex="-1" value="80" />
<input type="hidden" tabindex="-1" value="20" />
<input type="hidden" tabindex="-1" value="80" />

class="fui-RangeSlider__startThumb"
role="slider"
tabindex="0"
aria-labelledby="label-123"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soniaboller

I guess this DOM output should include visible label, otherwise instead of aria-labelledby here should be aria-label

Comment on lines +196 to +197
| `startInput` | `input` | Hidden `<input type="range">` mirroring the start value for form submissions and pointer/touch selection. |
| `endInput` | `input` | Hidden `<input type="range">` mirroring the end value. Receives most native props passed to `<RangeSlider>`. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| `startInput` | `input` | Hidden `<input type="range">` mirroring the start value for form submissions and pointer/touch selection. |
| `endInput` | `input` | Hidden `<input type="range">` mirroring the end value. Receives most native props passed to `<RangeSlider>`. |
| `startInput` | `input` | Hidden input mirroring the start value for form submissions and pointer/touch selection. |
| `endInput` | `input` | Hidden input mirroring the end value. Receives most native props passed to `<RangeSlider>`. |


## Slider Component

### Slider Variants
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you do not need to explicitly have Slider / RangeSlider for each heading. Second level headings for Slider and RangeSlider sections should be enough

aria-valuemin="20"
aria-valuemax="100"
aria-valuenow="80"
aria-valuetext="80"
Copy link
Contributor

@mainframev mainframev Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered an alternative approach where the native elements remain the actual interactive controls, but are visually hidden using SR only styles, with the thumbs being purely visual?

Instead of using aria-hidden on real inputs and recreating slider behavior with div role="slider", the real inputs stay accessible and focusable, and only their visuals are replaced.

.fui-RangeSlider__srOnlyInput {
  clip: rect(0 0 0 0);
  clip-path: inset(50%);
  width: 1px;
  height: 1px;
  overflow: hidden;
  position: absolute;
  border: 0;
  padding: 0;
  margin: 0;
}
<div class="fui-RangeSlider">
  <div className="fui-RangeSlider__rail" />
  <div className="fui-RangeSlider__track" />
  <div class="fui-RangeSlider__startThumb">
    <!-- visual thumb handle goes here (e.g. a div or pseudo-element) -->
    <input
      class="fui-RangeSlider__srOnlyInput"
      type="range"
      min="0"
      max="100"
      step="1"
      value="20"
      aria-labelledby="label-1"
      aria-valuetext="20"
    />
  </div>
  <div class="fui-RangeSlider__endThumb">
    <input
      class="fui-RangeSlider__srOnlyInput"
      type="range"
      min="0"
      max="100"
      step="1"
      value="80"
      aria-labelledby="label-2"
      aria-valuetext="80"
    />
  </div>
</div>

the native inputs remain the source of truth for accessibility and keyboard interaction, while the custom thumbs and track are purely presentational

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants