Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment on lines +64 to +70
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting that I'd hoped to have disabled these rules inline at the specific point in the file where the image_tag is called, but it doesn't currently appear to be possible to do with ERBLint.

Upstream issue: Shopify/erb_lint#243


IdentityIdp/RedirectBackLinter:
Enabled: true

Expand Down
4 changes: 2 additions & 2 deletions app/views/accounts/_pii.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<div class="grid-col-12">
<h2 class="margin-0">
<%= t('headings.account.profile_info') %>
<%= image_tag asset_url('lock.svg'), width: 8 %>
<%= image_tag asset_url('lock.svg'), width: 8, height: 10 %>
</h2>
</div>
</div>
Expand Down Expand Up @@ -65,7 +65,7 @@
<% unless locked_for_session %>
<div class="grid-row bg-base-lightest padding-x-2 padding-y-1 clearfix fs-12p">
<div class="grid-col-12">
<%= 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') %>
</div>
<%= link_to t('account.security.link'), MarketingSite.help_url %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/accounts/_unphishable_badge.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<div class="lg-verification-badge">
<%= image_tag(
design_system_asset_path('img/alerts/unphishable.svg'),
width: 16,
size: 16,
class: 'text-middle',
alt: '',
) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/accounts/_verified_account_badge.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<div class="lg-verification-badge">
<%= image_tag(
design_system_asset_path('img/alerts/success.svg'),
width: 16,
size: 16,
class: 'text-middle',
alt: '',
) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/idv/cancellations/destroy.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
<% c.header { t('idv.cancel.headings.confirmation.hybrid') } %>

<p><%= t('doc_auth.instructions.switch_back') %></p>
<%= 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 %>
2 changes: 1 addition & 1 deletion app/views/idv/capture_doc/capture_complete.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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')) %>
2 changes: 1 addition & 1 deletion app/views/idv/doc_auth/email_sent.html.erb
Original file line number Diff line number Diff line change
@@ -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) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/idv/doc_auth/link_sent.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

<div class="grid-row">
<div class="grid-col-12 tablet:grid-col-3">
<%= 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') %>
</div>
<div class="grid-col-12 tablet:grid-col-9">
<p><%= t('doc_auth.info.link_sent') %></p>
Expand Down
1 change: 1 addition & 0 deletions app/views/idv/doc_auth/upload.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
asset_url('idv/phone.png'),
alt: t('image_description.camera_mobile_phone'),
width: 80,
height: 119,
) %>
</div>
<div class="grid-col-12 tablet:grid-col-9">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
<% c.header { t('inherited_proofing.cancel.headings.confirmation.hybrid') } %>

<p><%= t('inherited_proofing.cancel.instructions.switch_back') %></p>
<%= 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 %>
2 changes: 2 additions & 0 deletions app/views/layouts/account_side_nav.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
<div class="grid-offset-2 grid-col-3">
<%= image_tag(
asset_path('user-access.svg'),
width: 180,
height: 59,
alt: '',
) %>
</div>
Expand Down
9 changes: 7 additions & 2 deletions app/views/pages/page_took_too_long.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@
<div class="cover-container">
<div class="masthead clearfix">
<div class="inner">
<%= 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',
) %>
</div>
</div>
<div class="inner cover">
Expand Down
2 changes: 2 additions & 0 deletions app/views/shared/_banner.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<div class="usa-banner__guidance tablet:grid-col-6">
<%= image_tag(
asset_url('icon-dot-gov.svg'),
size: 40,
alt: 'Dot gov',
class: 'usa-banner__icon usa-media-block__img',
) %>
Expand All @@ -42,6 +43,7 @@
<div class="usa-banner__guidance tablet:grid-col-6">
<%= image_tag(
asset_url('icon-https.svg'),
size: 40,
alt: 'Https',
class: 'usa-banner__icon usa-media-block__img',
) %>
Expand Down
4 changes: 2 additions & 2 deletions app/views/shared/_footer_lite.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
<%= 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 %>
</div>

<div class="display-none tablet:display-flex usa-dark-background bg-primary-darker">
<%= 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: '' %>
<span>
<%= t('shared.footer_lite.gsa') %>
</span>
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/phone_setup/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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')) %>

Expand Down
Original file line number Diff line number Diff line change
@@ -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')) %>

Expand Down
2 changes: 1 addition & 1 deletion app/views/users/totp_setup/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
<div class="text-center">
<%= 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') %>
</div>
<p><%= t('instructions.mfa.authenticator.manual_entry') %></p>
<code class="display-block margin-y-2 font-family-mono padding-y-2 padding-x-1 border-base-lighter border-1px text-bold text-wrap-anywhere" id="qr-code">
Expand Down
37 changes: 37 additions & 0 deletions lib/linters/image_size_linter.rb
Original file line number Diff line number Diff line change
@@ -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
43 changes: 43 additions & 0 deletions spec/lib/linters/image_size_linter_spec.rb
Original file line number Diff line number Diff line change
@@ -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