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

Add keyUP & keyDown + inverted button Up/Down to match UX #1

Merged
merged 1 commit into from
Sep 14, 2016

Conversation

matinfo
Copy link
Contributor

@matinfo matinfo commented Sep 8, 2016

No description provided.

@nkovacic
Copy link
Owner

I will review this PR in the following days

@nkovacic nkovacic merged commit 52fe7aa into nkovacic:master Sep 14, 2016
@nkovacic
Copy link
Owner

Removed the inverted up/down. Is there any reason you changed it?
The UX was consistent beacause usually the - is on the left side and the + on the right side.

@matinfo
Copy link
Contributor Author

matinfo commented Sep 17, 2016

Is was more with vertical layout mode that the up/down was not natural. Sorry I focused on this vertical mode I use. Need to have in vertical mode the button up that increment and the button down that decrement.

@nkovacic
Copy link
Owner

Please provide a codepen or jsfiddle example so I can see where the UX problem is.

@matinfo
Copy link
Contributor Author

matinfo commented Sep 17, 2016

See the Pen yaJRLy by Mathieu Meylan (@matinfo) on CodePen.

<script async src="//assets.codepen.io/assets/embed/ei.js"></script>

For vertical button only (verticalButtons = true):

  1. class UP/Down are inverted: bootstrap-touchspin-down = vm.touchSpinOptions.buttonUpClass / bootstrap-touchspin-up = vm.touchSpinOptions.buttonDownClass (small bug).
  2. if you click on button + the number is decremented in place of incremented. If click the up arrow number is decremented in place of incremented. But work ok/logical with mouse and keyboard (need to work like for the jQuery clone: http://www.virtuosoft.eu/code/bootstrap-touchspin/)

@nkovacic
Copy link
Owner

Ok i see where the bug is.
Will post a fix for this.

@nkovacic
Copy link
Owner

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.

None yet

2 participants