From a808edfd86435b3c40e0260021b7f84922e8b29e Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 13:46:29 -0500 Subject: [PATCH 01/19] Refactor button icon placement styling Anticipate simpler style defaults --- app/assets/stylesheets/components/_icon.scss | 15 +++++++++++++++ app/components/clipboard_button_component.rb | 7 +------ app/views/shared/_personal_key.html.erb | 8 ++++---- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/components/_icon.scss b/app/assets/stylesheets/components/_icon.scss index 5314c2f3fe2..9d782acd81b 100644 --- a/app/assets/stylesheets/components/_icon.scss +++ b/app/assets/stylesheets/components/_icon.scss @@ -1,3 +1,18 @@ +// Upstream: https://github.com/uswds/uswds/pull/4493 +.usa-icon { + .usa-button > &:first-child { + // Note that unlike the upstream pull request, there should not be any margins offsetting to + // account for line height, since Login.gov Design System normalizes button line height to 1. + float: left; + margin-right: 0.25rem; + } + + .usa-button:not(.usa-button--unstyled) > &:first-child { + margin-left: -0.5rem; + margin-right: 0.5rem; + } +} + .ico { &::before { background-repeat: no-repeat; diff --git a/app/components/clipboard_button_component.rb b/app/components/clipboard_button_component.rb index 4dce02116b9..cbdcb1cfd4f 100644 --- a/app/components/clipboard_button_component.rb +++ b/app/components/clipboard_button_component.rb @@ -13,11 +13,6 @@ def call end def content - safe_join( - [ - render(IconComponent.new(icon: :content_copy, class: 'position-absolute')), - content_tag(:span, t('links.copy'), class: 'padding-left-3'), - ], - ) + safe_join([render(IconComponent.new(icon: :content_copy)), t('links.copy')]) end end diff --git a/app/views/shared/_personal_key.html.erb b/app/views/shared/_personal_key.html.erb index 72044332d02..329cb9383cb 100644 --- a/app/views/shared/_personal_key.html.erb +++ b/app/views/shared/_personal_key.html.erb @@ -12,16 +12,16 @@ idv_download_personal_key_path, class: 'usa-button usa-button--outline margin-right-2 margin-bottom-2 tablet:margin-bottom-0', ) do %> - <%= render IconComponent.new(icon: :file_download, class: 'position-absolute') %> - <%= t('forms.backup_code.download') %> + <%= render IconComponent.new(icon: :file_download) %> + <%= t('forms.backup_code.download') %> <% end %> <%= button_tag( type: :button, data: { print: true }, class: 'usa-button usa-button--outline margin-right-2 margin-bottom-2 tablet:margin-bottom-0', ) do %> - <%= render IconComponent.new(icon: :print, class: 'position-absolute') %> - <%= t('users.personal_key.print') %> + <%= render IconComponent.new(icon: :print) %> + <%= t('users.personal_key.print') %> <% end %> <%= render ClipboardButtonComponent.new( clipboard_text: code, From ba5fe5ea75d060929854194a62d0acd7e30f7d9e Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 14:08:23 -0500 Subject: [PATCH 02/19] Use design system icons for icon buttons **Why**: So that we use a consistent icon set conforming to the design system rather than ad-hoc icons, and can make use of the IconComponent convenience abstraction. --- app/assets/images/ico-download.svg | 1 - app/assets/images/ico-print.svg | 1 - app/assets/images/ico-refresh.svg | 1 - app/assets/stylesheets/components/_icon.scss | 24 ------------------- app/assets/stylesheets/print.scss | 3 +-- app/views/idv/otp_verification/show.html.erb | 3 ++- .../otp_verification/show.html.erb | 8 ++++--- .../users/backup_code_setup/create.html.erb | 17 +++++++++---- 8 files changed, 20 insertions(+), 38 deletions(-) delete mode 100644 app/assets/images/ico-download.svg delete mode 100644 app/assets/images/ico-print.svg delete mode 100644 app/assets/images/ico-refresh.svg diff --git a/app/assets/images/ico-download.svg b/app/assets/images/ico-download.svg deleted file mode 100644 index bf23b5f8651..00000000000 --- a/app/assets/images/ico-download.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/app/assets/images/ico-print.svg b/app/assets/images/ico-print.svg deleted file mode 100644 index 83278594b97..00000000000 --- a/app/assets/images/ico-print.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/app/assets/images/ico-refresh.svg b/app/assets/images/ico-refresh.svg deleted file mode 100644 index 2c0081eaba6..00000000000 --- a/app/assets/images/ico-refresh.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/app/assets/stylesheets/components/_icon.scss b/app/assets/stylesheets/components/_icon.scss index 9d782acd81b..1d4234b1fcd 100644 --- a/app/assets/stylesheets/components/_icon.scss +++ b/app/assets/stylesheets/components/_icon.scss @@ -13,30 +13,6 @@ } } -.ico { - &::before { - background-repeat: no-repeat; - background-size: 1rem; - content: ''; - display: inline-block; - height: 1rem; - margin: -0.125rem 0.5rem -0.125rem 0; - width: 1rem; - } - - &.ico-download::before { - background-image: url('ico-download.svg'); - } - - &.ico-refresh::before { - background-image: url('ico-refresh.svg'); - } - - &.ico-print::before { - background-image: url('ico-print.svg'); - } -} - .ico-absolute { background-repeat: no-repeat; background-size: $h4; diff --git a/app/assets/stylesheets/print.scss b/app/assets/stylesheets/print.scss index 0749d805c36..8292aa80dea 100644 --- a/app/assets/stylesheets/print.scss +++ b/app/assets/stylesheets/print.scss @@ -3,8 +3,7 @@ footer, .usa-button, .usa-radio__input--bordered, - .usa-checkbox__input--bordered, - .ico { + .usa-checkbox__input--bordered { display: none; } } diff --git a/app/views/idv/otp_verification/show.html.erb b/app/views/idv/otp_verification/show.html.erb index b5baca05f4e..bf1cdf8291a 100644 --- a/app/views/idv/otp_verification/show.html.erb +++ b/app/views/idv/otp_verification/show.html.erb @@ -38,7 +38,8 @@ <%= button_to idv_resend_otp_path, method: :post, - class: 'usa-button usa-button--outline ico ico-refresh margin-bottom-4' do %> + class: 'usa-button usa-button--outline margin-bottom-4' do %> + <%= render IconComponent.new(icon: :loop) %> <%= t('links.two_factor_authentication.get_another_code') %> <% end %> diff --git a/app/views/two_factor_authentication/otp_verification/show.html.erb b/app/views/two_factor_authentication/otp_verification/show.html.erb index 68245264b85..05bb6004d0d 100644 --- a/app/views/two_factor_authentication/otp_verification/show.html.erb +++ b/app/views/two_factor_authentication/otp_verification/show.html.erb @@ -37,15 +37,17 @@ <%= hidden_field_tag 'otp_make_default_number', @presenter.otp_make_default_number %> <%= link_to( - t('links.two_factor_authentication.get_another_code'), otp_send_path( otp_delivery_selection_form: { otp_delivery_preference: @presenter.otp_delivery_preference, resend: true, }, ), - class: 'usa-button usa-button--outline ico ico-refresh margin-bottom-neg-1', - ) %> + class: 'usa-button usa-button--outline margin-bottom-neg-1', + ) do %> + <%= render IconComponent.new(icon: :loop) %> + <%= t('links.two_factor_authentication.get_another_code') %> + <% end %> <% end %> <% if @presenter.unconfirmed_phone? %> diff --git a/app/views/users/backup_code_setup/create.html.erb b/app/views/users/backup_code_setup/create.html.erb index d04db92e08d..7e04f69d061 100644 --- a/app/views/users/backup_code_setup/create.html.erb +++ b/app/views/users/backup_code_setup/create.html.erb @@ -32,12 +32,19 @@
<% if desktop_device? %> - <%= link_to t('forms.backup_code.download'), backup_code_download_path, - class: 'usa-button usa-button--outline ico ico-download' %> + <%= link_to(backup_code_download_path, class: 'usa-button usa-button--outline') do %> + <%= render IconComponent.new(icon: :file_download) %> + <%= t('forms.backup_code.download') %> + <% end %> + <% end %> + <%= button_tag( + type: :button, + data: { print: '' }, + class: 'usa-button usa-button--outline margin-top-2 mobile-lg:margin-top-0 mobile-lg:margin-left-2', + ) do %> + <%= render IconComponent.new(icon: :print) %> + <%= t('forms.backup_code.print') %> <% end %> - <%= link_to t('forms.backup_code.print'), '#', - data: { print: true }, - class: 'usa-button usa-button--outline margin-top-2 mobile-lg:margin-top-0 mobile-lg:margin-left-2 ico ico-print' -%> <%= render ClipboardButtonComponent.new( clipboard_text: @codes.join(' '), outline: true, From 83dc7249e6b40c6a467f4b62bc18cc84b632fd74 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 15:00:06 -0500 Subject: [PATCH 03/19] Use vertical-align for icon floating --- app/assets/stylesheets/components/_icon.scss | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/components/_icon.scss b/app/assets/stylesheets/components/_icon.scss index 1d4234b1fcd..950369f8307 100644 --- a/app/assets/stylesheets/components/_icon.scss +++ b/app/assets/stylesheets/components/_icon.scss @@ -1,9 +1,12 @@ // Upstream: https://github.com/uswds/uswds/pull/4493 .usa-icon { .usa-button > &:first-child { - // Note that unlike the upstream pull request, there should not be any margins offsetting to - // account for line height, since Login.gov Design System normalizes button line height to 1. - float: left; + // Note: This diverges from the upstream pull request in a couple ways: + // 1. There should not be any margins offsetting to account for line height, since Login.gov + // Design System normalizes button line height to 1. + // 2. Float is replaced by `vertical-align`, since otherwise it will have the effect of having + // the icon appear to the far edge of the button, rather than next to the text. + vertical-align: bottom; margin-right: 0.25rem; } From 2c8a67635d423b8ca19784abe88c829eceb99fef Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 15:14:09 -0500 Subject: [PATCH 04/19] Try to avoid excess whitespace --- app/views/idv/otp_verification/show.html.erb | 4 ++-- app/views/shared/_personal_key.html.erb | 8 ++++---- .../otp_verification/show.html.erb | 4 ++-- app/views/users/backup_code_setup/create.html.erb | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/views/idv/otp_verification/show.html.erb b/app/views/idv/otp_verification/show.html.erb index bf1cdf8291a..7e24d9095ab 100644 --- a/app/views/idv/otp_verification/show.html.erb +++ b/app/views/idv/otp_verification/show.html.erb @@ -39,8 +39,8 @@ <%= button_to idv_resend_otp_path, method: :post, class: 'usa-button usa-button--outline margin-bottom-4' do %> - <%= render IconComponent.new(icon: :loop) %> - <%= t('links.two_factor_authentication.get_another_code') %> + <%= render IconComponent.new(icon: :loop) -%> + <% concat(t('links.two_factor_authentication.get_another_code')) %> <% end %>

diff --git a/app/views/shared/_personal_key.html.erb b/app/views/shared/_personal_key.html.erb index 329cb9383cb..660eca0fa4b 100644 --- a/app/views/shared/_personal_key.html.erb +++ b/app/views/shared/_personal_key.html.erb @@ -12,16 +12,16 @@ idv_download_personal_key_path, class: 'usa-button usa-button--outline margin-right-2 margin-bottom-2 tablet:margin-bottom-0', ) do %> - <%= render IconComponent.new(icon: :file_download) %> - <%= t('forms.backup_code.download') %> + <%= render IconComponent.new(icon: :file_download) -%> + <% concat(t('forms.backup_code.download')) %> <% end %> <%= button_tag( type: :button, data: { print: true }, class: 'usa-button usa-button--outline margin-right-2 margin-bottom-2 tablet:margin-bottom-0', ) do %> - <%= render IconComponent.new(icon: :print) %> - <%= t('users.personal_key.print') %> + <%= render IconComponent.new(icon: :print) -%> + <% concat(t('users.personal_key.print')) %> <% end %> <%= render ClipboardButtonComponent.new( clipboard_text: code, diff --git a/app/views/two_factor_authentication/otp_verification/show.html.erb b/app/views/two_factor_authentication/otp_verification/show.html.erb index 05bb6004d0d..6bdd3831813 100644 --- a/app/views/two_factor_authentication/otp_verification/show.html.erb +++ b/app/views/two_factor_authentication/otp_verification/show.html.erb @@ -45,8 +45,8 @@ ), class: 'usa-button usa-button--outline margin-bottom-neg-1', ) do %> - <%= render IconComponent.new(icon: :loop) %> - <%= t('links.two_factor_authentication.get_another_code') %> + <%= render IconComponent.new(icon: :loop) -%> + <% concat(t('links.two_factor_authentication.get_another_code')) %> <% end %> <% end %> diff --git a/app/views/users/backup_code_setup/create.html.erb b/app/views/users/backup_code_setup/create.html.erb index 7e04f69d061..459f93ad337 100644 --- a/app/views/users/backup_code_setup/create.html.erb +++ b/app/views/users/backup_code_setup/create.html.erb @@ -33,8 +33,8 @@

<% if desktop_device? %> <%= link_to(backup_code_download_path, class: 'usa-button usa-button--outline') do %> - <%= render IconComponent.new(icon: :file_download) %> - <%= t('forms.backup_code.download') %> + <%= render IconComponent.new(icon: :file_download) -%> + <% concat(t('forms.backup_code.download')) %> <% end %> <% end %> <%= button_tag( @@ -42,8 +42,8 @@ data: { print: '' }, class: 'usa-button usa-button--outline margin-top-2 mobile-lg:margin-top-0 mobile-lg:margin-left-2', ) do %> - <%= render IconComponent.new(icon: :print) %> - <%= t('forms.backup_code.print') %> + <%= render IconComponent.new(icon: :print) -%> + <% concat(t('forms.backup_code.print')) %> <% end %> <%= render ClipboardButtonComponent.new( clipboard_text: @codes.join(' '), From f3e0bf11b1c2e1d49442a90ef7ad9f44f945ec18 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 15:22:58 -0500 Subject: [PATCH 05/19] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43e6587ce2d..a2323ddb7cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ Unreleased ----------- ### Improvements/Changes -- Layout: Improve layout margins and typographical consistency across several content pages. (#5880, #5884, #5887, #5888) +- Layout: Improve layout margins and typographical consistency across several content pages. (#5880, #5884, #5887, #5888, #5904) - Typography: Updated monospace font to Roboto Mono for consistency across login.gov sites. (#5891) ### Accessibility From 491f726712f781cfe9084611f83f10a649bdff34 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 16:34:57 -0500 Subject: [PATCH 06/19] Add custom button factory support --- app/components/button_component.html.erb | 2 +- app/components/button_component.rb | 6 ++++-- spec/components/button_component_spec.rb | 8 ++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/components/button_component.html.erb b/app/components/button_component.html.erb index 762ac4b710b..3e1ec65e264 100644 --- a/app/components/button_component.html.erb +++ b/app/components/button_component.html.erb @@ -1 +1 @@ -<%= button_tag(content, **tag_options, type: tag_type, class: css_class) %> +<%= public_send(factory, content, *factory_args, **tag_options, type: tag_type, class: css_class) %> diff --git a/app/components/button_component.rb b/app/components/button_component.rb index ad6ea90ac22..2a38a77940e 100644 --- a/app/components/button_component.rb +++ b/app/components/button_component.rb @@ -1,9 +1,11 @@ class ButtonComponent < BaseComponent - attr_reader :type, :outline, :tag_options + attr_reader :type, :factory_args, :factory, :outline, :tag_options DEFAULT_BUTTON_TYPE = :button - def initialize(outline: false, **tag_options) + def initialize(*factory_args, factory: :button_tag, outline: false, **tag_options) + @factory_args = factory_args + @factory = factory @outline = outline @tag_options = tag_options end diff --git a/spec/components/button_component_spec.rb b/spec/components/button_component_spec.rb index acc299d3f3f..3ac2cf25a1f 100644 --- a/spec/components/button_component_spec.rb +++ b/spec/components/button_component_spec.rb @@ -39,4 +39,12 @@ expect(rendered).to have_css('button[type=submit]') end end + + context 'with custom button tag factory' do + it 'sends to factory method' do + rendered = render_inline ButtonComponent.new('/', factory: :button_to) { content } + + expect(rendered).to have_css('form[action="/"]') + end + end end From 0f0fb9ac9e79a5d44f886abde9e4a87b735df4a2 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 16:46:06 -0500 Subject: [PATCH 07/19] Add icon support to ButtonComponent --- app/components/button_component.html.erb | 9 ++++++++- app/components/button_component.rb | 9 +++++++-- spec/components/button_component_spec.rb | 9 +++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/app/components/button_component.html.erb b/app/components/button_component.html.erb index 3e1ec65e264..89eb61c8955 100644 --- a/app/components/button_component.html.erb +++ b/app/components/button_component.html.erb @@ -1 +1,8 @@ -<%= public_send(factory, content, *factory_args, **tag_options, type: tag_type, class: css_class) %> +<%= public_send( + factory, + safe_join([icon_content, content].compact), + *factory_args, + **tag_options, + type: tag_type, + class: css_class, + ) %> diff --git a/app/components/button_component.rb b/app/components/button_component.rb index 2a38a77940e..f3d5c4aeadb 100644 --- a/app/components/button_component.rb +++ b/app/components/button_component.rb @@ -1,11 +1,12 @@ class ButtonComponent < BaseComponent - attr_reader :type, :factory_args, :factory, :outline, :tag_options + attr_reader :type, :factory_args, :factory, :icon, :outline, :tag_options DEFAULT_BUTTON_TYPE = :button - def initialize(*factory_args, factory: :button_tag, outline: false, **tag_options) + def initialize(*factory_args, factory: :button_tag, icon: nil, outline: false, **tag_options) @factory_args = factory_args @factory = factory + @icon = icon @outline = outline @tag_options = tag_options end @@ -19,4 +20,8 @@ def css_class def tag_type tag_options.fetch(:type, DEFAULT_BUTTON_TYPE) end + + def icon_content + render IconComponent.new(icon: icon) if icon + end end diff --git a/spec/components/button_component_spec.rb b/spec/components/button_component_spec.rb index 3ac2cf25a1f..6c62c13c51c 100644 --- a/spec/components/button_component_spec.rb +++ b/spec/components/button_component_spec.rb @@ -40,6 +40,15 @@ end end + context 'with icon' do + it 'renders an icon' do + rendered = render_inline ButtonComponent.new(icon: :print).with_content(content) + + expect(rendered).to have_css('use[href$="#print"]') + expect(rendered.first_element_child.xpath('./text()').text).to eq(content) + end + end + context 'with custom button tag factory' do it 'sends to factory method' do rendered = render_inline ButtonComponent.new('/', factory: :button_to) { content } From 06a87f24608fda1b3f6c78527d4829ff364bb84b Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 16:47:34 -0500 Subject: [PATCH 08/19] Refactor ClipboardButtonComponent to use ButtonComponent icon support --- app/components/clipboard_button_component.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/clipboard_button_component.rb b/app/components/clipboard_button_component.rb index cbdcb1cfd4f..cf87595b246 100644 --- a/app/components/clipboard_button_component.rb +++ b/app/components/clipboard_button_component.rb @@ -2,7 +2,7 @@ class ClipboardButtonComponent < ButtonComponent attr_reader :clipboard_text, :tag_options def initialize(clipboard_text:, **tag_options) - super(**tag_options) + super(**tag_options, icon: :content_copy) @clipboard_text = clipboard_text @tag_options = tag_options @@ -13,6 +13,6 @@ def call end def content - safe_join([render(IconComponent.new(icon: :content_copy)), t('links.copy')]) + t('links.copy') end end From 86fe68420caf92c22b9d2a34cdd34997aae2834b Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 16:53:33 -0500 Subject: [PATCH 09/19] Strip whitespace from button content --- app/components/button_component.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/button_component.html.erb b/app/components/button_component.html.erb index 89eb61c8955..db55872aaac 100644 --- a/app/components/button_component.html.erb +++ b/app/components/button_component.html.erb @@ -1,6 +1,6 @@ <%= public_send( factory, - safe_join([icon_content, content].compact), + safe_join([icon_content, content.strip].compact), *factory_args, **tag_options, type: tag_type, From 412347e5766619ddcc5978052dc1ef1e69803245 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 2 Feb 2022 16:56:39 -0500 Subject: [PATCH 10/19] Refactor button+icon to use new ButtonComponent icon support --- app/views/idv/otp_verification/show.html.erb | 14 +++++++----- app/views/shared/_personal_key.html.erb | 22 +++++++++---------- .../otp_verification/show.html.erb | 12 +++++----- .../users/backup_code_setup/create.html.erb | 22 +++++++++---------- 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/app/views/idv/otp_verification/show.html.erb b/app/views/idv/otp_verification/show.html.erb index 7e24d9095ab..73a6f5a5305 100644 --- a/app/views/idv/otp_verification/show.html.erb +++ b/app/views/idv/otp_verification/show.html.erb @@ -36,12 +36,14 @@
<% end %> -<%= button_to idv_resend_otp_path, - method: :post, - class: 'usa-button usa-button--outline margin-bottom-4' do %> - <%= render IconComponent.new(icon: :loop) -%> - <% concat(t('links.two_factor_authentication.get_another_code')) %> -<% end %> +<%= render ButtonComponent.new( + idv_resend_otp_path, + method: :post, + factory: :button_to, + outline: true, + icon: :loop, + class: 'margin-bottom-4', + ).with_content(t('links.two_factor_authentication.get_another_code')) %>

<%= t('instructions.mfa.wrong_number') %>
diff --git a/app/views/shared/_personal_key.html.erb b/app/views/shared/_personal_key.html.erb index 660eca0fa4b..e6d6ad23fc0 100644 --- a/app/views/shared/_personal_key.html.erb +++ b/app/views/shared/_personal_key.html.erb @@ -8,20 +8,18 @@

<%= render 'partials/personal_key/key', code: code %>
-<%= link_to( +<%= render ButtonComponent.new( idv_download_personal_key_path, - class: 'usa-button usa-button--outline margin-right-2 margin-bottom-2 tablet:margin-bottom-0', - ) do %> - <%= render IconComponent.new(icon: :file_download) -%> - <% concat(t('forms.backup_code.download')) %> -<% end %> -<%= button_tag( - type: :button, + factory: :link_to, + icon: :file_download, + outline: true, + class: 'margin-right-2 margin-bottom-2 tablet:margin-bottom-0', + ).with_content(t('forms.backup_code.download')) %> +<%= render ButtonComponent.new( + icon: :print, data: { print: true }, - class: 'usa-button usa-button--outline margin-right-2 margin-bottom-2 tablet:margin-bottom-0', - ) do %> - <%= render IconComponent.new(icon: :print) -%> - <% concat(t('users.personal_key.print')) %> + class: 'margin-right-2 margin-bottom-2 tablet:margin-bottom-0', + ).with_content(t('users.personal_key.print')) %> <% end %> <%= render ClipboardButtonComponent.new( clipboard_text: code, diff --git a/app/views/two_factor_authentication/otp_verification/show.html.erb b/app/views/two_factor_authentication/otp_verification/show.html.erb index 6bdd3831813..826c49a45f9 100644 --- a/app/views/two_factor_authentication/otp_verification/show.html.erb +++ b/app/views/two_factor_authentication/otp_verification/show.html.erb @@ -36,18 +36,18 @@ ) %> <%= hidden_field_tag 'otp_make_default_number', @presenter.otp_make_default_number %> - <%= link_to( + <%= render ButtonComponent.new( otp_send_path( otp_delivery_selection_form: { otp_delivery_preference: @presenter.otp_delivery_preference, resend: true, }, ), - class: 'usa-button usa-button--outline margin-bottom-neg-1', - ) do %> - <%= render IconComponent.new(icon: :loop) -%> - <% concat(t('links.two_factor_authentication.get_another_code')) %> - <% end %> + factory: :link_to, + outline: true, + icon: :loop, + class: 'margin-bottom-neg-1', + ).with_content(t('links.two_factor_authentication.get_another_code')) %> <% end %> <% if @presenter.unconfirmed_phone? %> diff --git a/app/views/users/backup_code_setup/create.html.erb b/app/views/users/backup_code_setup/create.html.erb index 459f93ad337..7e052a2618b 100644 --- a/app/views/users/backup_code_setup/create.html.erb +++ b/app/views/users/backup_code_setup/create.html.erb @@ -32,19 +32,19 @@
<% if desktop_device? %> - <%= link_to(backup_code_download_path, class: 'usa-button usa-button--outline') do %> - <%= render IconComponent.new(icon: :file_download) -%> - <% concat(t('forms.backup_code.download')) %> - <% end %> + <%= render ButtonComponent.new( + backup_code_download_path, + factory: :link_to, + outline: true, + icon: :file_download, + ).with_content(t('forms.backup_code.download')) %> <% end %> - <%= button_tag( - type: :button, + <%= render ButtonComponent.new( + icon: :print, + outline: true, data: { print: '' }, - class: 'usa-button usa-button--outline margin-top-2 mobile-lg:margin-top-0 mobile-lg:margin-left-2', - ) do %> - <%= render IconComponent.new(icon: :print) -%> - <% concat(t('forms.backup_code.print')) %> - <% end %> + class: 'margin-top-2 mobile-lg:margin-top-0 mobile-lg:margin-left-2', + ).with_content(t('forms.backup_code.print')) %> <%= render ClipboardButtonComponent.new( clipboard_text: @codes.join(' '), outline: true, From 47e654a29114034e386bd41f98bf409806b49238 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 3 Feb 2022 08:30:02 -0500 Subject: [PATCH 11/19] Refactor button content rendering --- app/components/button_component.html.erb | 2 +- app/components/button_component.rb | 8 ++++++-- spec/components/button_component_spec.rb | 4 +++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/components/button_component.html.erb b/app/components/button_component.html.erb index db55872aaac..310de4c13ba 100644 --- a/app/components/button_component.html.erb +++ b/app/components/button_component.html.erb @@ -1,6 +1,6 @@ <%= public_send( factory, - safe_join([icon_content, content.strip].compact), + content, *factory_args, **tag_options, type: tag_type, diff --git a/app/components/button_component.rb b/app/components/button_component.rb index f3d5c4aeadb..f226d536355 100644 --- a/app/components/button_component.rb +++ b/app/components/button_component.rb @@ -21,7 +21,11 @@ def tag_type tag_options.fetch(:type, DEFAULT_BUTTON_TYPE) end - def icon_content - render IconComponent.new(icon: icon) if icon + def content + if icon + safe_join([render(IconComponent.new(icon: icon)), super&.strip]) + else + super + end end end diff --git a/spec/components/button_component_spec.rb b/spec/components/button_component_spec.rb index 6c62c13c51c..54feef8feae 100644 --- a/spec/components/button_component_spec.rb +++ b/spec/components/button_component_spec.rb @@ -10,7 +10,9 @@ }.compact end - subject(:rendered) { render_inline ButtonComponent.new(outline: outline, **options) { content } } + subject(:rendered) do + render_inline ButtonComponent.new(outline: outline, **options).with_content(content) + end it 'renders button content' do expect(rendered).to have_content(content) From 5a9423f4a3627dd5b523410ac928931a993ed8be Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 3 Feb 2022 09:02:01 -0500 Subject: [PATCH 12/19] Revert "Refactor button content rendering" This reverts commit 47e654a29114034e386bd41f98bf409806b49238. --- app/components/button_component.html.erb | 2 +- app/components/button_component.rb | 8 ++------ spec/components/button_component_spec.rb | 4 +--- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/app/components/button_component.html.erb b/app/components/button_component.html.erb index 310de4c13ba..db55872aaac 100644 --- a/app/components/button_component.html.erb +++ b/app/components/button_component.html.erb @@ -1,6 +1,6 @@ <%= public_send( factory, - content, + safe_join([icon_content, content.strip].compact), *factory_args, **tag_options, type: tag_type, diff --git a/app/components/button_component.rb b/app/components/button_component.rb index f226d536355..f3d5c4aeadb 100644 --- a/app/components/button_component.rb +++ b/app/components/button_component.rb @@ -21,11 +21,7 @@ def tag_type tag_options.fetch(:type, DEFAULT_BUTTON_TYPE) end - def content - if icon - safe_join([render(IconComponent.new(icon: icon)), super&.strip]) - else - super - end + def icon_content + render IconComponent.new(icon: icon) if icon end end diff --git a/spec/components/button_component_spec.rb b/spec/components/button_component_spec.rb index 54feef8feae..6c62c13c51c 100644 --- a/spec/components/button_component_spec.rb +++ b/spec/components/button_component_spec.rb @@ -10,9 +10,7 @@ }.compact end - subject(:rendered) do - render_inline ButtonComponent.new(outline: outline, **options).with_content(content) - end + subject(:rendered) { render_inline ButtonComponent.new(outline: outline, **options) { content } } it 'renders button content' do expect(rendered).to have_content(content) From c241f5c87f355157f5bed946e982d9ee323fc49f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 3 Feb 2022 09:05:14 -0500 Subject: [PATCH 13/19] Fix nil content stripping --- app/components/button_component.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/button_component.html.erb b/app/components/button_component.html.erb index db55872aaac..cbdd6e18ead 100644 --- a/app/components/button_component.html.erb +++ b/app/components/button_component.html.erb @@ -1,6 +1,6 @@ <%= public_send( factory, - safe_join([icon_content, content.strip].compact), + safe_join([icon_content, content&.strip].compact), *factory_args, **tag_options, type: tag_type, From 3ae52e858c39d6fc92c84209c796a822f54873f6 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 3 Feb 2022 09:09:03 -0500 Subject: [PATCH 14/19] Remove stray end tag --- app/views/shared/_personal_key.html.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/shared/_personal_key.html.erb b/app/views/shared/_personal_key.html.erb index e6d6ad23fc0..74d5926fa1c 100644 --- a/app/views/shared/_personal_key.html.erb +++ b/app/views/shared/_personal_key.html.erb @@ -20,7 +20,6 @@ data: { print: true }, class: 'margin-right-2 margin-bottom-2 tablet:margin-bottom-0', ).with_content(t('users.personal_key.print')) %> -<% end %> <%= render ClipboardButtonComponent.new( clipboard_text: code, outline: true, From 2c1e7aaad624a8dc84f015a43173ba37d2b73c51 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 3 Feb 2022 09:09:24 -0500 Subject: [PATCH 15/19] Refactor button customization to action callable --- app/components/button_component.html.erb | 4 +--- app/components/button_component.rb | 12 ++++++---- app/views/idv/otp_verification/show.html.erb | 6 ++--- app/views/shared/_personal_key.html.erb | 5 ++-- .../otp_verification/show.html.erb | 18 +++++++++------ .../users/backup_code_setup/create.html.erb | 5 ++-- spec/components/button_component_spec.rb | 23 +++++++++++++++---- 7 files changed, 47 insertions(+), 26 deletions(-) diff --git a/app/components/button_component.html.erb b/app/components/button_component.html.erb index cbdd6e18ead..2c95172424a 100644 --- a/app/components/button_component.html.erb +++ b/app/components/button_component.html.erb @@ -1,7 +1,5 @@ -<%= public_send( - factory, +<%= action.call( safe_join([icon_content, content&.strip].compact), - *factory_args, **tag_options, type: tag_type, class: css_class, diff --git a/app/components/button_component.rb b/app/components/button_component.rb index f3d5c4aeadb..46d08ce8833 100644 --- a/app/components/button_component.rb +++ b/app/components/button_component.rb @@ -1,11 +1,15 @@ class ButtonComponent < BaseComponent - attr_reader :type, :factory_args, :factory, :icon, :outline, :tag_options + attr_reader :action, :icon, :outline, :tag_options DEFAULT_BUTTON_TYPE = :button - def initialize(*factory_args, factory: :button_tag, icon: nil, outline: false, **tag_options) - @factory_args = factory_args - @factory = factory + def initialize( + action: ->(content, **tag_options) { button_tag(content, **tag_options) }, + icon: nil, + outline: false, + **tag_options + ) + @action = action @icon = icon @outline = outline @tag_options = tag_options diff --git a/app/views/idv/otp_verification/show.html.erb b/app/views/idv/otp_verification/show.html.erb index 73a6f5a5305..f88159564bb 100644 --- a/app/views/idv/otp_verification/show.html.erb +++ b/app/views/idv/otp_verification/show.html.erb @@ -37,9 +37,9 @@ <% end %> <%= render ButtonComponent.new( - idv_resend_otp_path, - method: :post, - factory: :button_to, + action: ->(content, **tag_options) do + button_to(idv_resend_otp_path, method: :post, **tag_options) { content } + end, outline: true, icon: :loop, class: 'margin-bottom-4', diff --git a/app/views/shared/_personal_key.html.erb b/app/views/shared/_personal_key.html.erb index 74d5926fa1c..5c7e5bbeb91 100644 --- a/app/views/shared/_personal_key.html.erb +++ b/app/views/shared/_personal_key.html.erb @@ -9,8 +9,9 @@ <%= render 'partials/personal_key/key', code: code %>
<%= render ButtonComponent.new( - idv_download_personal_key_path, - factory: :link_to, + action: ->(content, **tag_options) do + link_to(idv_download_personal_key_path, **tag_options) { content } + end, icon: :file_download, outline: true, class: 'margin-right-2 margin-bottom-2 tablet:margin-bottom-0', diff --git a/app/views/two_factor_authentication/otp_verification/show.html.erb b/app/views/two_factor_authentication/otp_verification/show.html.erb index 826c49a45f9..1065c0fd314 100644 --- a/app/views/two_factor_authentication/otp_verification/show.html.erb +++ b/app/views/two_factor_authentication/otp_verification/show.html.erb @@ -37,13 +37,17 @@ <%= hidden_field_tag 'otp_make_default_number', @presenter.otp_make_default_number %> <%= render ButtonComponent.new( - otp_send_path( - otp_delivery_selection_form: { - otp_delivery_preference: @presenter.otp_delivery_preference, - resend: true, - }, - ), - factory: :link_to, + action: ->(content, **tag_options) do + link_to( + otp_send_path( + otp_delivery_selection_form: { + otp_delivery_preference: @presenter.otp_delivery_preference, + resend: true, + }, + ), + **tag_options, + ) { content } + end, outline: true, icon: :loop, class: 'margin-bottom-neg-1', diff --git a/app/views/users/backup_code_setup/create.html.erb b/app/views/users/backup_code_setup/create.html.erb index 7e052a2618b..51284eb0909 100644 --- a/app/views/users/backup_code_setup/create.html.erb +++ b/app/views/users/backup_code_setup/create.html.erb @@ -33,8 +33,9 @@
<% if desktop_device? %> <%= render ButtonComponent.new( - backup_code_download_path, - factory: :link_to, + action: ->(content, **tag_options) do + link_to(backup_code_download_path, **tag_options) { content } + end, outline: true, icon: :file_download, ).with_content(t('forms.backup_code.download')) %> diff --git a/spec/components/button_component_spec.rb b/spec/components/button_component_spec.rb index 6c62c13c51c..5cfab596b30 100644 --- a/spec/components/button_component_spec.rb +++ b/spec/components/button_component_spec.rb @@ -1,6 +1,9 @@ require 'rails_helper' RSpec.describe ButtonComponent, type: :component do + include ActionView::Context + include ActionView::Helpers::TagHelper + let(:type) { nil } let(:outline) { false } let(:content) { 'Button' } @@ -10,7 +13,9 @@ }.compact end - subject(:rendered) { render_inline ButtonComponent.new(outline: outline, **options) { content } } + subject(:rendered) do + render_inline ButtonComponent.new(outline: outline, **options).with_content(content) + end it 'renders button content' do expect(rendered).to have_content(content) @@ -49,11 +54,19 @@ end end - context 'with custom button tag factory' do - it 'sends to factory method' do - rendered = render_inline ButtonComponent.new('/', factory: :button_to) { content } + context 'with custom button action' do + it 'calls the action with content and tag_options' do + rendered = render_inline ButtonComponent.new( + action: ->(content, **tag_options) do + content_tag(:'lg-custom-button', **tag_options, data: { extra: '' }) { content } + end, + class: 'custom-class', + ).with_content(content) - expect(rendered).to have_css('form[action="/"]') + expect(rendered).to have_css( + 'lg-custom-button[type="button"][data-extra].custom-class', + text: content, + ) end end end From 45ff1e4eee6921c7580744662a62191e5f41a82b Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 3 Feb 2022 10:42:40 -0500 Subject: [PATCH 16/19] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2323ddb7cf..7a1ff50daf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Unreleased ### Improvements/Changes - Layout: Improve layout margins and typographical consistency across several content pages. (#5880, #5884, #5887, #5888, #5904) - Typography: Updated monospace font to Roboto Mono for consistency across login.gov sites. (#5891) +- Icons: Replaced custom button icons using U.S. Web Design system icons. (#5904) ### Accessibility From bc7b06210c0e7ca94c3bffe4fc1fa9bef4dfa8c0 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 3 Feb 2022 10:43:15 -0500 Subject: [PATCH 17/19] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a1ff50daf9..e5723098bac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ Unreleased ----------- ### Improvements/Changes -- Layout: Improve layout margins and typographical consistency across several content pages. (#5880, #5884, #5887, #5888, #5904) +- Layout: Improve layout margins and typographical consistency across several content pages. (#5880, #5884, #5887, #5888) - Typography: Updated monospace font to Roboto Mono for consistency across login.gov sites. (#5891) - Icons: Replaced custom button icons using U.S. Web Design system icons. (#5904) From 1f73b65dd2b3bff1affb1d92fe4692344ef05b6f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 3 Feb 2022 11:13:21 -0500 Subject: [PATCH 18/19] Defer tag_options ivar assignment to super **Why**: Avoids button kwargs being treated as attributes in button output --- app/components/clipboard_button_component.rb | 1 - spec/components/clipboard_button_component_spec.rb | 8 ++++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/components/clipboard_button_component.rb b/app/components/clipboard_button_component.rb index cf87595b246..4c0fd6bcfd2 100644 --- a/app/components/clipboard_button_component.rb +++ b/app/components/clipboard_button_component.rb @@ -5,7 +5,6 @@ def initialize(clipboard_text:, **tag_options) super(**tag_options, icon: :content_copy) @clipboard_text = clipboard_text - @tag_options = tag_options end def call diff --git a/spec/components/clipboard_button_component_spec.rb b/spec/components/clipboard_button_component_spec.rb index 3c4ef1e7fa1..deecb15fd6f 100644 --- a/spec/components/clipboard_button_component_spec.rb +++ b/spec/components/clipboard_button_component_spec.rb @@ -13,10 +13,14 @@ end context 'with tag options' do - let(:tag_options) { { outline: true } } + let(:tag_options) { { outline: true, data: { foo: 'bar' } } } it 'renders button given the tag options' do - expect(rendered).to have_css('button.usa-button.usa-button--outline') + expect(rendered).to have_css('button.usa-button[data-foo="bar"]') + end + + it 'respects keyword arguments of button component' do + expect(rendered).to have_css('.usa-button--outline:not([outline])') end end end From 69b74330fd4b6b9a38d3b2725a2c389126ae8487 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 3 Feb 2022 11:44:18 -0500 Subject: [PATCH 19/19] Restore outline appearance for personal key print button --- app/views/shared/_personal_key.html.erb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/shared/_personal_key.html.erb b/app/views/shared/_personal_key.html.erb index 5c7e5bbeb91..76c55fc5dec 100644 --- a/app/views/shared/_personal_key.html.erb +++ b/app/views/shared/_personal_key.html.erb @@ -18,7 +18,8 @@ ).with_content(t('forms.backup_code.download')) %> <%= render ButtonComponent.new( icon: :print, - data: { print: true }, + outline: true, + data: { print: '' }, class: 'margin-right-2 margin-bottom-2 tablet:margin-bottom-0', ).with_content(t('users.personal_key.print')) %> <%= render ClipboardButtonComponent.new(