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

Fix position calculations for 'vertical' mode #199

Merged
merged 2 commits into from
Dec 2, 2018

Conversation

mrkosima
Copy link
Contributor

@mrkosima mrkosima commented Nov 8, 2018

This PR fixes handle's position calculations for vertical mode.

Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks! Could you add a regression test that would have caught this?

@mrkosima
Copy link
Contributor Author

mrkosima commented Nov 9, 2018

I'm going to provide new PR with fixing vertical slider in storybook soon.
And will put a test for this case in it.

Can we merge this PR now and release a package, because the fix is obvious and very important for my current project?

@ljharb
Copy link
Collaborator

ljharb commented Nov 9, 2018

I’d prefer not to merge a fix without a regression test in the same PR.

It may be obvious looking at the diff, but it was non-obvious enough to be missed previously.

@mrkosima
Copy link
Contributor Author

@ljharb , I've added tests, check please.

@mrkosima mrkosima force-pushed the fix-vertical-percentage branch from bb41707 to ea18f80 Compare November 12, 2018 16:45
@dperyel
Copy link

dperyel commented Nov 15, 2018

Would be nice to have the fix merged soon (-;

@mrkosima
Copy link
Contributor Author

@majapw could you please check the PR?

@mrkosima
Copy link
Contributor Author

@ljharb could you help the PR to be merged?

@ljharb
Copy link
Collaborator

ljharb commented Nov 23, 2018

@mrkosima the only way for this PR to be merged is to wait patiently for maja to review and merge it :-)

Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Sorry for the long delay in reviewing!

@majapw majapw merged commit 41bf1af into airbnb:master Dec 2, 2018
@mrkosima
Copy link
Contributor Author

@ljharb is it possible to make a 3.0.2 release with this fix?

@ljharb
Copy link
Collaborator

ljharb commented Dec 10, 2018

@mrkosima not sure why you're pinging me; i'm sure @majapw will cut a release as soon as she has time.

@majapw
Copy link
Collaborator

majapw commented Dec 11, 2018

Released! I will do better next time at quick turnaround. :)

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.

4 participants