Prevent double click new issue/pull/comment button

[a1012112796]

when network is not good, these button maybe
double clicked, then more than one same issues
pulls or comments will be created. this pull
request will fix this bug.

Before:

Now:

[KN4CK3R]

Almost every button should become a once-button.

[silverwind]

I would call it loading-button.

[a1012112796]

Almost every button should become a once-button.

Sure, all button which used to post a form by default logic can use this way. Will check more latter.

[lunny]

Since almost button clicked will refresh the page, I think it's suitable. But once we convert to an ajax request button. It should be changed. @jolheiser has a similar PR #10459, but I think this one is better.

[a1012112796]

Since almost button clicked will refresh the page, I think it's suitable. But once we convert to an ajax request button. It should be changed. @jolheiser has a similar PR #10459, but I think this one is better.

Hmm, timeout logic is usefull, I will check more later.

[zeripath]

CI failing

[lafriks]

Does this also will prevent with interacting with button twice using keyboard (sapcebar, enter)?

[wxiaoguang]

I think you need capture instead of bubble here. Otherwise the event handler on the button would be executed first (for example: ajax, IIRC some form buttons are doing ajax requests)

https://stackoverflow.com/questions/4616694/what-is-event-bubbling-and-capturing

[wxiaoguang]

To be clear, what I mean is:

If we are trying to introduce a general method, should the all cases be considered together? Will this mechanism be used for ajax buttons in future?

• If this mechanism is for traditional forms (non-ajax) only, these CSS names are too broad, it will be mis-used by ajax buttons in future.

• If this mechanism is for both traditional forms and ajax requests, it could be fine tuned some more.

---

update: I would suggest to use form-once-submit as the CSS name for these form-only buttons.

In future, we can have ajax-once-request CSS class for ajax buttons (if necessary), or ajax buttons should handle the repeated clicks by themselves.

[lafriks]

I think it should be generic for all use cases where technically possible and needed

[wxiaoguang]

Either is fine to me:

• Make the CSS name in this PR clear for only traditional form buttons, and get this PR merged.
• (OR) Let's spend some more time to fine tune some more.

How do you think?

[a1012112796]

Either is fine to me:

• Make the CSS name in this PR clear for only traditional form buttons, and get this PR merged.
• (OR) Let's spend some more time to fine tune some more.

How do you think?

how about limit it by this way?

Suggested change

	// loading-button this logic used to prevent push one form more than one time
	$(document).on('click', '.button.loading-button', function (e) {
	const $btn = $(this);
	if ($btn.hasClass('loading')) {
	e.preventDefault();
	return false;
	}
	// loading-button this logic used to prevent push one form more than one time
	// like double create new issue , pull request or comments
	$(document).on('click', 'form .button.loading-button', function (e) {
	const $btn = $(this);
	if ($btn.hasClass('loading')) {
	e.preventDefault();
	return false;
	}

[wxiaoguang]

loading-button is not a proper name ......

If you are reading some code, when you see a button called loading-button, you might think it is a button which is loading something. BUT, the purpose of the class is to make the button can only submit the form once.

I would suggest to use a clear name to reduce misunderstanding.

[lafriks]

it should be fixed to prevent button to submit form using keyboard

[wxiaoguang]

it should be fixed to prevent button to submit form using keyboard

Oh sorry, maybe I didn't say clearly. To be clear:

• enter on the target button triggers click, it can be handled, see the demo : https://jsfiddle.net/L0r4yxtw/1/ , enter on the button won't submit the form.

• What I mean is, if user do enter on other text input fields in the form (not on the button, in such case the first button's click is triggered), or use our Ctrl+Enter to submit, then the submit might not be handled. It is another case (should it be handled in this PR?)

Is it still a change request?

[lafriks]

Ok, handling form submit could be handled in other PR then imho

[wxiaoguang]

Suggested change

	$btn.addClass('loading disabled');
	$btn.addClass('loading disabled');
	$btn.prop('disabled', true);

Should we use $btn.prop('disabled', true); to disable the button entirely? Then it's not clickable anymore, and we do not need to check .loading anymore, and no need to return false (preventDefault)

[a1012112796]

looks semantic ui has handled it, just add disabled class is enough.

[wxiaoguang]

But what if it is not a Fomantic UI button? For example, what if some one writes a form without .ui class but uses your code?

If you require the developers to use your code in Fomantic UI context, you should change the selector to .ui.button .xxxxxx to make sure the code only works for Fomantic UI.

[wxiaoguang]

No blocker from my side.

[6543]

🚀