]> git.openstreetmap.org Git - rails.git/commitdiff
Specify avatar dimensions in html tags
authorAndy Allan <git@gravitystorm.co.uk>
Wed, 29 Sep 2021 17:47:20 +0000 (18:47 +0100)
committerAndy Allan <git@gravitystorm.co.uk>
Wed, 13 Oct 2021 13:05:02 +0000 (14:05 +0100)
This prevents reflow when the images are loaded by the browser.

ActiveStorage variants are resized lazily when the image is requested,
so we only know the dimensions if the image was already loaded. This
means that there will be one reflow just after a new avatar is first
viewed.

app/helpers/user_helper.rb
test/helpers/user_helper_test.rb

index 69b0f6d6bc5ca833e9df2983b60596fee60f95d2..eed55cd9d9a722d00a56dea6a7ee5073a582430a 100644 (file)
@@ -8,9 +8,9 @@ module UserHelper
     if user.image_use_gravatar
       user_gravatar_tag(user, options)
     elsif user.avatar.attached?
     if user.image_use_gravatar
       user_gravatar_tag(user, options)
     elsif user.avatar.attached?
-      image_tag user_avatar_variant(user, :resize_to_limit => [100, 100]), options
+      user_avatar_variant_tag(user, { :resize_to_limit => [100, 100] }, options)
     else
     else
-      image_tag "avatar_large.png", options
+      image_tag "avatar_large.png", options.merge(:width => 100, :height => 100)
     end
   end
 
     end
   end
 
@@ -19,11 +19,11 @@ module UserHelper
     options[:alt] ||= ""
 
     if user.image_use_gravatar
     options[:alt] ||= ""
 
     if user.image_use_gravatar
-      user_gravatar_tag(user, options)
+      user_gravatar_tag(user, options.merge(:size => 50))
     elsif user.avatar.attached?
     elsif user.avatar.attached?
-      image_tag user_avatar_variant(user, :resize_to_limit => [50, 50]), options
+      user_avatar_variant_tag(user, { :resize_to_limit => [50, 50] }, options)
     else
     else
-      image_tag "avatar_small.png", options
+      image_tag "avatar_small.png", options.merge(:width => 50, :height => 50)
     end
   end
 
     end
   end
 
@@ -32,11 +32,11 @@ module UserHelper
     options[:alt] ||= ""
 
     if user.image_use_gravatar
     options[:alt] ||= ""
 
     if user.image_use_gravatar
-      user_gravatar_tag(user, options)
+      user_gravatar_tag(user, options.merge(:size => 50))
     elsif user.avatar.attached?
     elsif user.avatar.attached?
-      image_tag user_avatar_variant(user, :resize_to_limit => [50, 50]), options
+      user_avatar_variant_tag(user, { :resize_to_limit => [50, 50] }, options)
     else
     else
-      image_tag "avatar_small.png", options
+      image_tag "avatar_small.png", options.merge(:width => 50, :height => 50)
     end
   end
 
     end
   end
 
@@ -69,6 +69,22 @@ module UserHelper
   private
 
   # Local avatar support
   private
 
   # Local avatar support
+  def user_avatar_variant_tag(user, variant_options, options)
+    if user.avatar.variable?
+      variant = user.avatar.variant(variant_options)
+      # https://stackoverflow.com/questions/61893089/get-metadata-of-active-storage-variant/67228171
+      if variant.processed?
+        metadata = variant.processed.send(:record).image.blob.metadata
+        if metadata["width"]
+          options[:width] = metadata["width"]
+          options[:height] = metadata["height"]
+        end
+      end
+      image_tag variant, options
+    else
+      image_tag user.avatar, options
+    end
+  end
 
   def user_avatar_variant(user, options)
     if user.avatar.variable?
 
   def user_avatar_variant(user, options)
     if user.avatar.variable?
@@ -90,7 +106,7 @@ module UserHelper
 
   def user_gravatar_tag(user, options = {})
     url = user_gravatar_url(user, options)
 
   def user_gravatar_tag(user, options = {})
     url = user_gravatar_url(user, options)
-    options.delete(:size)
+    options[:height] = options[:width] = options.delete(:size) || 100
     image_tag url, options
   end
 end
     image_tag url, options
   end
 end
index 758baa1cb7d42dc9d56567c548f0ea6548b09322..8cef4d2dc570512340f50500801eb0d030e33dfb 100644 (file)
@@ -12,7 +12,6 @@ class UserHelperTest < ActionView::TestCase
 
     image = user_image(user, :class => "foo")
     assert_match %r{^<img class="foo" .* src="/images/avatar_large.png" />$}, image
 
     image = user_image(user, :class => "foo")
     assert_match %r{^<img class="foo" .* src="/images/avatar_large.png" />$}, image
-
     image = user_image(gravatar_user)
     assert_match %r{^<img class="user_image" .* src="http://www.gravatar.com/avatar/.*" />$}, image
 
     image = user_image(gravatar_user)
     assert_match %r{^<img class="user_image" .* src="http://www.gravatar.com/avatar/.*" />$}, image
 
@@ -66,6 +65,50 @@ class UserHelperTest < ActionView::TestCase
     assert_match %r{^http://www.gravatar.com/avatar/}, url
   end
 
     assert_match %r{^http://www.gravatar.com/avatar/}, url
   end
 
+  def test_user_image_sizes_default_image
+    user = create(:user)
+
+    image = user_image(user)
+    assert_match %r{^<img .* width="100" height="100" .* />$}, image
+
+    thumbnail = user_thumbnail(user)
+    assert_match %r{^<img .* width="50" height="50" .* />$}, thumbnail
+  end
+
+  def test_user_image_sizes_avatar
+    user = create(:user)
+    user.avatar.attach(:io => File.open("test/gpx/fixtures/a.gif"), :filename => "a.gif")
+
+    # first time access, no width or height is found
+    image = user_image(user)
+    assert_no_match %r{^<img .* width="100" height="100" .* />$}, image
+
+    thumbnail = user_thumbnail(user)
+    assert_no_match %r{^<img .* width="50" height="50" .* />$}, thumbnail
+
+    # Small hacks to simulate what happens when the images have been fetched at least once before
+    variant = user.avatar.variant(:resize_to_limit => [100, 100])
+    variant.processed.send(:record).image.blob.analyze
+    variant = user.avatar.variant(:resize_to_limit => [50, 50])
+    variant.processed.send(:record).image.blob.analyze
+
+    image = user_image(user)
+    assert_match %r{^<img .* width="100" height="100" .* />$}, image
+
+    thumbnail = user_thumbnail(user)
+    assert_match %r{^<img .* width="50" height="50" .* />$}, thumbnail
+  end
+
+  def test_user_image_sizes_gravatar
+    user = create(:user, :image_use_gravatar => true)
+
+    image = user_image(user)
+    assert_match %r{^<img .* width="100" height="100" .* />$}, image
+
+    thumbnail = user_thumbnail(user)
+    assert_match %r{^<img .* width="50" height="50" .* />$}, thumbnail
+  end
+
   def test_openid_logo
     logo = openid_logo
     assert_match %r{^<img .* class="openid_logo" src="/images/openid_small.png" />$}, logo
   def test_openid_logo
     logo = openid_logo
     assert_match %r{^<img .* class="openid_logo" src="/images/openid_small.png" />$}, logo