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

Fixed UI command behaviour #626

Merged
merged 3 commits into from
Jul 26, 2016
Merged

Fixed UI command behaviour #626

merged 3 commits into from
Jul 26, 2016

Conversation

mahoneyj2
Copy link
Contributor

@mahoneyj2 mahoneyj2 commented Jul 26, 2016

Fixed UI command behaviour across all pages, such that buttons can only be pressed once per command. This removes side effects such as being able to trigger the same job multiple times on the recurring jobs page. #611

@mahoneyj2 mahoneyj2 changed the title Fixed button behaviour Fixed UI command behaviour Jul 26, 2016
@odinserj
Copy link
Member

Hi @mahoneyj2, how this fixes the problem? The linked issue is more related to a delay between the click and loading state change (due to setTimeout call). It is better to make buttons disabled before making a call to setTimeout. Btw, original issue says that other buttons (such as "Trigger now") are affected.

@mahoneyj2
Copy link
Contributor Author

Sorry for the confusion @odinserj!

The problem which I could identify from the issue was that when you select multiple jobs and "Trigger now", the button is disabled for a short period during which the command is dealt with, however the button is re-enabled before the page is reloaded after the POST. This meant that you could press the button multiple times, with the same jobs selected, causing the same command to be issued to the back-end. In the case of the recurring jobs page, this meant the "Trigger now" could be pressed before the page was reloaded, causing the same job to be triggered multiple times.

This behaviour is the same across all the pages, although only recurring jobs causes a problem, as you cannot re-queue jobs which are already queued.

I'll have another look, but it seemed to solve all the problems on my end!

@odinserj
Copy link
Member

Ah, now I see. What do you think about removing the button('reset') method call at all and disabling the button before calling the setTimeout method to prevent from clicking a button before receiving a POST response?

@mahoneyj2
Copy link
Contributor Author

Yeah that sounds like a better idea, just realised it was pointless as the page will be reloaded anyway. The earlier the button is disabled the better! Will update to that now!

@odinserj
Copy link
Member

Nice! Would you like to update the other button as well? It is also in the hangfire.js file, with the same code.

@mahoneyj2
Copy link
Contributor Author

Didn't spot that before! Have updated it now. Sorry for being a little slow, one of the first times using GitHub properly!

@odinserj
Copy link
Member

Nevermind, @mahoneyj2, sometimes I'm much slower 😄 Thank you for the contribution!

@odinserj odinserj merged commit 8d1c1c5 into HangfireIO:master Jul 26, 2016
@odinserj odinserj added this to the 1.6.1 milestone Jul 26, 2016
@mahoneyj2 mahoneyj2 deleted the FixRepeatedCommands branch July 26, 2016 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants