Commands with a stop method, never programmatically stoped don`t run again!

[lucasschirm]

Problem

When you add a custom command with stop and dont stop will not run again if I dont set force: 1 as a parameter when running.

When you add a custom command with a stop method like:

var fired = 0;
editor.Commands.add('custom-command', {
run() {
fired++;
console.log("fired ", fired);
},
stop() {
console.log("Command stoped");
}
});

and if never run stop, the run method will never fired again.

editor.Commands.run('custom-command');
editor.Commands.run('custom-command');
editor.Commands.stop('custom-command');
editor.Commands.run('custom-command');

should print

fired 1
fired 2
stoped
fired 3

Other solution would add a property to Command Collection to be like:

editor.Commands.add('custom-command', {
runType: 'always' || 'after-stop' || 'after-stop-or-force',
run() {
fired++;
console.log("fired ", fired);
},
stop() {
console.log("Command stoped");
}
});

GrapeJS Fiddle

https://jsfiddle.net/lucasschirm/85juepgs/5/

How to replicate

• Create a new command with a stop function
• add a listener to modal:open to run it
• add a listener to modal:close to stop it
• add a listener on component:toggled to open a modal

What happens is:

• (click on any block) Command runs
• Modal Opens
• (close the modal) don`t fire modal:close
• (click outside of the block) Modal Opens

What should be happening and previous version behaviour:

• (click on any block) Command Runs
• Modal Opens
• (close the modal) don`t fire modal:close
• (click outside of the block) Command runs
• Modal Opens

[artf]

Hi @lucasschirm and thanks for the report.
As you've might notice this is the breaking change we've introduced in the latest version https://github.com/artf/grapesjs/releases/tag/v0.14.55
The reason behind it is quite simple. If you create a command with start and stop command you explicitly declare it as stateful one, so you activate it on start and disable it on stop.
When the command is active (you can get all currently active commands with editor.Commands.getActive()) it doesn't make sense running its activation instructions again (this prevents also weird issues you might face by running this instructions multiple times) and the same is once the command is disabled.
Apart from that you can always force it with force option so I don't see it as a problem.
One thing I can think about to add (just to keep it behave like the previous version, but I don't suggest it, because probably you're creating your command not properly) is some kind of a global option, eg:

grapesjs.init({
	...,
	commands: {
		strict: false, // by default is true
	},
})

[lucasschirm]

Thank you, I've sended an PR about the modal closing not firing the modal I close trigger.

…

On Thu, Mar 21, 2019, 3:14 PM Artur Arseniev ***@***.***> wrote: Hi @lucasschirm <https://github.com/lucasschirm> and thanks for the report. As you've might notice this is the breaking change we've introduced in the latest version https://github.com/artf/grapesjs/releases/tag/v0.14.55 The reason behind it is quite simple. If you create a command with start and stop command you explicitly declare it as *stateful* one, so you activate it on start and disable it on stop. When the command is active (you can get all currently active commands with editor.Commands.getActive()) it doesn't make sense running its activation instructions again (this prevents also weird issues you might face by running this instructions multiple times) and the same is once the command is disabled. Apart from that you can always force it with force option so I don't see it as a problem. One thing I can think about to add (just to keep it behave like the previous version, but I don't suggest it, because probably you're creating your command not properly) is some kind of a global option, eg: grapesjs.init({ ..., commands: { strict: false, // by default is true }, }) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1881 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFFgEmhHBOjwigByVdovuV-iEwj4FrWTks5vZAQ1gaJpZM4bp2N8> .

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.