-
Notifications
You must be signed in to change notification settings - Fork 21
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 #169: stepSize not updated when holding button #170
base: master
Are you sure you want to change the base?
Conversation
this.lastDirectionUp = true; | ||
} | ||
onValueChanged: { | ||
if(this.ignoreNextValueChange){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally this logic should be moved into BetterSpinBox, and it should only call the valueChanged signal if ignoreNextValueChange is false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have to fiddle with this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure. Just for some more context for you, I intend to pull out a bunch of the widgets into their own widget library at some point, so I'd like to make as many of them fully self contained as I can before I do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be best to create a second signal. I'm thinking "valueStable"? It doesn't look like Javascript's super
keyword will be any use here at all.
Unfortunately, all of this can be overwritten if someone intuitively attempts to use the original onValueChanged
signal.
I'd be curious to hear your thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, we could just try to make ignoreNextValueChange
an instance variable and not declare it in BetterSpinBox at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you can do is make Item be the root of BetterSpinBox and then declare the signals/properties there and map them into the SpinBox. That way you'll have more control over when things are called.
@danshick any chance you'll get back to this? I'd like to include it in 2.2 if possible. |
Probably not on any short timetable, sorry. |
Code Climate has analyzed commit d7f2fe2 and detected 0 issues on this pull request. View more on Code Climate. |
@danshick do you intend to get back to this? |
There's no way to preempt stepSize in the onValueChanged function, so we have to adjust the set value as the value is changing. This fixes #169 as cleanly as I can manage.