-
Notifications
You must be signed in to change notification settings - Fork 84
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
Make command executions (e.g. animations) stoppable #414
Conversation
Commands that implement the new introduced IStoppableCommand do stop the last execution immediately to start another one of the same kind. Signed-off-by: Jan Bicker <[email protected]>
7d02850
to
3eaa8c9
Compare
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.
Tested successfully. Changes look good. LGTM.
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.
Nice solution! I have a few more remarks though. @jbicker please address them in a follow-up PR.
} | ||
|
||
export function isStoppableCommand(command: any): command is IStoppableCommand { | ||
return command && 'stopExecution' in command && typeof command.stopExecution === 'function'; |
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.
You can use the utility function hasOwnProperty
from sprotty-protocol here.
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.
That does not work since the stopExecution is a member of the command objects prototype (MergeableCommand) and not a direct member at runtime.
@@ -676,16 +676,18 @@ export interface MoveAction extends Action { | |||
moves: ElementMove[] | |||
animate: boolean | |||
finished: boolean | |||
stoppable: boolean |
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.
Remark: When you do such protocol changes in futures, keep in mind that there's a Java (Xtend) based implementation, too, see here: https://github.com/eclipse-sprotty/sprotty-server/blob/master/org.eclipse.sprotty/src/main/java/org/eclipse/sprotty/Actions.xtend
I've updated it for now.
Commands that implement the new introduced IStoppableCommand do stop the last/current execution immediately to start another one of the same kind.
Solves #1
It is nicely testable by raising the duration time of the CommandStackOptions and trigger the scrambling in the circle example multiple times.