diff --git a/.rubocop.yml b/.rubocop.yml index 35f3e787d68..19a182cdd5b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,6 +7,7 @@ require: - rubocop-rails - rubocop-performance - ./lib/linters/localized_validation_message_linter.rb + - ./lib/linters/image_size_linter.rb - ./lib/linters/mail_later_linter.rb - ./lib/linters/redirect_back_linter.rb - ./lib/linters/url_options_linter.rb @@ -58,6 +59,16 @@ IdentityIdp/ErrorsAddLinter: Exclude: - 'spec/**/*.rb' +IdentityIdp/ImageSizeLinter: + Enabled: true + Exclude: + - app/components/barcode_component.html.erb + - app/views/partials/multi_factor_authentication/_mfa_selection.html.erb + - app/views/shared/_nav_branded.html.erb + - app/views/sign_up/completions/show.html.erb + - app/views/users/two_factor_authentication_setup/index.html.erb + - app/views/users/webauthn_setup/new.html.erb + IdentityIdp/RedirectBackLinter: Enabled: true diff --git a/app/views/accounts/_pii.html.erb b/app/views/accounts/_pii.html.erb index 6312ed5e554..6bf05dae6cd 100644 --- a/app/views/accounts/_pii.html.erb +++ b/app/views/accounts/_pii.html.erb @@ -16,7 +16,7 @@

<%= t('headings.account.profile_info') %> - <%= image_tag asset_url('lock.svg'), width: 8 %> + <%= image_tag asset_url('lock.svg'), width: 8, height: 10 %>

@@ -65,7 +65,7 @@ <% unless locked_for_session %>
- <%= image_tag asset_url('lock.svg'), width: 8, class: 'margin-right-1' %> + <%= image_tag asset_url('lock.svg'), width: 8, height: 10, class: 'margin-right-1' %> <%= t('account.security.text') %>
<%= link_to t('account.security.link'), MarketingSite.help_url %> diff --git a/app/views/accounts/_unphishable_badge.html.erb b/app/views/accounts/_unphishable_badge.html.erb index ca37143c294..ff2873288ba 100644 --- a/app/views/accounts/_unphishable_badge.html.erb +++ b/app/views/accounts/_unphishable_badge.html.erb @@ -1,7 +1,7 @@
<%= image_tag( design_system_asset_path('img/alerts/unphishable.svg'), - width: 16, + size: 16, class: 'text-middle', alt: '', ) %> diff --git a/app/views/accounts/_verified_account_badge.html.erb b/app/views/accounts/_verified_account_badge.html.erb index 771ffdf5dc7..f43f4f70768 100644 --- a/app/views/accounts/_verified_account_badge.html.erb +++ b/app/views/accounts/_verified_account_badge.html.erb @@ -1,7 +1,7 @@
<%= image_tag( design_system_asset_path('img/alerts/success.svg'), - width: 16, + size: 16, class: 'text-middle', alt: '', ) %> diff --git a/app/views/idv/cancellations/destroy.html.erb b/app/views/idv/cancellations/destroy.html.erb index 84a641f32f7..2e1654524e8 100644 --- a/app/views/idv/cancellations/destroy.html.erb +++ b/app/views/idv/cancellations/destroy.html.erb @@ -4,5 +4,5 @@ <% c.header { t('idv.cancel.headings.confirmation.hybrid') } %>

<%= t('doc_auth.instructions.switch_back') %>

- <%= image_tag(asset_url('idv/switch.png'), width: 193, alt: t('doc_auth.instructions.switch_back_image')) %> + <%= image_tag(asset_url('idv/switch.png'), width: 193, height: 109, alt: t('doc_auth.instructions.switch_back_image')) %> <% end %> diff --git a/app/views/idv/capture_doc/capture_complete.html.erb b/app/views/idv/capture_doc/capture_complete.html.erb index 4199e3d5dcc..2836472bb6d 100644 --- a/app/views/idv/capture_doc/capture_complete.html.erb +++ b/app/views/idv/capture_doc/capture_complete.html.erb @@ -6,4 +6,4 @@ <%= render PageHeadingComponent.new.with_content(t('doc_auth.instructions.switch_back')) %> -<%= image_tag(asset_url('idv/switch.png'), width: 193, alt: t('doc_auth.instructions.switch_back_image')) %> +<%= image_tag(asset_url('idv/switch.png'), width: 193, height: 109, alt: t('doc_auth.instructions.switch_back_image')) %> diff --git a/app/views/idv/doc_auth/email_sent.html.erb b/app/views/idv/doc_auth/email_sent.html.erb index 33fd4cf5e51..428185b129b 100644 --- a/app/views/idv/doc_auth/email_sent.html.erb +++ b/app/views/idv/doc_auth/email_sent.html.erb @@ -1,6 +1,6 @@ <% title t('titles.doc_auth.verify') %> -<%= image_tag(asset_url('state-id-confirm@3x.png'), width: 210, class: 'margin-bottom-2') %> +<%= image_tag(asset_url('state-id-confirm@3x.png'), width: 210, height: 127, class: 'margin-bottom-2') %> <%= render PageHeadingComponent.new do %> <%= t('doc_auth.instructions.email_sent', email: current_user.confirmed_email_addresses.first.email) %> diff --git a/app/views/idv/doc_auth/link_sent.html.erb b/app/views/idv/doc_auth/link_sent.html.erb index bb952544359..d37c9185006 100644 --- a/app/views/idv/doc_auth/link_sent.html.erb +++ b/app/views/idv/doc_auth/link_sent.html.erb @@ -15,7 +15,7 @@
- <%= image_tag asset_url('idv/phone.png'), alt: t('image_description.camera_mobile_phone') %> + <%= image_tag asset_url('idv/phone.png'), width: 80, height: 119, alt: t('image_description.camera_mobile_phone') %>

<%= t('doc_auth.info.link_sent') %>

diff --git a/app/views/idv/doc_auth/upload.html.erb b/app/views/idv/doc_auth/upload.html.erb index 87da877c1a5..016cf91f931 100644 --- a/app/views/idv/doc_auth/upload.html.erb +++ b/app/views/idv/doc_auth/upload.html.erb @@ -32,6 +32,7 @@ asset_url('idv/phone.png'), alt: t('image_description.camera_mobile_phone'), width: 80, + height: 119, ) %>
diff --git a/app/views/idv/inherited_proofing_cancellations/destroy.html.erb b/app/views/idv/inherited_proofing_cancellations/destroy.html.erb index 81444e35844..116524f15b5 100644 --- a/app/views/idv/inherited_proofing_cancellations/destroy.html.erb +++ b/app/views/idv/inherited_proofing_cancellations/destroy.html.erb @@ -4,5 +4,5 @@ <% c.header { t('inherited_proofing.cancel.headings.confirmation.hybrid') } %>

<%= t('inherited_proofing.cancel.instructions.switch_back') %>

- <%= image_tag(asset_url('inherited_proofing/switch.png'), width: 193, alt: t('inherited_proofing.cancel.instructions.switch_back_image')) %> + <%= image_tag(asset_url('inherited_proofing/switch.png'), width: 193, height: 109, alt: t('inherited_proofing.cancel.instructions.switch_back_image')) %> <% end %> diff --git a/app/views/layouts/account_side_nav.html.erb b/app/views/layouts/account_side_nav.html.erb index 6ea8fc64d54..c010436c021 100644 --- a/app/views/layouts/account_side_nav.html.erb +++ b/app/views/layouts/account_side_nav.html.erb @@ -11,6 +11,8 @@
<%= image_tag( asset_path('user-access.svg'), + width: 180, + height: 59, alt: '', ) %>
diff --git a/app/views/pages/page_took_too_long.html.erb b/app/views/pages/page_took_too_long.html.erb index 3fcf05965c1..2fb599b4643 100644 --- a/app/views/pages/page_took_too_long.html.erb +++ b/app/views/pages/page_took_too_long.html.erb @@ -15,8 +15,13 @@
- <%= image_tag asset_url('logo-white.svg'), width: 150, alt: APP_NAME, - class: 'masthead-brand' %> + <%= image_tag( + asset_url('logo-white.svg'), + width: 150, + height: 20, + alt: APP_NAME, + class: 'masthead-brand', + ) %>
diff --git a/app/views/shared/_banner.html.erb b/app/views/shared/_banner.html.erb index 16c84fae670..fc8e61568c4 100644 --- a/app/views/shared/_banner.html.erb +++ b/app/views/shared/_banner.html.erb @@ -29,6 +29,7 @@
<%= image_tag( asset_url('icon-dot-gov.svg'), + size: 40, alt: 'Dot gov', class: 'usa-banner__icon usa-media-block__img', ) %> @@ -42,6 +43,7 @@
<%= image_tag( asset_url('icon-https.svg'), + size: 40, alt: 'Https', class: 'usa-banner__icon usa-media-block__img', ) %> diff --git a/app/views/shared/_footer_lite.html.erb b/app/views/shared/_footer_lite.html.erb index 1e925e21424..9ebe3b4caf9 100644 --- a/app/views/shared/_footer_lite.html.erb +++ b/app/views/shared/_footer_lite.html.erb @@ -7,7 +7,7 @@ <%= new_window_link_to 'https://gsa.gov', { class: 'text-no-underline h6 margin-right-2 tablet:display-none', 'aria-label': t('shared.footer_lite.gsa') } do %> <%= image_tag asset_url('sp-logos/square-gsa-dark.svg'), - width: 20, alt: '' %> + size: 20, alt: '' %> <% end %>
@@ -15,7 +15,7 @@ <%= new_window_link_to 'https://gsa.gov', { class: 'text-no-underline h6 margin-right-2', 'aria-label': t('shared.footer_lite.gsa') } do %> <%= image_tag asset_url('sp-logos/square-gsa.svg'), - width: 20, class: 'float-left margin-right-1', alt: '' %> + size: 20, class: 'float-left margin-right-1', alt: '' %> <%= t('shared.footer_lite.gsa') %> diff --git a/app/views/users/phone_setup/index.html.erb b/app/views/users/phone_setup/index.html.erb index 07dd7570202..df9b03f6b5a 100644 --- a/app/views/users/phone_setup/index.html.erb +++ b/app/views/users/phone_setup/index.html.erb @@ -2,7 +2,7 @@ <%= render(VendorOutageAlertComponent.new(vendors: [:sms, :voice])) %> -<%= image_tag asset_url('2FA-voice.svg'), alt: '', width: 200, class: 'margin-bottom-2' %> +<%= image_tag asset_url('2FA-voice.svg'), alt: '', width: 200, height: 113, class: 'margin-bottom-2' %> <%= render PageHeadingComponent.new.with_content(t('titles.phone_setup')) %> diff --git a/app/views/users/piv_cac_setup_from_sign_in/success.html.erb b/app/views/users/piv_cac_setup_from_sign_in/success.html.erb index 80bce59cfce..ea8b3bc741b 100644 --- a/app/views/users/piv_cac_setup_from_sign_in/success.html.erb +++ b/app/views/users/piv_cac_setup_from_sign_in/success.html.erb @@ -1,6 +1,6 @@ <% title t('headings.piv_cac_login.success') %> -<%= image_tag asset_url('alert/success.svg'), width: 90, class: 'margin-bottom-2' %> +<%= image_tag asset_url('alert/success.svg'), size: 90, class: 'margin-bottom-2' %> <%= render PageHeadingComponent.new.with_content(t('headings.piv_cac_login.success')) %> diff --git a/app/views/users/totp_setup/new.html.erb b/app/views/users/totp_setup/new.html.erb index 6a0c46fb1e7..7876b436f7e 100644 --- a/app/views/users/totp_setup/new.html.erb +++ b/app/views/users/totp_setup/new.html.erb @@ -28,7 +28,7 @@ <%= c.item(heading: t('forms.totp_setup.totp_step_2')) %> <%= c.item(heading: t('forms.totp_setup.totp_step_3')) do %>
- <%= image_tag @qrcode, skip_pipeline: true, alt: t('image_description.totp_qrcode') %> + <%= image_tag @qrcode, size: 240, skip_pipeline: true, alt: t('image_description.totp_qrcode') %>

<%= t('instructions.mfa.authenticator.manual_entry') %>

diff --git a/lib/linters/image_size_linter.rb b/lib/linters/image_size_linter.rb new file mode 100644 index 00000000000..7a23a49b8e1 --- /dev/null +++ b/lib/linters/image_size_linter.rb @@ -0,0 +1,37 @@ +module RuboCop + module Cop + module IdentityIdp + # This lint ensures that images rendered with Rails tag helpers include a size attribute + # (`width` and `height`, or `size`), which is a best practice to avoid layout shifts. + # + # @see https://web.dev/optimize-cls/#images-without-dimensions + # + # @example + # # bad + # image_tag 'example.svg' + # + # # good + # image_tag 'example.svg', width: 10, height: 20 + # + class ImageSizeLinter < RuboCop::Cop::Cop + MSG = 'Assign width and height to images'.freeze + + RESTRICT_ON_SEND = [:image_tag] + + def on_send(node) + add_offense(node, location: :expression) if !valid?(node) + end + + private + + def valid?(node) + options = node.arguments.last + return false if options.type != :hash + return true if options.child_nodes.any? { |child_node| child_node.type == :kwsplat } + key_names = options.keys.map { |key| key.value } + key_names.include?(:size) || (key_names.include?(:width) && key_names.include?(:height)) + end + end + end + end +end diff --git a/spec/lib/linters/image_size_linter_spec.rb b/spec/lib/linters/image_size_linter_spec.rb new file mode 100644 index 00000000000..b01bbfd134c --- /dev/null +++ b/spec/lib/linters/image_size_linter_spec.rb @@ -0,0 +1,43 @@ +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../lib/linters/image_size_linter' + +describe RuboCop::Cop::IdentityIdp::ImageSizeLinter do + include CopHelper + include RuboCop::RSpec::ExpectOffense + + let(:config) { RuboCop::Config.new } + let(:cop) { RuboCop::Cop::IdentityIdp::ImageSizeLinter.new(config) } + + it 'registers offense when calling image_tag without any size attributes' do + expect_offense(<<~RUBY) + image_tag 'example.svg' + ^^^^^^^^^^^^^^^^^^^^^^^ Assign width and height to images + RUBY + end + + it 'registers offense when calling image_tag with only one of width or height' do + expect_offense(<<~RUBY) + image_tag 'example.svg', width: 10 + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Assign width and height to images + RUBY + end + + it 'registers no offense if there is ambiguous hash splatting' do + expect_no_offenses(<<~RUBY) + image_tag 'example.svg', **size_attributes + RUBY + end + + it 'registers no offense when calling image_tag with size' do + expect_no_offenses(<<~RUBY) + image_tag 'example.svg', size: 10 + RUBY + end + + it 'registers no offense when calling image_tag with width and height' do + expect_no_offenses(<<~RUBY) + image_tag 'example.svg', width: 10, height: 20 + RUBY + end +end