Skip to content

Remove submit logic from click event handler #79

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 10, 2018
Merged

Remove submit logic from click event handler #79

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions s3file/static/s3file/js/s3file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,13 @@
}

function clickSubmit (e) {
e.preventDefault()
let submitButton = e.target
let form = submitButton.closest('form')
const submitInput = document.createElement('input')
submitInput.type = 'hidden'
submitInput.value = submitButton.value || true
submitInput.name = submitButton.name
form.appendChild(submitInput)
uploadS3Inputs(form)
Copy link
Owner

Choose a reason for hiding this comment

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

@tilboerner how does this still work?

Copy link
Contributor Author

@tilboerner tilboerner Mar 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

It still works because with the other removed line in this patch, the default action for the click event is not prevented. That default action is to cause a 'submit' event on the form. (Note that the function containing the change is not handling the submit event itself, but the click event leading up to it. Preventing its default action means no submit event gets dispatched, and not submit event handlers get called, even if we later submit the form from the prototype.)

The submit event is then handled by the form.onsubmit function which was set below, on 'DOMContentLoaded':

django-s3file/s3file/static/s3file/js/s3file.js

Lines 111 to 114 in 7e7656d

form.onsubmit = (e) => {
e.preventDefault()
uploadS3Inputs(e.target)
}

If I understand JS correctly, things proceed synchronously, and we can trust at this point that if the submit was triggered by a click, the click handler has added the hidden input to the form.

I found one caveat: When manually dispatching a click event for the button, it needs to be a MouseEvent to trigger the submit logic; a generic Event("click") triggers the click handler, but does not default to submitting the form. Without the change in this PR, any old Event would do.

However, the canonical click event is a MouseEvent, and gets dispatched as such even when triggering the button by keyboard or calling button.click(). All things considered, I think it's better to make the proposed change and honor the expectation of other submit handlers that they will be called on submit, rather than cater to code that is trying to use non-standard, non-MouseEvents to trigger submit clicks.

edit Forgot to mention @codingjoe. :)

}

function uploadS3Inputs (form) {
Expand Down