Skip to content

Commit

Permalink
Make #govuk_submit render button instead of input
Browse files Browse the repository at this point in the history
This should address the bug where `<input type="submit">` doesn't
register clicks when the very top of the element is clicked[0] and is
more in-line with the Design System guidance[1].

[0] alphagov/govuk_elements#545
[1] https://design-system.service.gov.uk/components/button/#default-buttons

Fixes #295
  • Loading branch information
peteryates committed Jun 21, 2021
1 parent a7be5ae commit 8e54a52
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 28 deletions.
3 changes: 2 additions & 1 deletion lib/govuk_design_system_formbuilder/elements/submit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@ def buttons
end

def submit
@builder.submit(@text, **attributes(@html_attributes))
@builder.tag.button(@text, **attributes(@html_attributes))
end

def options
{
type: 'submit',
formnovalidate: !@validate,
disabled: @disabled,
class: classes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@
end

specify 'should use the default value when no override supplied' do
expect(subject).to have_tag('input', with: { type: 'submit', value: default_submit_button_text })
expect(subject).to have_tag('button', with: { type: 'submit' }, text: default_submit_button_text)
end

context %(overriding with 'Engage') do
let(:submit_button_text) { 'Engage' }
let(:args) { [method, submit_button_text] }

specify 'should use supplied value when overridden' do
expect(subject).to have_tag('input', with: { type: 'submit', value: submit_button_text })
expect(subject).to have_tag('button', with: { type: 'submit' }, text: submit_button_text)
end
end
end
Expand All @@ -55,14 +55,14 @@
end

specify 'should have no formnovalidate attribute' do
expect(parsed_subject.at_css('input').attributes.keys).not_to include('formnovalidate')
expect(parsed_subject.at_css('button').attributes.keys).not_to include('formnovalidate')
end

context %(overriding with false) do
let(:kwargs) { { validate: false } }

specify 'should have a formnovalidate attribute' do
expect(subject).to have_tag('input', with: { type: 'submit', formnovalidate: 'formnovalidate' })
expect(subject).to have_tag('button', with: { type: 'submit', formnovalidate: 'formnovalidate' })
end
end
end
Expand Down
46 changes: 23 additions & 23 deletions spec/govuk_design_system_formbuilder/builder/submit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,37 +12,37 @@
it_behaves_like 'a field that supports custom branding'

it_behaves_like 'a field that supports custom classes' do
let(:element) { 'input' }
let(:element) { 'button' }
let(:default_classes) { %w(govuk-button) }
let(:block_content) { -> { %(Example) } }
end

it_behaves_like 'a field that allows extra HTML attributes to be set' do
let(:described_element) { 'input' }
let(:described_element) { 'button' }
let(:expected_class) { 'govuk-button' }
end

specify 'output should be a submit input' do
expect(subject).to have_tag('input', with: { type: 'submit' })
specify 'output should be a button element' do
expect(subject).to have_tag('button', with: { type: 'submit' })
end

specify 'button should have the correct classes' do
expect(subject).to have_tag('input', with: { class: 'govuk-button' })
expect(subject).to have_tag('button', with: { class: 'govuk-button' })
end

specify 'button should have the correct text' do
expect(subject).to have_tag('input', with: { value: text })
expect(subject).to have_tag('button', text: text)
end

specify 'button should have the govuk-button data-module' do
expect(subject).to have_tag('input', with: { 'data-module' => 'govuk-button' })
expect(subject).to have_tag('button', with: { 'data-module' => 'govuk-button' })
end

context 'when no value is passed in' do
subject { builder.send(method) }

specify %(it should default to 'Continue) do
expect(subject).to have_tag('input', with: { value: 'Continue' })
expect(subject).to have_tag('button', text: 'Continue')
end
end

Expand All @@ -51,15 +51,15 @@
subject { builder.send(*args.push('Create'), warning: true) }

specify 'button should have the warning class' do
expect(subject).to have_tag('input', with: { class: %w(govuk-button govuk-button--warning) })
expect(subject).to have_tag('button', with: { class: %w(govuk-button govuk-button--warning) })
end
end

context 'secondary' do
subject { builder.send(*args.push('Create'), secondary: true) }

specify 'button should have the secondary class' do
expect(subject).to have_tag('input', with: { class: %w(govuk-button govuk-button--secondary) })
expect(subject).to have_tag('button', with: { class: %w(govuk-button govuk-button--secondary) })
end
end

Expand All @@ -75,22 +75,22 @@
subject { builder.send(*args.push('Create'), classes: %w(custom-class--one custom-class--two)) }

specify 'button should have the custom class' do
expect(subject).to have_tag('input', with: { class: %w(govuk-button custom-class--one custom-class--two) })
expect(subject).to have_tag('button', with: { class: %w(govuk-button custom-class--one custom-class--two) })
end
end
end

describe 'preventing double clicks' do
specify 'data attribute should be present by default' do
expect(subject).to have_tag('input', with: { 'data-prevent-double-click' => true })
expect(subject).to have_tag('button', with: { 'data-prevent-double-click' => true })
end

context 'when disabled' do
subject { builder.send(*args.push(text), prevent_double_click: false) }

specify 'data attribute should not be present by default' do
expect(
parsed_subject.at_css('input').attributes.keys
parsed_subject.at_css('button').attributes.keys
).not_to include('data-prevent-double-click')
end
end
Expand All @@ -112,8 +112,8 @@

specify 'should wrap the buttons and extra content in a button group' do
expect(subject).to have_tag('div', with: { class: 'govuk-button-group' }) do
with_tag('input', value: text)
with_tag('a', with: { href: target })
with_tag('button', text: 'Continue')
with_tag('a', text: text, with: { href: target })
end
end
end
Expand All @@ -123,23 +123,23 @@
subject { builder.send(*args) }

specify 'should have attribute formnovalidate' do
expect(subject).to have_tag('input', with: { type: 'submit', formnovalidate: 'formnovalidate' })
expect(subject).to have_tag('button', with: { type: 'submit', formnovalidate: 'formnovalidate' })
end
end

context 'when validate is false' do
subject { builder.send(*args, validate: false) }

specify 'should have attribute formnovalidate' do
expect(subject).to have_tag('input', with: { type: 'submit', formnovalidate: 'formnovalidate' })
expect(subject).to have_tag('button', with: { type: 'submit', formnovalidate: 'formnovalidate' })
end
end

context 'when validate is true' do
subject { builder.send(*args, validate: true) }

specify 'should have attribute formnovalidate' do
expect(parsed_subject.at_css('input').attributes.keys).not_to include('formnovalidate')
expect(parsed_subject.at_css('button').attributes.keys).not_to include('formnovalidate')
end
end
end
Expand All @@ -148,17 +148,17 @@
context 'when disabled is false' do
subject { builder.send(*args.push('Create')) }

specify 'input should not have the disabled attribute' do
expect(parsed_subject.at_css('input').attributes.keys).not_to include('disabled')
specify 'button should not have the disabled attribute' do
expect(parsed_subject.at_css('button').attributes.keys).not_to include('disabled')
end
end

context 'when disabled is true' do
subject { builder.send(*args.push('Create'), disabled: true) }

specify 'input should have the disabled attribute' do
expect(parsed_subject.at_css('input').attributes.keys).to include('disabled')
expect(subject).to have_tag('input', with: { class: %w(govuk-button govuk-button--disabled) })
specify 'button should have the disabled attribute' do
expect(parsed_subject.at_css('button').attributes.keys).to include('disabled')
expect(subject).to have_tag('button', with: { class: %w(govuk-button govuk-button--disabled) })
end
end
end
Expand Down

0 comments on commit 8e54a52

Please sign in to comment.