Skip to content

Conversation

@behowell
Copy link
Contributor

@behowell behowell commented Oct 25, 2022

Current Behavior

The thumb of Slider extends beyond the bounds of its root, which causes it to appear misaligned with a label or other form controls:
image

New Behavior

Add padding to the edges of slider, so the track is now inset by half of the thumb width. The edges of the thumb will never extend past the bounds of the root element.

image

Preview: https://fluentuipr.z22.web.core.windows.net/pull/25378/public-docsite-v9/storybook/index.html?path=/docs/components-slider--default

Related Issue(s)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 25, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 0ed7414:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 25, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1278 1289 5000
Button mount 931 921 5000
FluentProvider mount 1508 1487 5000
FluentProviderWithTheme mount 583 584 10
FluentProviderWithTheme virtual-rerender 546 546 10
FluentProviderWithTheme virtual-rerender-with-unmount 570 578 10
MakeStyles mount 1957 1942 50000
SpinButton mount 2346 2362 5000

@size-auditor
Copy link

size-auditor bot commented Oct 25, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 870f51fc39736198871559d6e12ece5663b0525f (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 25, 2022

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-slider
Slider
31.805 kB
10.085 kB
31.978 kB
10.149 kB
173 B
64 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
62.94 kB
17.663 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
189.603 kB
52.909 kB
react-components
react-components: FluentProvider & webLightTheme
33.446 kB
11.033 kB
react-portal-compat
PortalCompatProvider
5.857 kB
1.978 kB
🤖 This report was generated against 870f51fc39736198871559d6e12ece5663b0525f

@behowell behowell marked this pull request as ready for review October 26, 2022 22:03
@behowell behowell requested review from a team and micahgodbolt as code owners October 26, 2022 22:03
@spmonahan
Copy link
Contributor

I see two small issues with Screener.

  1. The vertical RTL story is cut off. This was case before this PR so I don't think it's a blocker, though it should be fixed.
    slider_vertical_rtl
  2. None of the VR tests actually verify the change. This issue is only apparent when the slider thumb is at the left-most values (in LTR; right-most in RTL) but all of the VR tests have a value toward the middle. I think we should add a test where the slider value is such that the thumb would show this issue were it not fixed.

@behowell behowell requested a review from a team as a code owner October 27, 2022 17:43
@behowell
Copy link
Contributor Author

@spmonahan I fixed the VR tests, and added some 0% and 100% ones as well, to test this particular change. The fixes to the VR tests resulted in some extra test-only padding changes.

@behowell behowell merged commit cf51abd into microsoft:master Oct 27, 2022
@behowell behowell deleted the slider/inset branch October 27, 2022 23:46
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Oct 31, 2022
* master: (22 commits)
  fix(react-menu): removes exposing of internal type FluentTriggerComponent (microsoft#25410)
  fix(react-popover): removes exposing of internal type FluentTriggerComponent (microsoft#25411)
  applying package updates
  fix(react-tooltip): removes exposing of internal type FluentTriggerComponent (microsoft#25409)
  chore: Reducing bundle size of Stack by moving selector used in multiple places to local const (microsoft#25429)
  docs(rfcs): Simple component implementation (microsoft#25139)
  Fix migration publishing (microsoft#25422)
  Integrate storywright for story tests - As part of exploring screener alternative (microsoft#25399)
  fix(react-utilities): exposes internal methods used in API surface (microsoft#25406)
  fix(react-dialog): removes exposing of internal type FluentTriggerComponent (microsoft#25408)
  fix(react-context-selector): exposes internal type ContextSelector (microsoft#25404)
  fix(react-aria): exposes internal leaking types (microsoft#25403)
  fix(react-shared-contexts): exposes internal leaks used in the API surface (microsoft#25405)
  fix(react-positioning): exposes new typings to avoid exposing internal methods (microsoft#25407)
  applying package updates
  fix: Allowing DatePicker to be focusable within FocusZones by default (microsoft#25428)
  fix: Pad in slider so the thumb does not render outside the bounds of the root element (microsoft#25378)
  feat: Add enableScopedSelectors prop to Stack that, when true, makes the Stack styles selectors be more scoped to not be as expensive in style recalculation (microsoft#25397)
  fix(react-avatar): Remove gaps between AvatarGroupItem/OveflowButton and its outline (microsoft#25382)
  fix: Combobox text attribute ignored when empty string is passed (microsoft#24665)
  ...
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Slider's thumb extends past the edges of the control

5 participants