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

Prevent multiple form submissions #1018

Merged
merged 3 commits into from
Mar 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,29 @@

([PR #1178](https://github.com/alphagov/govuk-frontend/pull/1178))

- Prevent accidental multiple submissions of forms

If a user double clicks a submit button in a form, we debounce this event and ignore the second click.

HTML data attribute:

```html
<button class="govuk-button" data-prevent-double-click="true">
Submit
</button>
```

Nunjucks macro:

```js
{{ govukButton({
text: "Submit",
preventDoubleClick: true
}) }}
```

([PR #1018](https://github.com/alphagov/govuk-frontend/pull/1018))

🔧 Fixes:

- Pull Request Title goes here
Expand Down
27 changes: 26 additions & 1 deletion src/components/button/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@
import '../../vendor/polyfills/Event' // addEventListener and event.target normaliziation

var KEY_SPACE = 32
var DEBOUNCE_TIMEOUT_IN_SECONDS = 1
var debounceFormSubmitTimer = null

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.


function Button ($module) {
this.$module = $module
}

/**
* Add event handler for KeyDown
* if the event target element has a role='button' and the event is key space pressed
* then it prevents the default event and triggers a click event
* @param {object} event event
Expand All @@ -33,12 +34,36 @@ Button.prototype.handleKeyDown = function (event) {
}
}

/**
* If the click quickly succeeds a previous click then nothing will happen.
* This stops people accidentally causing multiple form submissions by
* double clicking buttons.
*/
Button.prototype.debounce = function (event) {
var target = event.target
// Check the button that is clicked on has the preventDoubleClick feature enabled
if (target.getAttribute('data-prevent-double-click') !== 'true') {
return
}

// If the timer is still running then we want to prevent the click from submitting the form
if (debounceFormSubmitTimer) {
event.preventDefault()
return false
}

debounceFormSubmitTimer = setTimeout(function () {
debounceFormSubmitTimer = null
}, DEBOUNCE_TIMEOUT_IN_SECONDS * 1000)
}

/**
* Initialise an event listener for keydown at document level
* this will help listening for later inserted elements with a role="button"
*/
Button.prototype.init = function () {
this.$module.addEventListener('keydown', this.handleKeyDown)
this.$module.addEventListener('click', this.debounce)
}

export default Button
82 changes: 81 additions & 1 deletion src/components/button/button.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('/components/button', () => {

// we need to start the waitForNavigation() before the keyboard action
// so we return a promise that is fulfilled when the navigation and the keyboard action are respectively fulfilled
// this is somewhat counter intuitive, as we humans expect to act and then wait for something
// this is somewhat counter intuitive, as we humans expect to act and then wait for something.
await Promise.all([
page.waitForNavigation(),
page.keyboard.press('Space')
Expand All @@ -41,5 +41,85 @@ describe('/components/button', () => {
const url = await page.url()
expect(url).toBe(baseUrl + href)
})
describe('preventing double clicks', () => {
it('prevents unintentional submissions when in a form', async () => {
await page.goto(baseUrl + '/components/button/prevent-double-click/preview', { waitUntil: 'load' })

// Our examples don't have form wrappers so we need to overwrite it.
await page.evaluate(() => {
const $button = document.querySelector('button')
const $form = document.createElement('form')
$button.parentNode.appendChild($form)
$button.parentNode.removeChild($button)
$form.appendChild($button)

window.__SUBMIT_EVENTS = 0
$form.addEventListener('submit', event => {
window.__SUBMIT_EVENTS++
// Don't refresh the page, which will destroy the context to test against.
event.preventDefault()
})
})

await page.click('button')
await page.click('button')

const submitCount = await page.evaluate(() => window.__SUBMIT_EVENTS)

expect(submitCount).toBe(1)
})
it('does not prevent intentional multiple clicks', async () => {

This comment was marked as resolved.

This comment was marked as resolved.

await page.goto(baseUrl + '/components/button/prevent-double-click/preview', { waitUntil: 'load' })

// Our examples don't have form wrappers so we need to overwrite it.
await page.evaluate(() => {
const $button = document.querySelector('button')
const $form = document.createElement('form')
$button.parentNode.appendChild($form)
$button.parentNode.removeChild($button)
$form.appendChild($button)

window.__SUBMIT_EVENTS = 0
$form.addEventListener('submit', event => {
window.__SUBMIT_EVENTS++
// Don't refresh the page, which will destroy the context to test against.
event.preventDefault()
})
})

await page.click('button')
await page.click('button', { delay: 1000 })

const submitCount = await page.evaluate(() => window.__SUBMIT_EVENTS)

expect(submitCount).toBe(2)
})
it('does not prevent multiple submissions when feature is not enabled', async () => {
await page.goto(baseUrl + '/components/button/preview', { waitUntil: 'load' })

// Our examples don't have form wrappers so we need to overwrite it.
await page.evaluate(() => {
const $button = document.querySelector('button')
const $form = document.createElement('form')
$button.parentNode.appendChild($form)
$button.parentNode.removeChild($button)
$form.appendChild($button)

window.__SUBMIT_EVENTS = 0
$form.addEventListener('submit', event => {
window.__SUBMIT_EVENTS++
// Don't refresh the page, which will destroy the context to test against.
event.preventDefault()
})
})

await page.click('button')
await page.click('button')

const submitCount = await page.evaluate(() => window.__SUBMIT_EVENTS)

expect(submitCount).toBe(2)
})
})
})
})
8 changes: 8 additions & 0 deletions src/components/button/button.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ params:
type: object
required: false
description: HTML attributes (for example data attributes) to add to the button component.
- name: preventDoubleClick
type: boolean
required: false
description: Prevent accidental double clicks on submit buttons from submitting forms multiple times

examples:
- name: default
Expand Down Expand Up @@ -72,6 +76,10 @@ examples:
element: input
text: Explicit input button disabled
disabled: true
- name: prevent double click
data:
text: Submit
preventDoubleClick: true
- name: with active state
description: Simulate triggering the :active CSS pseudo-class, not available in the production build.
data:
Expand Down
2 changes: 1 addition & 1 deletion src/components/button/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

{#- Define common attributes we can use for both button and input types #}

{%- set buttonAttributes %}{% if params.name %} name="{{ params.name }}"{% endif %} type="{{ params.type if params.type else 'submit' }}"{% if params.disabled %} disabled="disabled" aria-disabled="true"{% endif %}{% endset %}
{%- set buttonAttributes %}{% if params.name %} name="{{ params.name }}"{% endif %} type="{{ params.type if params.type else 'submit' }}"{% if params.disabled %} disabled="disabled" aria-disabled="true"{% endif %}{% if params.preventDoubleClick %} data-prevent-double-click="true"{% endif %}{% endset %}

{#- Actually create a button... or a link! #}

Expand Down
9 changes: 9 additions & 0 deletions src/components/button/template.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,15 @@ describe('Button', () => {
const $component = $('.govuk-button')
expect($component.attr('type')).toEqual('button')
})

it('renders with preventDoubleClick attribute', () => {
const $ = render('button', {
preventDoubleClick: true
})

const $component = $('.govuk-button')
expect($component.attr('data-prevent-double-click')).toEqual('true')
})
})

describe('implicitly as no "element" param is set', () => {
Expand Down