-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Prevent multiple codecomments to be submitted #28139
Conversation
- Utilize the existing `is-loading` class to prevent the submit code from running twice or more. - The `is-loading` class gives us a nice loading indicator when added to the form. - Resolves https://codeberg.org/forgejo/forgejo/issues/1476 (cherry picked from commit 85048c96608004988f57a57358c9cda91f095e23)
if ($form.hasClass('is-loading')) return; | ||
$form.addClass('is-loading'); |
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.
Yes and no.
That class exists but is used for content that has not yet been loaded, not to click buttons only once.
There is a class that ensures buttons/forms are only submitted once.
I think it is form-fetch-action
for forms or link-action
for buttons/links.
Please use the correct class instead.
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.
The form-fetch-action uses this https://github.com/go-gitea/gitea/blob/main/web_src/js/features/common-global.js#L114-L116 and we cannot use form-fetch-action because the code comments has custom javascript code, which is initRepoDiffConversationForm.
Am I missing something?
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.
Okay, as @wxiaoguang pointed out in #28143, is-loading
seems to be correct.
Sorry for that.
Anyway, #28143 does look better to me at the moment as it does not deadlock itself.
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.
This change is not right.
You only use straight code to do something, but some edge cases are not handled, which lead to bad user experience.
Which unimplemented edge cases do you see? |
-> #28143 At least 2 bugs here:
|
replaced by #28143 |
is-loading
class to prevent the submit code from running twice or more.is-loading
class gives us a nice loading indicator when added to the form.(cherry picked from commit 85048c96608004988f57a57358c9cda91f095e23)