From edbafb7dadfeba5fe9e326fc34d2825a914e8fe4 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 9 Feb 2024 13:58:15 -0500 Subject: [PATCH 01/12] Reimplement icon component as SVG image mask changelog: Internal, Performance, Reduce download size for icon images --- .../stylesheets/components/_language-picker.scss | 8 ++++---- app/components/icon_component.html.erb | 12 +++--------- app/components/icon_component.rb | 2 +- app/components/language_picker_component.html.erb | 5 ++--- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/app/assets/stylesheets/components/_language-picker.scss b/app/assets/stylesheets/components/_language-picker.scss index 5910f030e92..481c220f5d2 100644 --- a/app/assets/stylesheets/components/_language-picker.scss +++ b/app/assets/stylesheets/components/_language-picker.scss @@ -43,10 +43,6 @@ } } - span { - margin: 0 units(1); - } - &::after { content: ''; display: block; @@ -84,6 +80,10 @@ } } +.language-picker__label-text { + margin: 0 units(1); +} + .language-picker__list { @include list-reset; diff --git a/app/components/icon_component.html.erb b/app/components/icon_component.html.erb index 59d6b4b5695..5e2c1da4412 100644 --- a/app/components/icon_component.html.erb +++ b/app/components/icon_component.html.erb @@ -1,10 +1,4 @@ -<%= content_tag( - :svg, - aria: { hidden: true }, - focusable: 'false', - role: 'img', - **tag_options, - class: css_class, - ) do %> - +<%= content_tag(:span, '', **tag_options, id: "icon-#{unique_id}", class: css_class) %> +<%= content_tag(:style, nonce: content_security_policy_nonce) do %> + <%= "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); mask-size: 100%; -webkit-mask-size: 100%; background-color: currentColor; }" %> <% end %> diff --git a/app/components/icon_component.rb b/app/components/icon_component.rb index 40250ff994a..0f3931efbc5 100644 --- a/app/components/icon_component.rb +++ b/app/components/icon_component.rb @@ -261,7 +261,7 @@ def css_class end def icon_path - asset_path([asset_path('sprite.svg'), '#', icon].join, host: asset_host) + asset_path("usa-icons/#{icon}.svg", host: asset_host) end private diff --git a/app/components/language_picker_component.html.erb b/app/components/language_picker_component.html.erb index 7caa09ead3a..5217b896379 100644 --- a/app/components/language_picker_component.html.erb +++ b/app/components/language_picker_component.html.erb @@ -7,9 +7,8 @@ expanded: false, }, ) do %> - <%= image_tag(asset_url('globe-blue.svg'), width: 12, height: 12, alt: '', class: 'tablet:display-none') %> - <%= image_tag(asset_url('globe-white.svg'), width: 12, height: 12, alt: '', class: 'display-none tablet:display-inline') %> - + <%= render IconComponent.new(icon: :language) %> + <%= t('i18n.language') %> <% end %> From 826c1cecfca43264d679c143342785da3e3994d2 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 9 Feb 2024 14:00:25 -0500 Subject: [PATCH 02/12] Remove unused images --- app/assets/images/globe-blue.svg | 1 - app/assets/images/globe-white.svg | 1 - 2 files changed, 2 deletions(-) delete mode 100644 app/assets/images/globe-blue.svg delete mode 100644 app/assets/images/globe-white.svg diff --git a/app/assets/images/globe-blue.svg b/app/assets/images/globe-blue.svg deleted file mode 100644 index a1782e86118..00000000000 --- a/app/assets/images/globe-blue.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/app/assets/images/globe-white.svg b/app/assets/images/globe-white.svg deleted file mode 100644 index 88a59e3d128..00000000000 --- a/app/assets/images/globe-white.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file From c0d886434202d1bc4615fc9b12b18542ee62ddc6 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 9 Feb 2024 17:02:27 -0500 Subject: [PATCH 03/12] Permit nonce for style-src CSP directive --- config/initializers/content_security_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/content_security_policy.rb b/config/initializers/content_security_policy.rb index 5f734f347bc..29d2b0e5002 100644 --- a/config/initializers/content_security_policy.rb +++ b/config/initializers/content_security_policy.rb @@ -47,5 +47,5 @@ # rubocop:enable Metrics/BlockLength Rails.application.configure do config.content_security_policy_nonce_generator = ->(request) { request.session.id.to_s } - config.content_security_policy_nonce_directives = ['script-src'] + config.content_security_policy_nonce_directives = ['script-src', 'style-src'] end From 80fa89a741ef6c04c8e1d9e31c600812ce57f48a Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 9 Feb 2024 17:03:54 -0500 Subject: [PATCH 04/12] Update CSP specs --- spec/requests/csp_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/requests/csp_spec.rb b/spec/requests/csp_spec.rb index 73440a443ac..cbe92e3b7b2 100644 --- a/spec/requests/csp_spec.rb +++ b/spec/requests/csp_spec.rb @@ -31,7 +31,7 @@ expect(content_security_policy['script-src']).to match( /'self' 'unsafe-eval' 'nonce-[\w\d=\/+]+'/, ) - expect(content_security_policy['style-src']).to eq("'self'") + expect(content_security_policy['style-src']).to match(/'self' 'nonce-[\w\d=\/+]+'/) end it 'uses logout SP to override CSP form action that will allow a redirect to the CSP' do @@ -75,7 +75,7 @@ expect(content_security_policy['script-src']).to match( /'self' 'unsafe-eval' 'nonce-[\w\d=\/+]+'/, ) - expect(content_security_policy['style-src']).to eq("'self'") + expect(content_security_policy['style-src']).to match(/'self' 'nonce-[\w\d=\/+]+'/) end it 'uses logout SP to override CSP form action that will allow a redirect to the CSP' do @@ -111,7 +111,7 @@ expect(content_security_policy['script-src']).to match( /'self' 'unsafe-eval' 'nonce-[\w\d=\/+]+'/, ) - expect(content_security_policy['style-src']).to eq("'self'") + expect(content_security_policy['style-src']).to match(/'self' 'nonce-[\w\d=\/+]+'/) end end From d6fa4a88ec1a2e25a20d5c566096f882d9f98786 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Feb 2024 10:41:02 -0500 Subject: [PATCH 05/12] Move static inline styles to stylesheet --- app/components/icon_component.html.erb | 2 +- app/components/icon_component.rb | 2 +- app/components/icon_component.scss | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/components/icon_component.html.erb b/app/components/icon_component.html.erb index 5e2c1da4412..5d7d781bd96 100644 --- a/app/components/icon_component.html.erb +++ b/app/components/icon_component.html.erb @@ -1,4 +1,4 @@ <%= content_tag(:span, '', **tag_options, id: "icon-#{unique_id}", class: css_class) %> <%= content_tag(:style, nonce: content_security_policy_nonce) do %> - <%= "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); mask-size: 100%; -webkit-mask-size: 100%; background-color: currentColor; }" %> + <%= "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); }" %> <% end %> diff --git a/app/components/icon_component.rb b/app/components/icon_component.rb index 0f3931efbc5..ba4fa10d84e 100644 --- a/app/components/icon_component.rb +++ b/app/components/icon_component.rb @@ -255,7 +255,7 @@ def initialize(icon:, size: nil, **tag_options) end def css_class - classes = ['usa-icon', *tag_options[:class]] + classes = ['icon', 'usa-icon', *tag_options[:class]] classes << "usa-icon--size-#{size}" if size classes end diff --git a/app/components/icon_component.scss b/app/components/icon_component.scss index 6b0bdc27e2a..2591b1d1b9c 100644 --- a/app/components/icon_component.scss +++ b/app/components/icon_component.scss @@ -3,6 +3,11 @@ @forward 'usa-icon'; +.icon { + mask-size: 100%; + background-color: currentColor; +} + $icon-min-padding: 2px; // Upstream: https://github.com/uswds/uswds/pull/4493 From 9d7e68ec29a218556decf7ee3083266ce133ebaf Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Feb 2024 10:43:53 -0500 Subject: [PATCH 06/12] Memoize icon_path Multiple calls per render --- app/components/icon_component.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/icon_component.rb b/app/components/icon_component.rb index ba4fa10d84e..4923b3ea551 100644 --- a/app/components/icon_component.rb +++ b/app/components/icon_component.rb @@ -261,7 +261,7 @@ def css_class end def icon_path - asset_path("usa-icons/#{icon}.svg", host: asset_host) + @icon_path ||= asset_path("usa-icons/#{icon}.svg", host: asset_host) end private From 407968d9ebf2d0b50d99af97c35027ac67cf28a6 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Feb 2024 10:54:02 -0500 Subject: [PATCH 07/12] Update IconComponent specs --- spec/components/icon_component_spec.rb | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/spec/components/icon_component_spec.rb b/spec/components/icon_component_spec.rb index ea92deadcaa..d1dfdb6a9ce 100644 --- a/spec/components/icon_component_spec.rb +++ b/spec/components/icon_component_spec.rb @@ -12,9 +12,15 @@ it 'renders icon svg' do rendered = render_inline IconComponent.new(icon: :print) - expect(rendered).to have_css( - ".usa-icon use[href^='#{vc_test_request.base_url}'][href$='.svg#print']", - ) + icon = rendered.at_css('.icon.usa-icon') + id = icon.attr(:id) + inline_style = rendered.at_css('style').text.strip + + expect(icon).to be_present + expect(inline_style).to match(%r{##{id}\s{.+?}}). + and include('-webkit-mask-image:'). + and include('mask-image:'). + and match(%r{url\([^)]+/print-\w+\.svg\)}) end context 'with invalid icon' do @@ -27,7 +33,7 @@ it 'adds size variant class' do rendered = render_inline IconComponent.new(icon: :print, size: 2) - expect(rendered).to have_css('.usa-icon.usa-icon--size-2') + expect(rendered).to have_css('.icon.usa-icon.usa-icon--size-2') end end @@ -35,7 +41,7 @@ it 'renders with class' do rendered = render_inline IconComponent.new(icon: :print, class: 'my-custom-class') - expect(rendered).to have_css('.usa-icon.my-custom-class') + expect(rendered).to have_css('.icon.usa-icon.my-custom-class') end end @@ -43,7 +49,7 @@ it 'renders with attributes' do rendered = render_inline IconComponent.new(icon: :print, data: { foo: 'bar' }) - expect(rendered).to have_css('.usa-icon[data-foo="bar"]') + expect(rendered).to have_css('.icon.usa-icon[data-foo="bar"]') end end @@ -55,9 +61,9 @@ it 'bypasses configured asset_host and uses domain_name instead' do rendered = render_inline IconComponent.new(icon: :print) - href = rendered.css('use').first['href'] + inline_style = rendered.at_css('style').text.strip - expect(href).to start_with(domain_name) + expect(inline_style).to match(%r{url\(#{Regexp.escape(domain_name)}}) end end end From 39c82679e599b9faebb1f3eed8da501776bf86fd Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Feb 2024 17:01:13 -0500 Subject: [PATCH 08/12] Revise link matcher to check text Previous assertion used XPath, where XPath text matchers don't correctly ignore inline style tag contents --- spec/features/phone/add_phone_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/phone/add_phone_spec.rb b/spec/features/phone/add_phone_spec.rb index 9ee08661408..1b14f111958 100644 --- a/spec/features/phone/add_phone_spec.rb +++ b/spec/features/phone/add_phone_spec.rb @@ -6,7 +6,7 @@ phone = '+1 (225) 278-1234' sign_in_and_2fa_user(user) - expect(page).to have_link(t('account.index.phone_add'), normalize_ws: true, exact: true) + expect(page).to have_link(href: phone_setup_path, text: t('account.index.phone_add')) within('.sidenav') do click_on t('account.navigation.add_phone_number') end From 59aef88abb3fec2606ba52a9496dbb6c90f1c144 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Feb 2024 17:01:28 -0500 Subject: [PATCH 09/12] Update IconListComponent specs --- spec/components/icon_list_component_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/components/icon_list_component_spec.rb b/spec/components/icon_list_component_spec.rb index 599ede1b38c..83acec19eb7 100644 --- a/spec/components/icon_list_component_spec.rb +++ b/spec/components/icon_list_component_spec.rb @@ -35,7 +35,7 @@ it 'renders items with default color' do expect(rendered).to have_css('.usa-icon-list__icon:not([class*="text-"])', count: 2) - expect(rendered).to have_css('.usa-icon use[href$=".svg#cancel"]', count: 2) + expect(rendered).to have_xpath('//style[contains(text(), "/cancel-")]') end context 'with icon or color attributes specified on parent component' do @@ -48,7 +48,7 @@ it 'passes those attributes to slotted items' do expect(rendered).to have_css('.usa-icon-list__icon.text-error', count: 2) - expect(rendered).to have_css('.usa-icon use[href$=".svg#cancel"]', count: 2) + expect(rendered).to have_xpath('//style[contains(text(), "/cancel-")]', count: 2) end end @@ -62,9 +62,9 @@ it 'renders items with their attributes' do expect(rendered).to have_css('.usa-icon-list__icon.text-success', count: 1) - expect(rendered).to have_css('.usa-icon use[href$=".svg#check_circle"]', count: 1) + expect(rendered).to have_xpath('//style[contains(text(), "/check_circle-")]', count: 1) expect(rendered).to have_css('.usa-icon-list__icon.text-error', count: 1) - expect(rendered).to have_css('.usa-icon use[href$=".svg#cancel"]', count: 1) + expect(rendered).to have_xpath('//style[contains(text(), "/cancel-")]', count: 1) end end From 13467a9da493bbbdaf00ea351a56d60882778f8b Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Feb 2024 17:02:01 -0500 Subject: [PATCH 10/12] Render IconComponent inline style as direct string content Since it's already rendered as simple string, pass as argument in content_tag. Also avoids excess whitespace --- app/components/icon_component.html.erb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/components/icon_component.html.erb b/app/components/icon_component.html.erb index 5d7d781bd96..0412a5f97b1 100644 --- a/app/components/icon_component.html.erb +++ b/app/components/icon_component.html.erb @@ -1,4 +1,6 @@ <%= content_tag(:span, '', **tag_options, id: "icon-#{unique_id}", class: css_class) %> -<%= content_tag(:style, nonce: content_security_policy_nonce) do %> - <%= "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); }" %> -<% end %> +<%= content_tag( + :style, + "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); }", + nonce: content_security_policy_nonce, + ) %> From 5bd96881e67e890c94329033a06466b535d54d82 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Feb 2024 17:03:25 -0500 Subject: [PATCH 11/12] Add method parenthesis For clarity, avoid ambiguity with "and" keyword Co-authored-by: Zach Margolis --- spec/components/icon_component_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/components/icon_component_spec.rb b/spec/components/icon_component_spec.rb index d1dfdb6a9ce..dc1fce80ed4 100644 --- a/spec/components/icon_component_spec.rb +++ b/spec/components/icon_component_spec.rb @@ -18,9 +18,9 @@ expect(icon).to be_present expect(inline_style).to match(%r{##{id}\s{.+?}}). - and include('-webkit-mask-image:'). - and include('mask-image:'). - and match(%r{url\([^)]+/print-\w+\.svg\)}) + and(include('-webkit-mask-image:')). + and(include('mask-image:')). + and(match(%r{url\([^)]+/print-\w+\.svg\)})) end context 'with invalid icon' do From d3990d5899f1bb158f6e7ff01ee9b2e066b9d6ec Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Feb 2024 17:11:50 -0500 Subject: [PATCH 12/12] Trim whitespace in icon component output Significant for use in ButtonComponent --- app/components/icon_component.html.erb | 15 ++++++++++----- spec/components/button_component_spec.rb | 8 +++++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/app/components/icon_component.html.erb b/app/components/icon_component.html.erb index 0412a5f97b1..274a84e440e 100644 --- a/app/components/icon_component.html.erb +++ b/app/components/icon_component.html.erb @@ -1,6 +1,11 @@ -<%= content_tag(:span, '', **tag_options, id: "icon-#{unique_id}", class: css_class) %> <%= content_tag( - :style, - "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); }", - nonce: content_security_policy_nonce, - ) %> + :span, + content_tag( + :style, + "#icon-#{unique_id} { mask-image: url(#{icon_path}); -webkit-mask-image: url(#{icon_path}); }", + nonce: content_security_policy_nonce, + ), + **tag_options, + id: "icon-#{unique_id}", + class: css_class, + ) -%> diff --git a/spec/components/button_component_spec.rb b/spec/components/button_component_spec.rb index ce0ef65f49e..f448f220ce9 100644 --- a/spec/components/button_component_spec.rb +++ b/spec/components/button_component_spec.rb @@ -83,7 +83,7 @@ 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).to have_xpath('//style[contains(text(), "/print-")]') expect(rendered.first_element_child.xpath('./text()').text).to eq(content) end @@ -93,7 +93,7 @@ it 'trims text of the content, maintaining html safety' do rendered = render_inline ButtonComponent.new(icon: :print).with_content(content) - expect(rendered.to_html).to include('Button') + expect(rendered.to_html).to include('Button') end end @@ -103,7 +103,9 @@ it 'trims text of the content, maintaining html safety' do rendered = render_inline ButtonComponent.new(icon: :print).with_content(content) - expect(rendered.to_html).to include('<span class="example">Button</span>') + expect(rendered.to_html).to include( + '<span class="example">Button</span>', + ) end end