Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Slider] fix appearance #1468

Merged
merged 1 commit into from
Aug 24, 2015
Merged

[Slider] fix appearance #1468

merged 1 commit into from
Aug 24, 2015

Conversation

KapJI
Copy link
Contributor

@KapJI KapJI commented Aug 20, 2015

I found several issues with Slider appearance:

  1. When slider is at 0% the track overflows from the right.
  2. After moving slider to 0% sometimes handle has black border because of overlapping border styles.
  3. Spacing around disabled handle is relative and equal to 1% of width, but it should be equal to 2px.
  4. Incorrect cursor and handle size when 0% and disabled.
  5. No FocusRipple at 0%.
  6. Incorrect handle size on click, 24px instead of 18px.
  7. Transition to 0% when active.

This should fix all of them, but doesn't fix _setRippleSize() bug from #1451.
I used behavior and dimensions of sliders from https://www.google.com/design/spec/components/sliders.html
Tested on the latest stable Chrome, Firefox and Safari on OS X and on Chrome for Android.

@hai-cea
Copy link
Member

hai-cea commented Aug 21, 2015

@KapJI Can you rebase please?

@KapJI KapJI closed this Aug 21, 2015
@KapJI KapJI force-pushed the slider-appearance branch from cd92ce8 to 4c38959 Compare August 21, 2015 22:39
@KapJI KapJI reopened this Aug 21, 2015
@KapJI
Copy link
Contributor Author

KapJI commented Aug 21, 2015

Done.

@Kagami
Copy link

Kagami commented Aug 24, 2015

Does this PR fix the following 2 bugs? Or should I create separate issues for them?

1

2

@KapJI
Copy link
Contributor Author

KapJI commented Aug 24, 2015

In first case it changes how handle looks at 0% to
In second case I see two bugs: track shaking and black border of handle in the end. This PR fixes both.

@Kagami
Copy link

Kagami commented Aug 24, 2015

Great, thanks!

@Kagami
Copy link

Kagami commented Aug 24, 2015

I built your branch and can confirm: now both issues are fixed. 👍 for merging this.

The only minor thing that I've found is that slider stays focused after dragging. I guess this is correct behavior?

5

@KapJI
Copy link
Contributor Author

KapJI commented Aug 24, 2015

I didn't change this behavior and I think that it is correct.

@hai-cea hai-cea changed the title [Slider] fix appearance [Slider] fix handle focus appearance Aug 24, 2015
hai-cea pushed a commit that referenced this pull request Aug 24, 2015
[Slider] fix handle focus appearance
@hai-cea hai-cea merged commit fd1cef7 into mui:master Aug 24, 2015
@hai-cea
Copy link
Member

hai-cea commented Aug 24, 2015

Thanks @KapJI @Kagami

@hai-cea hai-cea changed the title [Slider] fix handle focus appearance [Slider] fix appearance Aug 24, 2015
@zannager zannager added the component: slider This is the name of the generic UI component, not the React module! label Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: slider This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants