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

Block any further submissions until the current one finishes. #3582

Merged
merged 3 commits into from Jun 15, 2016

Conversation

mkhatib
Copy link
Contributor

@mkhatib mkhatib commented Jun 15, 2016

ITI: #3343

@@ -77,9 +88,10 @@ export class AmpForm {
* @private
*/
handleSubmit_(e) {
if (e.defaultPrevented) {
if (e.defaultPrevented || this.state_ == FormState_.SUBMITTING) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to call preventDefault?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we should. Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -80,6 +91,12 @@ export class AmpForm {
if (e.defaultPrevented) {
return;
}

if (this.state_ == FormState_.SUBMITTING) {
e.preventDefault();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you capture this in a test case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pointed out where this is being captured in test.

@mkhatib
Copy link
Contributor Author

mkhatib commented Jun 15, 2016

PTAL

@cramforce
Copy link
Member

LGTM

@mkhatib mkhatib merged commit bcd0043 into ampproject:master Jun 15, 2016
@mkhatib mkhatib deleted the block-submit branch June 15, 2016 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants