Use <button> not <input>s for form submission

Both `<button type='submit'>Submit<button>` and
`<input type='submit' value='Submit'>` can be used to submit a form.

We have historically[1] used `<input>` because it’s better-supported by
IE6 in that:
- the `submit` attribute is mandatory on `<button>`, not on `<input>`
- the `innerHTML` of a button will be submitted to the server, not the
  value (as in other browsers)

Reasons to now use `<button>` instead:
- IE6/7 support is no longer a concern (especially with deprecation of
  TLS 1.0 on the way)
- Because an `<input>` element can’t have children, the pseudo-element
  hack[2] used to ensure the top edge of the button is clickable doesn’t
  work. We’re seeing this bug[3] affect real users in research.

1. We inhereted our buttons from Digital Marketplace, here is me making
   that change in their code:  8df7e2e79e (diff-b1420f7b7a25657d849edf90a70ef541)
2. 24e1906c0d (diff-ef0e4eb6f1e90b44b0c3fe39dce274a4R79)

3. https://github.com/alphagov/govuk_elements/issues/545
This commit is contained in:
Chris Hill-Scott
2018-02-01 13:32:27 +00:00
parent 0d23775a62
commit 9aa4a7267a
15 changed files with 18 additions and 16 deletions

View File

@@ -13,7 +13,7 @@
{% if delete_button %}
<form method='post'>
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="button" name="delete" value="{{ delete_button }}" />
<button type="submit" class="button" name="delete">{{ delete_button }}</button
</form>
{% endif %}
</div>

View File

@@ -26,6 +26,6 @@
{% endif %}
<label class="file-upload-filename" for="{{ field.name }}"></label>
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="file-upload-submit" value="Submit" />
<button type="submit" class="file-upload-submit">Submit</button>
</form>
{% endmacro %}

View File

@@ -11,7 +11,7 @@
<div class="page-footer">
{% if button_text %}
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="button{% if destructive %}-destructive{% endif %}" value="{{ button_text }}" />
<button type="submit" class="button{% if destructive %}-destructive{% endif %}">{{ button_text }}</button>
{% endif %}
{% if back_link %}
<a class="page-footer-back-link" href="{{ back_link }}">{{ back_link_text }}</a>

View File

@@ -18,7 +18,7 @@
</p>
<form method='post'>
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="button" name="delete" value="Confirm" />
<button type="submit" class="button" name="delete">Confirm</button>
</form>
{% endcall %}
</div>

View File

@@ -37,7 +37,7 @@
) }}
{% endif %}
{% if template.template_type != 'letter' or not request.args.from_test %}
<input type="submit" class="button" value="Send {{ count_of_recipients }} {{ message_count_label(count_of_recipients, template.template_type, suffix='') }}" />
<button type="submit" class="button">Send {{ count_of_recipients }} {{ message_count_label(count_of_recipients, template.template_type, suffix='') }}</button>
{% else %}
<a href="{{ url_for('main.check_messages_preview', service_id=current_service.id, template_type=template.template_type, upload_id=upload_id, filetype='pdf') }}" download class="button">Download as a printable PDF</a>
{% endif %}

View File

@@ -34,7 +34,7 @@
</div>
<div class="column-one-quarter align-button-with-textbox">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<input type="submit" class="button" value="Search">
<button type="submit" class="button">Search</button>
</div>
</form>

View File

@@ -51,7 +51,7 @@
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
{% if not error %}
{% if template.template_type != 'letter' or not request.args.from_test %}
<input type="submit" class="button" value="Send 1 {{ message_count_label(1, template.template_type, suffix='') }}" />
<button type="submit" class="button">Send 1 {{ message_count_label(1, template.template_type, suffix='') }}</button>
{% else %}
<a href="{{ url_for('main.check_messages_preview', service_id=current_service.id, template_type=template.template_type, upload_id=upload_id, filetype='pdf') }}" download class="button">Download as a printable PDF</a>
{% endif %}

View File

@@ -22,7 +22,7 @@
{{ textbox(form.end_date, hint="Enter end date in format YYYY-MM-DD") }}
{{ checkbox(form.include_from_test_key) }}
</br>
<input type="submit" class="button">
<button type="submit" class="button">Filter</button>
</form>
</details>

View File

@@ -107,7 +107,7 @@
{{ textbox(form.end_date, hint="Enter end date in format YYYY-MM-DD") }}
{{ checkbox(form.include_from_test_key) }}
</br>
<input type="submit" class="button">
<button type="submit" class="button">Filter</button>
</form>
</details>

View File

@@ -23,7 +23,7 @@
</ul>
<form method='post'>
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="button" name="delete" value="Confirm" />
<button type="submit" class="button" name="delete">Confirm</button>
</form>
{% endcall %}
</div>
@@ -37,7 +37,7 @@
{% endif %}
<form method='post'>
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="button" name="delete" value="Confirm" />
<button type="submit" class="button" name="delete">Confirm</button>
</form>
{% endcall %}
</div>

View File

@@ -265,7 +265,8 @@ def test_should_show_confirm_revoke_api_key(
)
assert normalize_spaces(page.select('.banner-dangerous')[0].text) == (
'Are you sure you want to revoke this API key? '
'some key name will no longer let you connect to GOV.UK Notify.'
'some key name will no longer let you connect to GOV.UK Notify. '
'Confirm'
)
assert mock_get_api_keys.call_args_list == [
call(

View File

@@ -242,7 +242,8 @@ def test_should_show_confirm_revoke_api_key(
)
assert normalize_spaces(page.select('.banner-dangerous')[0].text) == (
'Are you sure you want to revoke this API key? '
'some key name will no longer let you connect to GOV.UK Notify.'
'some key name will no longer let you connect to GOV.UK Notify. '
'Confirm'
)
assert mock_get_api_keys.call_args_list == [
call(

View File

@@ -255,7 +255,7 @@ def test_should_show_scheduled_job(
template_id='5d729fbd-239c-44ab-b498-75a985f3198f',
version=1,
)
assert page.find('input', {'type': 'submit', 'value': 'Cancel sending'})
assert page.select_one('button[type=submit]').text.strip() == 'Cancel sending'
def test_should_cancel_job(

View File

@@ -148,7 +148,7 @@ def test_shows_temp_logo_after_uploading_logo(request_post_manage_org_redirect):
def test_save_enabled_after_uploading_logo(request_post_manage_org_redirect):
page, _ = request_post_manage_org_redirect
assert not page.select_one('div.page-footer input.button').has_attr('disabled')
assert not page.select_one('div.page-footer button.button').has_attr('disabled')
def test_deletes_previous_temp_logo_after_uploading_logo(logged_in_platform_admin_client, mocker, fake_uuid):

View File

@@ -611,7 +611,7 @@ def test_send_test_doesnt_show_file_contents(
assert page.select('h1')[0].text.strip() == 'Preview of Two week reminder'
assert len(page.select('table')) == 0
assert len(page.select('.banner-dangerous')) == 0
assert page.select('input[type=submit]')[0]['value'].strip() == 'Send 1 text message'
assert page.select_one('button[type=submit]').text.strip() == 'Send 1 text message'
@pytest.mark.parametrize('endpoint, template_mock, expected_recipient', [