From 65c7947d34d8c2c0d1b02ab5713b20e7e79d767f Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Sun, 11 Jun 2023 10:13:27 +0100 Subject: [PATCH 1/4] Add new set of link and button helpers These are clean implementations of that significantly simplify the codebase and allow us to easily support features of the design system, like opening links in new tabs. The new methods are: * govuk_link_to * govuk_button_to * govuk_button_link_to * govuk_link_classes * govuk_button_classes * govuk_breadcrumb_link_to They drop support for Rails' legacy-style link building where the action, controller and id arguments were passed in. Rails discourages their use in the API docs[0]: > Because it relies on url_for, link_to supports both older-style > controller/action/id arguments and newer RESTful routes. Current Rails > style favors RESTful routes whenever possible, so base your application > on resources and use [0] https://api.rubyonrails.org/v7.0/classes/ActionView/Helpers/UrlHelper.html#method-i-link_to --- .../action_component.rb | 2 +- app/helpers/govuk_link_helper.rb | 169 +++--- .../govuk_rails_compatibile_link_helper.rb | 157 ++++++ .../cookie_banner_component_spec.rb | 2 +- spec/helpers/govuk_link_helper_spec.rb | 499 +++++++++++------- ...ovuk_rails_compatibile_link_helper_spec.rb | 325 ++++++++++++ 6 files changed, 854 insertions(+), 300 deletions(-) create mode 100644 app/helpers/govuk_rails_compatibile_link_helper.rb create mode 100644 spec/helpers/govuk_rails_compatibile_link_helper_spec.rb diff --git a/app/components/govuk_component/summary_list_component/action_component.rb b/app/components/govuk_component/summary_list_component/action_component.rb index a472737d..03e208d8 100644 --- a/app/components/govuk_component/summary_list_component/action_component.rb +++ b/app/components/govuk_component/summary_list_component/action_component.rb @@ -27,7 +27,7 @@ def call private def default_attributes - link_classes = govuk_link_classes.append(classes).flatten + link_classes = safe_join([govuk_link_classes, classes], " ") { class: link_classes } end diff --git a/app/helpers/govuk_link_helper.rb b/app/helpers/govuk_link_helper.rb index 5d16c5a8..9c16cc3a 100644 --- a/app/helpers/govuk_link_helper.rb +++ b/app/helpers/govuk_link_helper.rb @@ -3,156 +3,123 @@ module GovukLinkHelper using HTMLAttributesUtils - def govuk_link_classes(*styles, default_class: "#{brand}-link") - if (invalid_styles = (styles - link_styles.keys)) && invalid_styles.any? - fail(ArgumentError, "invalid styles #{invalid_styles.to_sentence}. Valid styles are #{link_styles.keys.to_sentence}") + def govuk_link_classes(inverse: false, muted: false, no_underline: false, no_visited_state: false, text_colour: false) + if [text_colour, inverse, muted].count(true) > 1 + fail("links can be either text_colour, inverse or muted - not combinations of the three") end - [default_class] + link_styles.values_at(*styles).compact + class_names( + "#{brand}-link", + "#{brand}-link--inverse" => inverse, + "#{brand}-link--muted" => muted, + "#{brand}-link--no-underline" => no_underline, + "#{brand}-link--no-visited-state" => no_visited_state, + "#{brand}-link--text-colour" => text_colour, + ) end - def govuk_button_classes(*styles, default_class: "#{brand}-button") - if (invalid_styles = (styles - button_styles.keys)) && invalid_styles.any? - fail(ArgumentError, "invalid styles #{invalid_styles.to_sentence}. Valid styles are #{button_styles.keys.to_sentence}") + def govuk_button_classes(disabled: false, inverse: false, secondary: false, warning: false) + if [inverse, secondary, warning].count(true) > 1 + fail("buttons can be either inverse, secondary or warning - not combinations of the three") end - [default_class] + button_styles.values_at(*styles).compact + class_names( + "#{brand}-button", + "#{brand}-button--disabled" => disabled, + "#{brand}-button--inverse" => inverse, + "#{brand}-button--secondary" => secondary, + "#{brand}-button--warning" => warning, + ) end - def govuk_link_to(name = nil, options = nil, extra_options = {}, &block) - extra_options = options if block_given? - html_options = build_html_options(extra_options) + def govuk_link_to(name, href = nil, new_tab: false, inverse: false, muted: false, no_underline: false, no_visited_state: false, text_colour: false, **kwargs, &block) + link_args = extract_link_args(new_tab: new_tab, inverse: inverse, muted: muted, no_underline: no_underline, no_visited_state: no_visited_state, text_colour: text_colour, **kwargs) if block_given? - link_to(name, html_options, &block) + link_to(block.call, href, **link_args) else - link_to(name, options, html_options) + link_to(name, href, **link_args) end end - def govuk_mail_to(email_address, name = nil, extra_options = {}, &block) - extra_options = name if block_given? - html_options = build_html_options(extra_options) + def govuk_mail_to(email_address, name = nil, new_tab: false, inverse: false, muted: false, no_underline: false, no_visited_state: false, text_colour: false, **kwargs, &block) + link_args = extract_link_args(new_tab: new_tab, inverse: inverse, muted: muted, no_underline: no_underline, no_visited_state: no_visited_state, text_colour: text_colour, **kwargs) if block_given? - mail_to(email_address, html_options, &block) + mail_to(email_address, block.call, **link_args) else - mail_to(email_address, name, html_options) + mail_to(email_address, name, **link_args) end end - def govuk_button_to(name = nil, options = nil, extra_options = {}, &block) - extra_options = options if block_given? - html_options = { - data: { module: "govuk-button" } - } - - if extra_options && extra_options[:prevent_double_click] - html_options[:data]["prevent-double-click"] = "true" - extra_options = extra_options.except(:prevent_double_click) - end - - html_options.merge! build_html_options(extra_options, style: :button) + def govuk_button_to(name, href = nil, disabled: false, inverse: false, secondary: false, warning: false, **kwargs, &block) + button_args = extract_button_args(new_tab: false, disabled: disabled, inverse: inverse, secondary: secondary, warning: warning, **kwargs) if block_given? - button_to(options, html_options, &block) + button_to(block.call, href, **button_args) else - button_to(name, options, html_options) + button_to(name, href, **button_args) end end - def govuk_button_link_to(name = nil, options = nil, extra_options = {}, &block) - extra_options = options if block_given? - html_options = { - data: { module: "#{brand}-button" }, - draggable: 'false', - role: 'button', - }.merge build_html_options(extra_options, style: :button) + def govuk_button_link_to(name, href = nil, new_tab: false, disabled: false, inverse: false, secondary: false, warning: false, **kwargs, &block) + button_args = extract_button_args(new_tab: new_tab, disabled: disabled, inverse: inverse, secondary: secondary, warning: warning, **kwargs) if block_given? - link_to(name, html_options, &block) + link_to(block.call, href, **button_args) else - link_to(name, options, html_options) + link_to(name, href, **button_args) end end - def govuk_breadcrumb_link_to(name = nil, options = nil, extra_options = {}, &block) - extra_options = options if block_given? - html_options = build_html_options(extra_options, style: :breadcrumb) + def govuk_breadcrumb_link_to(name, href = nil, **kwargs, &block) + link_args = { class: "#{brand}-breadcrumbs--link" }.deep_merge_html_attributes(kwargs) if block_given? - link_to(name, html_options, &block) + link_to(block.call, href, **link_args) else - link_to(name, options, html_options) + link_to(name, href, **link_args) end end private - def brand - Govuk::Components.brand + def new_tab_args(new_tab) + new_tab ? { target: "_blank", rel: "noreferrer noopener" } : {} end - def link_styles - { - inverse: "#{brand}-link--inverse", - muted: "#{brand}-link--muted", - no_underline: "#{brand}-link--no-underline", - no_visited_state: "#{brand}-link--no-visited-state", - text_colour: "#{brand}-link--text-colour", - } + def button_attributes(disabled) + disabled ? { disabled: true, aria: { disabled: true } } : {} end - def button_styles + def extract_link_args(new_tab: false, inverse: false, muted: false, no_underline: false, no_visited_state: false, text_colour: false, **kwargs) { - disabled: "#{brand}-button--disabled", - secondary: "#{brand}-button--secondary", - warning: "#{brand}-button--warning", - inverse: "#{brand}-button--inverse", - } + class: govuk_link_classes( + inverse: inverse, + muted: muted, + no_underline: no_underline, + no_visited_state: no_visited_state, + text_colour: text_colour + ), + **new_tab_args(new_tab) + }.deep_merge_html_attributes(kwargs) end - def build_html_options(provided_options, style: :link) - element_styles = { link: link_styles, button: button_styles }.fetch(style, {}) - - # we need to take a couple of extra steps here because we don't want the style - # params (inverse, muted, etc) to end up as extra attributes on the link. - - remaining_options = remove_styles_from_provided_options(element_styles, provided_options) - - style_classes = build_style_classes(style, extract_styles_from_provided_options(element_styles, provided_options)) - - combine_attributes(remaining_options, class_name: style_classes) - end - - def build_style_classes(style, provided_options) - keys = *provided_options&.keys - - case style - when :link then govuk_link_classes(*keys) - when :button then govuk_button_classes(*keys) - when :breadcrumb then "#{brand}-breadcrumbs__link" - end - end - - def combine_attributes(attributes, class_name:) - attributes ||= {} - - attributes.with_indifferent_access.tap do |attrs| - attrs[:class] = Array.wrap(attrs[:class]).prepend(class_name).flatten.join(" ") - end - end - - def extract_styles_from_provided_options(styles, provided_options) - return {} if provided_options.blank? - - provided_options.slice(*styles.keys) + def extract_button_args(new_tab: false, disabled: false, inverse: false, secondary: false, warning: false, **kwargs) + { + class: govuk_button_classes( + disabled: disabled, + inverse: inverse, + secondary: secondary, + warning: warning + ), + **button_attributes(disabled), + **new_tab_args(new_tab) + }.deep_merge_html_attributes(kwargs) end - def remove_styles_from_provided_options(styles, provided_options) - return {} if provided_options.blank? - - provided_options&.except(*styles.keys) + def brand + Govuk::Components.brand end end diff --git a/app/helpers/govuk_rails_compatibile_link_helper.rb b/app/helpers/govuk_rails_compatibile_link_helper.rb new file mode 100644 index 00000000..59e57b04 --- /dev/null +++ b/app/helpers/govuk_rails_compatibile_link_helper.rb @@ -0,0 +1,157 @@ +require "html_attributes_utils" + +module GovukRailsCompatibileLinkHelper + using HTMLAttributesUtils + + def govuk_link_classes(*styles, default_class: "#{brand}-link") + if (invalid_styles = (styles - link_styles.keys)) && invalid_styles.any? + fail(ArgumentError, "invalid styles #{invalid_styles.to_sentence}. Valid styles are #{link_styles.keys.to_sentence}") + end + + [default_class] + link_styles.values_at(*styles).compact + end + + def govuk_button_classes(*styles, default_class: "#{brand}-button") + if (invalid_styles = (styles - button_styles.keys)) && invalid_styles.any? + fail(ArgumentError, "invalid styles #{invalid_styles.to_sentence}. Valid styles are #{button_styles.keys.to_sentence}") + end + + [default_class] + button_styles.values_at(*styles).compact + end + + def govuk_link_to(name = nil, options = nil, extra_options = {}, &block) + extra_options = options if block_given? + html_options = build_html_options(extra_options) + + if block_given? + link_to(name, html_options, &block) + else + link_to(name, options, html_options) + end + end + + def govuk_mail_to(email_address, name = nil, extra_options = {}, &block) + extra_options = name if block_given? + html_options = build_html_options(extra_options) + + if block_given? + mail_to(email_address, html_options, &block) + else + mail_to(email_address, name, html_options) + end + end + + def govuk_button_to(name = nil, options = nil, extra_options = {}, &block) + extra_options = options if block_given? + html_options = { + data: { module: "govuk-button" } + } + + if extra_options && extra_options[:prevent_double_click] + html_options[:data]["prevent-double-click"] = "true" + extra_options = extra_options.except(:prevent_double_click) + end + + html_options.merge! build_html_options(extra_options, style: :button) + + if block_given? + button_to(options, html_options, &block) + else + button_to(name, options, html_options) + end + end + + def govuk_button_link_to(name = nil, options = nil, extra_options = {}, &block) + extra_options = options if block_given? + html_options = { + data: { module: "#{brand}-button" }, + draggable: 'false', + role: 'button', + }.merge build_html_options(extra_options, style: :button) + + if block_given? + link_to(name, html_options, &block) + else + link_to(name, options, html_options) + end + end + + def govuk_breadcrumb_link_to(name = nil, options = nil, extra_options = {}, &block) + extra_options = options if block_given? + html_options = build_html_options(extra_options, style: :breadcrumb) + + if block_given? + link_to(name, html_options, &block) + else + link_to(name, options, html_options) + end + end + +private + + def brand + Govuk::Components.brand + end + + def link_styles + { + inverse: "#{brand}-link--inverse", + muted: "#{brand}-link--muted", + no_underline: "#{brand}-link--no-underline", + no_visited_state: "#{brand}-link--no-visited-state", + text_colour: "#{brand}-link--text-colour", + } + end + + def button_styles + { + disabled: "#{brand}-button--disabled", + secondary: "#{brand}-button--secondary", + warning: "#{brand}-button--warning", + inverse: "#{brand}-button--inverse", + } + end + + def build_html_options(provided_options, style: :link) + element_styles = { link: link_styles, button: button_styles }.fetch(style, {}) + + # we need to take a couple of extra steps here because we don't want the style + # params (inverse, muted, etc) to end up as extra attributes on the link. + + remaining_options = remove_styles_from_provided_options(element_styles, provided_options) + + style_classes = build_style_classes(style, extract_styles_from_provided_options(element_styles, provided_options)) + + combine_attributes(remaining_options, class_name: style_classes) + end + + def build_style_classes(style, provided_options) + keys = *provided_options&.keys + + case style + when :link then govuk_link_classes(*keys) + when :button then govuk_button_classes(*keys) + when :breadcrumb then "#{brand}-breadcrumbs__link" + end + end + + def combine_attributes(attributes, class_name:) + attributes ||= {} + + attributes.with_indifferent_access.tap do |attrs| + attrs[:class] = Array.wrap(attrs[:class]).prepend(class_name).flatten.join(" ") + end + end + + def extract_styles_from_provided_options(styles, provided_options) + return {} if provided_options.blank? + + provided_options.slice(*styles.keys) + end + + def remove_styles_from_provided_options(styles, provided_options) + return {} if provided_options.blank? + + provided_options&.except(*styles.keys) + end +end diff --git a/spec/components/govuk_component/cookie_banner_component_spec.rb b/spec/components/govuk_component/cookie_banner_component_spec.rb index eb4050b5..110ed5a5 100644 --- a/spec/components/govuk_component/cookie_banner_component_spec.rb +++ b/spec/components/govuk_component/cookie_banner_component_spec.rb @@ -60,7 +60,7 @@ subject! do render_inline(described_class.new(**kwargs)) do |cookie_banner| cookie_banner.with_message(heading_text: custom_heading_text, role: custom_role, text: custom_message_text) do |message| - message.with_action { helper.govuk_button_link_to("/accept") { "Accept" } } + message.with_action { helper.govuk_button_link_to("Accept", "/accept") } message.with_action { helper.govuk_link_to("View cookie policy", "/cookie-policy") } end end diff --git a/spec/helpers/govuk_link_helper_spec.rb b/spec/helpers/govuk_link_helper_spec.rb index 8922341b..77ba0e6d 100644 --- a/spec/helpers/govuk_link_helper_spec.rb +++ b/spec/helpers/govuk_link_helper_spec.rb @@ -1,324 +1,429 @@ require 'spec_helper' +INVALID_BUTTON_COMBINATIONS = [ + { warning: true, inverse: true }, + { warning: true, secondary: true }, + { secondary: true, inverse: true }, + { secondary: true, inverse: true, warning: true }, +].freeze + +INVALID_LINK_COMBINATIONS = [ + { text_colour: true, inverse: true }, + { muted: true, inverse: true }, + { muted: true, text_colour: true }, + { muted: true, text_colour: true, inverse: true }, +].freeze + RSpec.describe(GovukLinkHelper, type: 'helper') do + include GovukLinkHelper include ActionView::Helpers::UrlHelper - include ActionView::Context - - let(:text) { 'Menu' } - let(:url) { '/stuff/menu/' } - let(:args) { [text, url] } - let(:kwargs) { {} } - describe '#govuk_link_classes' do - subject { govuk_link_classes(*args) } + before do + allow(self).to receive(:url_for).with(any_args).and_return("/world") + end - describe "by default" do - let(:args) { [] } + describe "govuk_link_to" do + let(:kwargs) { {} } + subject { govuk_link_to("hello", "/world", **kwargs) } - specify "contains only 'govuk-link'" do - expect(subject).to eql(%w(govuk-link)) - end + specify "renders a link with the correct class" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: "govuk-link" }) end - { - no_visited_state: 'govuk-link--no-visited-state', - muted: 'govuk-link--muted', - text_colour: 'govuk-link--text-colour', - inverse: 'govuk-link--inverse', - no_underline: 'govuk-link--no-underline', - }.each do |style, css_class| - describe "generating a #{style}-style link with '#{style}: true'" do - let(:args) { [style] } + context "calling with a block of content" do + subject { govuk_link_to("/world", **kwargs) { "hello" } } - specify %(contains 'govuk-link' and '#{css_class}') do - expect(subject).to match_array(['govuk-link', css_class]) - end + specify "renders a link with the correct class" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: "govuk-link" }) end end - end - - describe '#govuk_button_classes' do - subject { govuk_button_classes(*args) } - describe "by default" do - let(:args) { [] } + context "when inverse: true" do + let(:kwargs) { { inverse: true } } - specify "contains only 'govuk-button'" do - expect(subject).to eql(%w(govuk-button)) + specify "the inverse class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: %w(govuk-link govuk-link--inverse) }) end end - { - secondary: 'govuk-button--secondary', - warning: 'govuk-button--warning', - disabled: 'govuk-button--disabled', - inverse: 'govuk-button--inverse', - }.each do |style, css_class| - describe "generating a #{style}-style button with '#{style}: true'" do - let(:args) { [style] } + context "when muted: true" do + let(:kwargs) { { muted: true } } - specify %(contains 'govuk-button' and '#{css_class}') do - expect(subject).to match_array(['govuk-button', css_class]) - end + specify "the muted class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: %w(govuk-link govuk-link--muted) }) end end - end - describe "#govuk_link_to" do - let(:link_text) { 'Onwards!' } - let(:link_url) { '/some/link' } - let(:link_params) { { controller: :some_controller, action: :some_action } } + context "when no_underline: true" do + let(:kwargs) { { no_underline: true } } - before do - allow(self).to receive(:url_for).with(link_params).and_return(link_url) + specify "the no-underline class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: %w(govuk-link govuk-link--no-underline) }) + end end - context "when provided with a path instead of params" do - let(:path) { "/some/path" } - before { allow(self).to receive(:url_for).with(path).and_return(path) } - subject { govuk_link_to(path) { link_text } } + context "when no_visited_state: true" do + let(:kwargs) { { no_visited_state: true } } - specify "renders a link with the given path" do - expect(subject).to have_tag("a", with: { href: path, class: "govuk-link" }, text: link_text) + specify "the no-underline class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: %w(govuk-link govuk-link--no-visited-state) }) end end - context "when provided with link text and url params" do - subject { govuk_link_to link_text, link_params } + context "when text_colour: true" do + let(:kwargs) { { text_colour: true } } - it { is_expected.to have_tag('a', text: link_text, with: { href: link_url, class: 'govuk-link' }) } + specify "the no-underline class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: %w(govuk-link govuk-link--text-colour) }) + end end - context "when provided with url params and the block" do - let(:link_html) { tag.span(link_text) } - - subject { govuk_link_to(link_params) { link_html } } + context "when new_tab: true" do + let(:kwargs) { { new_tab: true } } - it { is_expected.to have_tag('a', with: { href: link_url }) { with_tag(:span, text: link_text) } } + specify "the new tab attributes are present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: "govuk-link", target: "_blank", rel: "noreferrer noopener" }) + end end - context "customising the GOV.UK link style" do - let(:custom_html_options) { { inverse: true } } + # the link modifiers text_colour, inverse, muted all change the link's text colour + # and shouldn't be used together + describe "invalid combinations" do + INVALID_LINK_COMBINATIONS.each do |invalid_combination| + context "when #{invalid_combination}" do + let(:kwargs) { invalid_combination } - subject { govuk_link_to(link_text, link_params, custom_html_options) } - - it { is_expected.to have_tag('a', with: { href: link_url, class: "govuk-link--inverse" }, text: link_text) } + specify "throws an error" do + expect { subject }.to raise_error("links can be either text_colour, inverse or muted - not combinations of the three") + end + end + end end - context "adding custom classes" do - let(:custom_class) { { class: "green" } } - - subject { govuk_link_to(link_text, link_params, { class: "green", no_underline: true }) } + context "when there are custom attributes" do + let(:kwargs) { { lang: "en-GB", dir: "ltr" } } - it { is_expected.to have_tag('a', with: { href: link_url, class: %w(green govuk-link--no-underline) }, text: link_text) } + specify "the custom attributes are present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: "govuk-link", lang: "en-GB", dir: "ltr" }) + end end end - describe "#govuk_mail_to" do - let(:link_text) { 'Send a message' } - let(:link_html) { tag.span(link_text) } - let(:email_address) { 'barney@gumble.net' } - let(:mailto_address) { "mailto:" + email_address } + describe "govuk_mail_to" do + let(:kwargs) { {} } + subject { govuk_mail_to("world@solar.system", "hello", **kwargs) } - context "when email address and link text are provided via args" do - subject { govuk_mail_to(email_address, link_text) } + specify "renders a link with the correct class" do + expect(subject).to have_tag("a", text: "hello", with: { href: "mailto:world@solar.system", class: "govuk-link" }) + end - it { is_expected.to have_tag('a', text: link_text, with: { href: mailto_address, class: 'govuk-link' }) } + context "calling with a block of content" do + subject { govuk_mail_to("world@solar.system", **kwargs) { "hello" } } - context "customising the GOV.UK link style" do - let(:custom_html_options) { { no_underline: true } } + specify "renders a link with the correct class" do + expect(subject).to have_tag("a", text: "hello", with: { href: "mailto:world@solar.system", class: "govuk-link" }) + end + end - subject { govuk_mail_to(email_address, link_text, custom_html_options) } + context "when inverse: true" do + let(:kwargs) { { inverse: true } } - it { is_expected.to have_tag('a', with: { href: mailto_address, class: "govuk-link--no-underline" }, text: link_text) } + specify "the inverse class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "mailto:world@solar.system", class: %w(govuk-link govuk-link--inverse) }) end end - context "when the link text is provided via an block" do - subject { govuk_mail_to(email_address) { link_html } } - - it { is_expected.to have_tag('a', with: { href: mailto_address }) { with_tag(:span, text: link_text) } } + context "when muted: true" do + let(:kwargs) { { muted: true } } - context "customising the GOV.UK link style" do - let(:custom_html_options) { { text_colour: true } } + specify "the muted class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "mailto:world@solar.system", class: %w(govuk-link govuk-link--muted) }) + end + end - subject { govuk_mail_to(email_address, custom_html_options) { link_html } } + context "when no_underline: true" do + let(:kwargs) { { no_underline: true } } - it { is_expected.to have_tag('a', with: { href: mailto_address, class: "govuk-link--text-colour" }) { with_tag(:span, text: link_text) } } + specify "the no-underline class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "mailto:world@solar.system", class: %w(govuk-link govuk-link--no-underline) }) end end - context "adding custom classes" do - let(:custom_class) { { class: "green" } } - - subject { govuk_mail_to(email_address, link_text, { class: "green", no_underline: true }) } + context "when no_visited_state: true" do + let(:kwargs) { { no_visited_state: true } } - it { is_expected.to have_tag('a', with: { href: mailto_address, class: %w(green govuk-link--no-underline) }, text: link_text) } + specify "the no-underline class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "mailto:world@solar.system", class: %w(govuk-link govuk-link--no-visited-state) }) + end end - end - describe "#govuk_button_to" do - let(:button_text) { 'Do something' } - let(:button_url) { '/some/action' } - let(:button_params) { { controller: :some_controller, action: :some_action } } + context "when text_colour: true" do + let(:kwargs) { { text_colour: true } } - before do - allow(self).to receive(:url_for).with(anything).and_return(button_url) + specify "the no-underline class is present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "mailto:world@solar.system", class: %w(govuk-link govuk-link--text-colour) }) + end end - context "when provided with button text and url params" do - subject { govuk_button_to(button_text, button_params) } + context "when new_tab: true" do + let(:kwargs) { { new_tab: true } } - specify "renders a form with a button and the correct attributes" do - expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do - with_tag("button", with: { class: "govuk-button", "data-module": "govuk-button" }, text: button_text) - end + specify "the new tab attributes are present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "mailto:world@solar.system", class: "govuk-link", target: "_blank", rel: "noreferrer noopener" }) end end - context "when provided with button text, a path and preventing double click" do - subject { govuk_button_to(button_text, "/", prevent_double_click: true) } + # the link modifiers text_colour, inverse, muted all change the link's text colour + # and shouldn't be used together + describe "invalid combinations" do + INVALID_LINK_COMBINATIONS.each do |invalid_combination| + context "when #{invalid_combination}" do + let(:kwargs) { invalid_combination } - specify "renders a form with a button and the correct attributes" do - expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do - with_tag("button", with: { class: "govuk-button", "data-module": "govuk-button", "data-prevent-double-click": "true" }, text: button_text) + specify "throws an error" do + expect { subject }.to raise_error("links can be either text_colour, inverse or muted - not combinations of the three") + end end end end - context "when provided with url params and a block" do - let(:button_html) { tag.span(button_text) } - - subject { govuk_button_to(button_params) { button_html } } + context "when there are custom attributes" do + let(:kwargs) { { lang: "en-GB", dir: "ltr" } } - specify "renders a form with a button and the correct attributes" do - expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do - with_tag("button", with: { class: "govuk-button", "data-module": "govuk-button" }, text: button_text) - end + specify "the custom attributes are present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "mailto:world@solar.system", class: "govuk-link", lang: "en-GB", dir: "ltr" }) end end + end - context "customising the GOV.UK button style" do - let(:custom_button_options) { { secondary: true } } + describe "govuk_button_link_to" do + let(:kwargs) { {} } + subject { govuk_button_link_to("hello", "/world", **kwargs) } - subject { govuk_button_to(button_text, button_params, custom_button_options) } + specify "renders a link styled as a button with the correct class" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: "govuk-button" }) + end - specify "renders a form with a button that has the GOV.UK modifier classes" do - expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do - with_tag("button", with: { class: %w(govuk-button govuk-button--secondary), "data-module": "govuk-button" }, text: button_text) - end + context "calling with a block of content" do + subject { govuk_button_link_to("/world", **kwargs) { "hello" } } + + specify "renders a link with the correct class" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: "govuk-button" }) end end - context "adding custom classes" do - subject { govuk_button_to(button_text, button_params, { class: "yellow", disabled: true }) } + context "when disabled: true" do + let(:kwargs) { { disabled: true } } - specify "renders a form with an button that has the custom classes" do - expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do - with_tag("button", with: { type: "submit", class: %w(govuk-button yellow govuk-button--disabled), "data-module": "govuk-button" }, text: button_text) - end + specify "the disabled class is present on the button link" do + expect(subject).to have_tag( + "a", + text: "hello", + with: { + href: "/world", + class: %w(govuk-button govuk-button--disabled), + disabled: "disabled", + "aria-disabled" => true, + } + ) end end - end - describe "#govuk_button_link_to" do - let(:button_link_text) { 'Activate!' } - let(:button_link_url) { '/another/link' } - let(:link_params) { { controller: :some_controller, action: :some_action } } + context "when inverse: true" do + let(:kwargs) { { inverse: true } } - before do - allow(self).to receive(:url_for).with(link_params).and_return(button_link_url) + specify "the inverse class is present on the button link" do + expect(subject).to have_tag( + "a", + text: "hello", + with: { + href: "/world", + class: %w(govuk-button govuk-button--inverse), + } + ) + end end - context "when provided with button text and url params" do - subject { govuk_button_link_to(button_link_text, link_params) } + context "when secondary: true" do + let(:kwargs) { { secondary: true } } - specify "renders a link styled as a button with the correct attributes" do + specify "the secondary class is present on the button link" do expect(subject).to have_tag( "a", + text: "hello", with: { - href: button_link_url, - class: "govuk-button", - draggable: false, - role: "button", - "data-module": "govuk-button" - }, - text: button_link_text + href: "/world", + class: %w(govuk-button govuk-button--secondary), + } ) end end - context "when provided with a path instead of params" do - let(:path) { "/some/path" } - before { allow(self).to receive(:url_for).with(path).and_return(path) } - subject { govuk_button_link_to(path) { button_link_text } } + context "when warning: true" do + let(:kwargs) { { warning: true } } - specify "renders a link with the given path" do - expect(subject).to have_tag("a", with: { href: path, class: "govuk-button" }, text: button_link_text) + specify "the warning class is present on the button link" do + expect(subject).to have_tag( + "a", + text: "hello", + with: { + href: "/world", + class: %w(govuk-button govuk-button--warning), + } + ) end end - context "when provided with url params and a block" do - let(:button_html) { tag.span(button_text) } + context "when new_tab: true" do + let(:kwargs) { { new_tab: true } } - subject { govuk_button_link_to(link_params) { button_link_text } } - - specify "renders a link styled as a button with the correct attributes" do - expect(subject).to have_tag("a", with: { href: button_link_url, class: "govuk-button" }, text: button_link_text) + specify "the warning class is present on the button link" do + expect(subject).to have_tag( + "a", + text: "hello", + with: { + href: "/world", + class: %w(govuk-button), + target: "_blank", + rel: "noreferrer noopener" + } + ) end end - context "customising the GOV.UK button style" do - let(:custom_link_button_options) { { secondary: true } } + # a button can be disabled in combination with other styles but cannot + # be called with more than one of eitehr warning, inverse or secondary + describe "invalid combinations" do + INVALID_BUTTON_COMBINATIONS.each do |invalid_combination| + context "when #{invalid_combination}" do + let(:kwargs) { invalid_combination } - subject { govuk_button_link_to(button_link_text, link_params, custom_link_button_options) } - - specify "renders a form with an button that has the GOV.UK modifier classes" do - expect(subject).to have_tag("a", with: { href: button_link_url, class: %w(govuk-button govuk-button--secondary) }, text: button_link_text) + specify "throws an error" do + expect { subject }.to raise_error("buttons can be either inverse, secondary or warning - not combinations of the three") + end + end end end - context "adding custom classes" do - subject { govuk_button_link_to(button_link_text, link_params, { class: "yellow", disabled: true }) } + context "when there are custom attributes" do + let(:kwargs) { { lang: "en-GB", dir: "ltr" } } - specify "renders a form with an button that has the custom classes" do - expect(subject).to have_tag("a", with: { href: button_link_url, class: %w(govuk-button yellow) }, text: button_link_text) + specify "the custom attributes are present on the link" do + expect(subject).to have_tag("a", text: "hello", with: { href: "/world", class: "govuk-button", lang: "en-GB", dir: "ltr" }) end end end - describe "#govuk_breadcrumb_link_to" do - let(:breadcrumb_text) { 'Grandparent' } - let(:breadcrumb_url) { '/several/levels/up' } + describe "govuk_button_to" do + let(:kwargs) { {} } + subject { govuk_button_to("hello", "/world", **kwargs) } - before do - allow(self).to receive(:url_for).with(breadcrumb_params).and_return(breadcrumb_url) + specify "renders a form with a button that has the right attributes and classes" do + expect(subject).to have_tag("form", with: { method: "post", action: "/world" }) do + with_tag("button", with: { class: "govuk-button" }, text: "hello") + end end - context "when provided with text and url params" do - let(:breadcrumb_params) { { controller: :some_controller, action: :some_action } } + context "calling with a block of content" do + subject { govuk_button_to("/world", **kwargs) { "hello" } } - subject { govuk_breadcrumb_link_to(breadcrumb_text, breadcrumb_params) } + specify "renders a form with a button that has the right attributes and classes" do + expect(subject).to have_tag("form", with: { method: "post", action: "/world" }) do + with_tag("button", with: { class: "govuk-button" }, text: "hello") + end + end + end - it { is_expected.to have_tag('a', text: breadcrumb_text, with: { href: breadcrumb_url, class: 'govuk-breadcrumbs__link' }) } + context "when disabled: true" do + let(:kwargs) { { disabled: true } } + + specify "the disabled class is present on the button" do + expect(subject).to have_tag("form", with: { method: "post", action: "/world" }) do + with_tag( + "button", + text: "hello", + with: { + class: %w[govuk-button govuk-button--disabled], + disabled: "disabled", + "aria-disabled" => true, + } + ) + end + end end - context "when provided with url params and the block" do - let(:breadcrumb_html) { tag.span(breadcrumb_text) } - let(:breadcrumb_params) { { controller: :some_controller, action: :some_action } } + context "when inverse: true" do + let(:kwargs) { { inverse: true } } + + specify "the inverse class is present on the button" do + expect(subject).to have_tag("form", with: { method: "post", action: "/world" }) do + with_tag( + "button", + text: "hello", + with: { class: %w[govuk-button govuk-button--inverse] } + ) + end + end + end - subject { govuk_breadcrumb_link_to(breadcrumb_params) { breadcrumb_html } } + context "when secondary: true" do + let(:kwargs) { { secondary: true } } - it { is_expected.to have_tag('a', with: { href: breadcrumb_url }) { with_tag(:span, text: breadcrumb_text) } } + specify "the secondary class is present on the button" do + expect(subject).to have_tag("form", with: { method: "post", action: "/world" }) do + with_tag( + "button", + text: "hello", + with: { class: %w[govuk-button govuk-button--secondary] } + ) + end + end end - context "adding custom classes" do - let(:breadcrumb_params) { { controller: :some_controller, action: :some_action } } - let(:custom_class) { "emphatic" } + context "when warning: true" do + let(:kwargs) { { warning: true } } - subject { govuk_breadcrumb_link_to(breadcrumb_text, breadcrumb_params, { class: custom_class }) } + specify "the warning class is present on the button" do + expect(subject).to have_tag("form", with: { method: "post", action: "/world" }) do + with_tag( + "button", + text: "hello", + with: { class: %w[govuk-button govuk-button--warning] } + ) + end + end + end + + # a button can be disabled in combination with other styles but cannot + # be called with more than one of eitehr warning, inverse or secondary + describe "invalid combinations" do + INVALID_BUTTON_COMBINATIONS.each do |invalid_combination| + context "when #{invalid_combination}" do + let(:kwargs) { invalid_combination } - it { is_expected.to have_tag('a', with: { href: breadcrumb_url, class: ['govuk-breadcrumbs__link', custom_class] }, text: breadcrumb_text) } + specify "throws an error" do + expect { subject }.to raise_error("buttons can be either inverse, secondary or warning - not combinations of the three") + end + end + end + end + + context "when there are custom attributes" do + let(:kwargs) { { lang: "en-GB", dir: "ltr" } } + + specify "the custom attributes are present on the button" do + expect(subject).to have_tag("form", with: { method: "post", action: "/world" }) do + with_tag( + "button", + text: "hello", + with: { + class: %w[govuk-button], + dir: "ltr", + lang: "en-GB", + } + ) + end + end end end end diff --git a/spec/helpers/govuk_rails_compatibile_link_helper_spec.rb b/spec/helpers/govuk_rails_compatibile_link_helper_spec.rb new file mode 100644 index 00000000..1d273487 --- /dev/null +++ b/spec/helpers/govuk_rails_compatibile_link_helper_spec.rb @@ -0,0 +1,325 @@ +require 'spec_helper' + +RSpec.describe(GovukRailsCompatibileLinkHelper, type: 'helper') do + include ActionView::Helpers::UrlHelper + include ActionView::Context + include GovukRailsCompatibileLinkHelper + + let(:text) { 'Menu' } + let(:url) { '/stuff/menu/' } + let(:args) { [text, url] } + let(:kwargs) { {} } + + describe '#govuk_link_classes' do + subject { govuk_link_classes(*args) } + + describe "by default" do + let(:args) { [] } + + specify "contains only 'govuk-link'" do + expect(subject).to eql(%w(govuk-link)) + end + end + + { + no_visited_state: 'govuk-link--no-visited-state', + muted: 'govuk-link--muted', + text_colour: 'govuk-link--text-colour', + inverse: 'govuk-link--inverse', + no_underline: 'govuk-link--no-underline', + }.each do |style, css_class| + describe "generating a #{style}-style link with '#{style}: true'" do + let(:args) { [style] } + + specify %(contains 'govuk-link' and '#{css_class}') do + expect(subject).to match_array(['govuk-link', css_class]) + end + end + end + end + + describe '#govuk_button_classes' do + subject { govuk_button_classes(*args) } + + describe "by default" do + let(:args) { [] } + + specify "contains only 'govuk-button'" do + expect(subject).to eql(%w(govuk-button)) + end + end + + { + secondary: 'govuk-button--secondary', + warning: 'govuk-button--warning', + disabled: 'govuk-button--disabled', + inverse: 'govuk-button--inverse', + }.each do |style, css_class| + describe "generating a #{style}-style button with '#{style}: true'" do + let(:args) { [style] } + + specify %(contains 'govuk-button' and '#{css_class}') do + expect(subject).to match_array(['govuk-button', css_class]) + end + end + end + end + + describe "#govuk_link_to" do + let(:link_text) { 'Onwards!' } + let(:link_url) { '/some/link' } + let(:link_params) { { controller: :some_controller, action: :some_action } } + + before do + allow(self).to receive(:url_for).with(link_params).and_return(link_url) + end + + context "when provided with a path instead of params" do + let(:path) { "/some/path" } + before { allow(self).to receive(:url_for).with(path).and_return(path) } + subject { govuk_link_to(path) { link_text } } + + specify "renders a link with the given path" do + expect(subject).to have_tag("a", with: { href: path, class: "govuk-link" }, text: link_text) + end + end + + context "when provided with link text and url params" do + subject { govuk_link_to link_text, link_params } + + it { is_expected.to have_tag('a', text: link_text, with: { href: link_url, class: 'govuk-link' }) } + end + + context "when provided with url params and the block" do + let(:link_html) { tag.span(link_text) } + + subject { govuk_link_to(link_params) { link_html } } + + it { is_expected.to have_tag('a', with: { href: link_url }) { with_tag(:span, text: link_text) } } + end + + context "customising the GOV.UK link style" do + let(:custom_html_options) { { inverse: true } } + + subject { govuk_link_to(link_text, link_params, custom_html_options) } + + it { is_expected.to have_tag('a', with: { href: link_url, class: "govuk-link--inverse" }, text: link_text) } + end + + context "adding custom classes" do + let(:custom_class) { { class: "green" } } + + subject { govuk_link_to(link_text, link_params, { class: "green", no_underline: true }) } + + it { is_expected.to have_tag('a', with: { href: link_url, class: %w(green govuk-link--no-underline) }, text: link_text) } + end + end + + describe "#govuk_mail_to" do + let(:link_text) { 'Send a message' } + let(:link_html) { tag.span(link_text) } + let(:email_address) { 'barney@gumble.net' } + let(:mailto_address) { "mailto:" + email_address } + + context "when email address and link text are provided via args" do + subject { govuk_mail_to(email_address, link_text) } + + it { is_expected.to have_tag('a', text: link_text, with: { href: mailto_address, class: 'govuk-link' }) } + + context "customising the GOV.UK link style" do + let(:custom_html_options) { { no_underline: true } } + + subject { govuk_mail_to(email_address, link_text, custom_html_options) } + + it { is_expected.to have_tag('a', with: { href: mailto_address, class: "govuk-link--no-underline" }, text: link_text) } + end + end + + context "when the link text is provided via an block" do + subject { govuk_mail_to(email_address) { link_html } } + + it { is_expected.to have_tag('a', with: { href: mailto_address }) { with_tag(:span, text: link_text) } } + + context "customising the GOV.UK link style" do + let(:custom_html_options) { { text_colour: true } } + + subject { govuk_mail_to(email_address, custom_html_options) { link_html } } + + it { is_expected.to have_tag('a', with: { href: mailto_address, class: "govuk-link--text-colour" }) { with_tag(:span, text: link_text) } } + end + end + + context "adding custom classes" do + let(:custom_class) { { class: "green" } } + + subject { govuk_mail_to(email_address, link_text, { class: "green", no_underline: true }) } + + it { is_expected.to have_tag('a', with: { href: mailto_address, class: %w(green govuk-link--no-underline) }, text: link_text) } + end + end + + describe "#govuk_button_to" do + let(:button_text) { 'Do something' } + let(:button_url) { '/some/action' } + let(:button_params) { { controller: :some_controller, action: :some_action } } + + before do + allow(self).to receive(:url_for).with(anything).and_return(button_url) + end + + context "when provided with button text and url params" do + subject { govuk_button_to(button_text, button_params) } + + specify "renders a form with a button and the correct attributes" do + expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do + with_tag("button", with: { class: "govuk-button", "data-module": "govuk-button" }, text: button_text) + end + end + end + + context "when provided with button text, a path and preventing double click" do + subject { govuk_button_to(button_text, "/", prevent_double_click: true) } + + specify "renders a form with a button and the correct attributes" do + expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do + with_tag("button", with: { class: "govuk-button", "data-module": "govuk-button", "data-prevent-double-click": "true" }, text: button_text) + end + end + end + + context "when provided with url params and a block" do + let(:button_html) { tag.span(button_text) } + + subject { govuk_button_to(button_params) { button_html } } + + specify "renders a form with a button and the correct attributes" do + expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do + with_tag("button", with: { class: "govuk-button", "data-module": "govuk-button" }, text: button_text) + end + end + end + + context "customising the GOV.UK button style" do + let(:custom_button_options) { { secondary: true } } + + subject { govuk_button_to(button_text, button_params, custom_button_options) } + + specify "renders a form with a button that has the GOV.UK modifier classes" do + expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do + with_tag("button", with: { class: %w(govuk-button govuk-button--secondary), "data-module": "govuk-button" }, text: button_text) + end + end + end + + context "adding custom classes" do + subject { govuk_button_to(button_text, button_params, { class: "yellow", disabled: true }) } + + specify "renders a form with an button that has the custom classes" do + expect(subject).to have_tag("form", with: { class: "button_to", action: button_url }) do + with_tag("button", with: { type: "submit", class: %w(govuk-button yellow govuk-button--disabled), "data-module": "govuk-button" }, text: button_text) + end + end + end + end + + describe "#govuk_button_link_to" do + let(:button_link_text) { 'Activate!' } + let(:button_link_url) { '/another/link' } + let(:link_params) { { controller: :some_controller, action: :some_action } } + + before do + allow(self).to receive(:url_for).with(link_params).and_return(button_link_url) + end + + context "when provided with button text and url params" do + subject { govuk_button_link_to(button_link_text, link_params) } + + specify "renders a link styled as a button with the correct attributes" do + expect(subject).to have_tag( + "a", + with: { + href: button_link_url, + class: "govuk-button", + draggable: false, + role: "button", + "data-module": "govuk-button" + }, + text: button_link_text + ) + end + end + + context "when provided with a path instead of params" do + let(:path) { "/some/path" } + before { allow(self).to receive(:url_for).with(path).and_return(path) } + subject { govuk_button_link_to(path) { button_link_text } } + + specify "renders a link with the given path" do + expect(subject).to have_tag("a", with: { href: path, class: "govuk-button" }, text: button_link_text) + end + end + + context "when provided with url params and a block" do + let(:button_html) { tag.span(button_text) } + + subject { govuk_button_link_to(link_params) { button_link_text } } + + specify "renders a link styled as a button with the correct attributes" do + expect(subject).to have_tag("a", with: { href: button_link_url, class: "govuk-button" }, text: button_link_text) + end + end + + context "customising the GOV.UK button style" do + let(:custom_link_button_options) { { secondary: true } } + + subject { govuk_button_link_to(button_link_text, link_params, custom_link_button_options) } + + specify "renders a form with an button that has the GOV.UK modifier classes" do + expect(subject).to have_tag("a", with: { href: button_link_url, class: %w(govuk-button govuk-button--secondary) }, text: button_link_text) + end + end + + context "adding custom classes" do + subject { govuk_button_link_to(button_link_text, link_params, { class: "yellow", disabled: true }) } + + specify "renders a form with an button that has the custom classes" do + expect(subject).to have_tag("a", with: { href: button_link_url, class: %w(govuk-button yellow) }, text: button_link_text) + end + end + end + + describe "#govuk_breadcrumb_link_to" do + let(:breadcrumb_text) { 'Grandparent' } + let(:breadcrumb_url) { '/several/levels/up' } + + before do + allow(self).to receive(:url_for).with(breadcrumb_params).and_return(breadcrumb_url) + end + + context "when provided with text and url params" do + let(:breadcrumb_params) { { controller: :some_controller, action: :some_action } } + + subject { govuk_breadcrumb_link_to(breadcrumb_text, breadcrumb_params) } + + it { is_expected.to have_tag('a', text: breadcrumb_text, with: { href: breadcrumb_url, class: 'govuk-breadcrumbs__link' }) } + end + + context "when provided with url params and the block" do + let(:breadcrumb_html) { tag.span(breadcrumb_text) } + let(:breadcrumb_params) { { controller: :some_controller, action: :some_action } } + + subject { govuk_breadcrumb_link_to(breadcrumb_params) { breadcrumb_html } } + + it { is_expected.to have_tag('a', with: { href: breadcrumb_url }) { with_tag(:span, text: breadcrumb_text) } } + end + + context "adding custom classes" do + let(:breadcrumb_params) { { controller: :some_controller, action: :some_action } } + let(:custom_class) { "emphatic" } + + subject { govuk_breadcrumb_link_to(breadcrumb_text, breadcrumb_params, { class: custom_class }) } + + it { is_expected.to have_tag('a', with: { href: breadcrumb_url, class: ['govuk-breadcrumbs__link', custom_class] }, text: breadcrumb_text) } + end + end +end From e925f0a3293859913b00485d701b994b23a5b1c5 Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Sun, 11 Jun 2023 10:50:04 +0100 Subject: [PATCH 2/4] Replace Nanoc's LinkTo with Rails' UrlHelper Nanoc's link helpers expect classes to be provided in a string but now we're using arrays internally throughout so it makes sense to use the Rails version. --- guide/lib/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guide/lib/helpers.rb b/guide/lib/helpers.rb index 1bff7607..72246793 100644 --- a/guide/lib/helpers.rb +++ b/guide/lib/helpers.rb @@ -16,8 +16,8 @@ class Application < Rails::Application; end Dir.glob(File.join('./lib', '**', '*.rb')).sort.each { |f| require f } +use_helper ActionView::Helpers::UrlHelper use_helper Nanoc::Helpers::Rendering -use_helper Nanoc::Helpers::LinkTo use_helper Nanoc::Helpers::XMLSitemap use_helper Helpers::LinkHelpers use_helper Helpers::TitleAnchorHelpers From 3a4ec34ccf8551112ff841af293390f87cfe6498 Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Sun, 11 Jun 2023 10:54:16 +0100 Subject: [PATCH 3/4] Update guide link examples to use keyword args Now we're using the new link helper we need to use keyword arguments instead of hashes or arrays. --- guide/lib/examples/link_helpers.rb | 20 ++++++++++---------- guide/lib/helpers.rb | 2 ++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/guide/lib/examples/link_helpers.rb b/guide/lib/examples/link_helpers.rb index 44086c69..98a1a394 100644 --- a/guide/lib/examples/link_helpers.rb +++ b/guide/lib/examples/link_helpers.rb @@ -16,23 +16,23 @@ def govuk_link_to_new_tab end def govuk_link_to_inverse - %(= govuk_link_to('An inverse hyperlink', '#', { inverse: true })) + %(= govuk_link_to('An inverse hyperlink', '#', inverse: true)) end def govuk_link_to_muted - %(= govuk_link_to('A muted hyperlink', '#', { muted: true })) + %(= govuk_link_to('A muted hyperlink', '#', muted: true)) end def govuk_link_other_styles <<~LINKS p - = govuk_link_to('A hyperlink without an underline', '#', { no_underline: true }) + = govuk_link_to('A hyperlink without an underline', '#', no_underline: true ) p - = govuk_link_to('A hyperlink without a visited state', '#', { no_visited_state: true }) + = govuk_link_to('A hyperlink without a visited state', '#', no_visited_state: true) p - = govuk_link_to('A text-coloured hyperlink', '#', { text_colour: true }) + = govuk_link_to('A text-coloured hyperlink', '#', text_colour: true) LINKS end @@ -46,16 +46,16 @@ def govuk_button_link_to_normal def govuk_button_inverse <<~BUTTON - = govuk_button_link_to('An inverse button', '#', { inverse: true }) + = govuk_button_link_to('An inverse button', '#', inverse: true) BUTTON end def govuk_button_other_styles <<~BUTTONS .govuk-button-group - = govuk_button_link_to('A disabled button', '#', { disabled: true }) - = govuk_button_link_to('A secondary button', '#', { secondary: true }) - = govuk_button_link_to('A warning button', '#', { warning: true }) + = govuk_button_link_to('A disabled button', '#', disabled: true) + = govuk_button_link_to('A secondary button', '#', secondary: true) + = govuk_button_link_to('A warning button', '#', warning: true) BUTTONS end @@ -71,7 +71,7 @@ def govuk_link_classes_multiple = link_to_if(true, 'A muted and not underlined link generated by Rails', '#', - class: govuk_link_classes(:muted, :no_underline)) + class: govuk_link_classes(muted: true, no_underline: true)) LINK end end diff --git a/guide/lib/helpers.rb b/guide/lib/helpers.rb index 72246793..42e5cd85 100644 --- a/guide/lib/helpers.rb +++ b/guide/lib/helpers.rb @@ -110,3 +110,5 @@ class Application < Rails::Application; end use_helper Examples::CommonOptionsHelpers use_helper Examples::BackToTopLinkHelpers use_helper Examples::TitleWithErrorPrefixHelpers + +ActiveSupport.on_load(:action_view) { include GovukLinkHelper } From ceba1d0275a8ad97a775f8fc14ed4868723a79c4 Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Fri, 24 Nov 2023 12:07:25 +0000 Subject: [PATCH 4/4] Reword error messages for bad class combinations Co-authored-by: Frankie Roberto --- app/helpers/govuk_link_helper.rb | 58 +++++++++++++------------- spec/helpers/govuk_link_helper_spec.rb | 8 ++-- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/app/helpers/govuk_link_helper.rb b/app/helpers/govuk_link_helper.rb index 9c16cc3a..e696f20d 100644 --- a/app/helpers/govuk_link_helper.rb +++ b/app/helpers/govuk_link_helper.rb @@ -3,35 +3,6 @@ module GovukLinkHelper using HTMLAttributesUtils - def govuk_link_classes(inverse: false, muted: false, no_underline: false, no_visited_state: false, text_colour: false) - if [text_colour, inverse, muted].count(true) > 1 - fail("links can be either text_colour, inverse or muted - not combinations of the three") - end - - class_names( - "#{brand}-link", - "#{brand}-link--inverse" => inverse, - "#{brand}-link--muted" => muted, - "#{brand}-link--no-underline" => no_underline, - "#{brand}-link--no-visited-state" => no_visited_state, - "#{brand}-link--text-colour" => text_colour, - ) - end - - def govuk_button_classes(disabled: false, inverse: false, secondary: false, warning: false) - if [inverse, secondary, warning].count(true) > 1 - fail("buttons can be either inverse, secondary or warning - not combinations of the three") - end - - class_names( - "#{brand}-button", - "#{brand}-button--disabled" => disabled, - "#{brand}-button--inverse" => inverse, - "#{brand}-button--secondary" => secondary, - "#{brand}-button--warning" => warning, - ) - end - def govuk_link_to(name, href = nil, new_tab: false, inverse: false, muted: false, no_underline: false, no_visited_state: false, text_colour: false, **kwargs, &block) link_args = extract_link_args(new_tab: new_tab, inverse: inverse, muted: muted, no_underline: no_underline, no_visited_state: no_visited_state, text_colour: text_colour, **kwargs) @@ -82,6 +53,35 @@ def govuk_breadcrumb_link_to(name, href = nil, **kwargs, &block) end end + def govuk_link_classes(inverse: false, muted: false, no_underline: false, no_visited_state: false, text_colour: false) + if [text_colour, inverse, muted].count(true) > 1 + fail("links can be only be one of text_colour, inverse or muted") + end + + class_names( + "#{brand}-link", + "#{brand}-link--inverse" => inverse, + "#{brand}-link--muted" => muted, + "#{brand}-link--no-underline" => no_underline, + "#{brand}-link--no-visited-state" => no_visited_state, + "#{brand}-link--text-colour" => text_colour, + ) + end + + def govuk_button_classes(disabled: false, inverse: false, secondary: false, warning: false) + if [inverse, secondary, warning].count(true) > 1 + fail("buttons can only be one of inverse, secondary or warning") + end + + class_names( + "#{brand}-button", + "#{brand}-button--disabled" => disabled, + "#{brand}-button--inverse" => inverse, + "#{brand}-button--secondary" => secondary, + "#{brand}-button--warning" => warning, + ) + end + private def new_tab_args(new_tab) diff --git a/spec/helpers/govuk_link_helper_spec.rb b/spec/helpers/govuk_link_helper_spec.rb index 77ba0e6d..22bbac6e 100644 --- a/spec/helpers/govuk_link_helper_spec.rb +++ b/spec/helpers/govuk_link_helper_spec.rb @@ -94,7 +94,7 @@ let(:kwargs) { invalid_combination } specify "throws an error" do - expect { subject }.to raise_error("links can be either text_colour, inverse or muted - not combinations of the three") + expect { subject }.to raise_error("links can be only be one of text_colour, inverse or muted") end end end @@ -181,7 +181,7 @@ let(:kwargs) { invalid_combination } specify "throws an error" do - expect { subject }.to raise_error("links can be either text_colour, inverse or muted - not combinations of the three") + expect { subject }.to raise_error("links can be only be one of text_colour, inverse or muted") end end end @@ -299,7 +299,7 @@ let(:kwargs) { invalid_combination } specify "throws an error" do - expect { subject }.to raise_error("buttons can be either inverse, secondary or warning - not combinations of the three") + expect { subject }.to raise_error("buttons can only be one of inverse, secondary or warning") end end end @@ -402,7 +402,7 @@ let(:kwargs) { invalid_combination } specify "throws an error" do - expect { subject }.to raise_error("buttons can be either inverse, secondary or warning - not combinations of the three") + expect { subject }.to raise_error("buttons can only be one of inverse, secondary or warning") end end end