-
Notifications
You must be signed in to change notification settings - Fork 948
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
new: repeat button for Play widget, can be hidden #1190
new: repeat button for Play widget, can be hidden #1190
Conversation
Awesome! |
Is it an idea to take this in a seperate PR, since it will be nice to have this consistent for 7.0, and leave the extra button for 7.x or higher. |
I think either way (fixing the consistency issue, or also including the new button) is fine for 7.0. |
So, do you mind if I merge this then, I've tested it myself. Should the change in behaviour of Play go in some changelog? |
jupyter-js-widgets/src/widget_int.ts
Outdated
@@ -660,12 +662,16 @@ class PlayModel extends BoundedIntModel { | |||
loop() { | |||
if (this.get('_playing')) { | |||
var next_value = this.get('value') + this.get('step'); | |||
if (next_value < this.get('max')) { | |||
if (next_value <= this.get('max')) { | |||
this.set('value', next_value); | |||
window.setTimeout(this.loop.bind(this), this.get('interval')); | |||
} else { | |||
this.set('value', this.get('min')); |
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.
Can we only reset it to the min if we are repeating? Reverting to the min after a straight play through has been frustrating to me, instead of just stopping at the end and freezing the state there.
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 agree!
Yes, there is a changelog at https://github.com/jupyter-widgets/ipywidgets/blob/master/docs/source/changelog.md |
$#@% messed up rebasing, any idea what I did? :) |
8eacee5
to
8e47708
Compare
Looks like you figured it out. |
I don't see that I can 'approve' sth
Yes, I had to |
As discussed in #1186 this adds a repeat button. I've chosed the fa-retweet button, since the fa-repeat does not look like a repeat button.
Futhermore I've included what I think is a fix for Play, it now includes max, since that is more consistent with the slider's max behavious.
hide repeat button, and turn on repeat: