Merge pull request #2159 from MushroomObserver/nimmo-refactor-carousels

Refactor carousels

app/assets/stylesheets/mo/_utilities.scss

@@ -357,6 +357,14 @@
   padding: 1rem !important;
 }
 
+.p-4 {
+  padding: 1.5rem !important;
+}
+
+.p-5 {
+  padding: 3rem !important;
+}
+
 .p-5px {
   padding: 5px !important;
 }

app/controllers/account/profile/images_controller.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 # Clicking on an image currently fires a GET to these actions... because it
-# comes from a link made by ImageHelper#interactive_image(link: url_args)
-# with CRUD refactor, change ImageHelper helper to fire a POST somehow?
+# comes from a link made by ImagesHelper#interactive_image(link: url_args)
+# with CRUD refactor, change ImagesHelper helper to fire a POST somehow?
 
 # No need to remove_images from Account profile: reuse_image removes image
 module Account::Profile

app/controllers/glossary_terms/images_controller.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 # Clicking on an image currently fires a GET to these actions... because it
-# comes from a link made by thumbnail_helper#interactive_image(link: url_args)
-# with CRUD refactor, change thumbnail helper to fire a POST somehow?
+# comes from a link made by ImagesHelper#interactive_image(link: url_args)
+# with CRUD refactor, change ImagesHelper to fire a POST somehow?
 
 module GlossaryTerms
   class ImagesController < ApplicationController

app/controllers/observations/images_controller.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 # Clicking on an image currently fires a GET to these actions... because it
-# comes from a link made by ImageHelper#interactive_image(link: url_args)
-# with CRUD refactor, change ImageHelper helper to fire a POST somehow?
+# comes from a link made by ImagesHelper#interactive_image(link: url_args)
+# with CRUD refactor, change ImagesHelper helper to fire a POST somehow?
 
 module Observations
   # Upload, attach, detach, edit Observation Images

app/helpers/carousel_helper.rb

@@ -1,72 +1,6 @@
 # frozen_string_literal: true
 
 module CarouselHelper
-  # Use this to print a carousel within a div
-  # Required args: images:
-  # Optional args: object: (the carousel is usually for an object)
-  # top_img: (defaults to first), (carousel)title:, (image_edit)links:
-  # thumbnails: true for thumbnail navigation
-  # img_args: args for ImagePresenter
-  #
-  # Note: uses concat(x) instead of [x,y].safe_join because of conditionals
-  def carousel_html(**args)
-    args[:images] ||= nil
-    args[:object] ||= nil
-    args[:size] ||= :large
-    args[:top_img] ||= args[:images].first
-    args[:title] ||= :IMAGES.t
-    args[:links] ||= ""
-    args[:thumbnails] = true if args[:thumbnails].nil?
-    type = args[:object]&.type_tag || "image"
-    args[:html_id] ||= "#{type}_#{args[:object].id}_carousel"
-
-    capture do
-      if !args[:images].nil? && args[:images].any?
-        concat(carousel_basic_html(**args))
-      else
-        if args[:thumbnails] # only need this heading on a show page
-          concat(carousel_heading(args[:title], args[:links]))
-        end
-        concat(carousel_no_images_message)
-      end
-    end
-  end
-
-  def carousel_basic_html(**args)
-    tag.div(class: "carousel slide", id: args[:html_id],
-            data: { ride: "false", interval: "false" }) do
-      concat(carousel_heading(args[:title], args[:links])) if args[:thumbnails]
-      concat(tag.div(class: "carousel-inner bg-light", role: "listbox") do
-        args[:images].each do |image|
-          concat(carousel_item(image, **args))
-        end
-        concat(carousel_controls(args[:html_id])) if args[:images].length > 1
-      end)
-      concat(carousel_thumbnails(**args)) if args[:thumbnails]
-    end
-  end
-
-  # args are leftover from template, could be used
-  def carousel_item(image, **args)
-    # Caption needs object for copyright info
-    img_args = args.except(:images, :object, :top_img, :title, :links,
-                           :thumbnails, :html_id)
-    presenter_args = img_args.merge({ fit: :contain, original: true,
-                                      extra_classes: "carousel-image" })
-    presenter = ImagePresenter.new(image, presenter_args)
-    active = image == args[:top_img] ? "active" : ""
-
-    tag.div(class: class_names("item", active)) do
-      concat(image_tag(presenter.img_src, presenter.options_lazy))
-      if presenter.image_link
-        concat(image_stretched_link(presenter.image_link,
-                                    presenter.image_link_method))
-      end
-      concat(lightbox_link(presenter.lightbox_data))
-      concat(carousel_caption(image, args[:object], presenter))
-    end
-  end
-
   # Very similar to an interactive_image caption
   def carousel_caption(image, object, presenter)
     classes = "carousel-caption"
@@ -115,29 +49,8 @@ def carousel_heading(title, links = "")
     end
   end
 
-  def carousel_thumbnails(**args)
-    tag.ol(class: "carousel-indicators panel-footer py-2 px-0 mb-0") do
-      args[:images].each_with_index do |image, index|
-        active = image == args[:top_img] ? "active" : ""
-
-        concat(tag.li(class: class_names("carousel-indicator mx-1", active),
-                      data: { target: "##{args[:html_id]}",
-                              slide_to: index.to_s }) do
-                 carousel_thumbnail(image)
-               end)
-      end
-    end
-  end
-
-  def carousel_thumbnail(image)
-    presenter_args = { fit: :contain, extra_classes: "carousel-thumbnail" }
-    presenter = ImagePresenter.new(image, presenter_args)
-
-    image_tag(presenter.img_src, presenter.options_lazy)
-  end
-
   def carousel_no_images_message
     tag.div(:show_observation_no_images.l,
-            class: "p-4 w-100 h-100 text-center h3 text-muted")
+            class: "p-4 my-5 w-100 h-100 text-center h3 text-muted")
   end
 end

app/helpers/images_helper.rb

@@ -3,11 +3,12 @@
 module ImagesHelper
   # Draw an image with all the fixin's. Takes either an Image instance or an id.
   #
-  # Note: this is NOT rendering a partial because nested partials have been
-  # demonstrated to be VERY slow in loops or collections.
+  # TODO: Make this a component. This should probably not be a partial: using
+  # nested partials has been demonstrated to be VERY slow in loops or
+  # collections. (Caching helps though).
   #
   # Uses ImagePresenter to assemble data.
-  #
+
   #   link::             Hash of { controller: xxx, action: xxx, etc. }
   #   size::             Size to show, default is thumbnail.
   #   votes::            Show vote buttons?
@@ -16,19 +17,15 @@ module ImagesHelper
   #   html_options::     Additional HTML attributes to add to <img> tag.
   #   notes::            Show image notes??
   #
-  # USE: interactive_image(
-  #   image,
-  #   args = {
-  #     notes: "",
-  #     extra_classes: ""
-  #   }
-  # )
+  # USE: interactive_image( image, args = { notes: "", extra_classes: "" } )
   def interactive_image(image, **args)
+    # Caption needs object for copyright info
     presenter = ImagePresenter.new(image, args)
     set_width = presenter.width.present? ? "width: #{presenter.width}px;" : ""
 
     [
-      tag.div(class: "image-sizer position-relative mx-auto",
+      tag.div(id: presenter.html_id,
+              class: "image-sizer position-relative mx-auto",
               style: set_width.to_s) do
         [
           tag.div(class: "image-lazy-sizer overflow-hidden",
@@ -50,6 +47,15 @@ def interactive_image(image, **args)
     ].safe_join
   end
 
+  # Args for the interactive image on Images#show (particular to that context)
+  def image_show_presenter_args
+    { size: :huge,
+      image_link: "#",
+      img_class: "huge-image",
+      theater_on_click: true,
+      votes: false }
+  end
+
   # Needs object for copyright info
   def image_info(image, object, original: false)
     notes = []

app/helpers/matrix_box_helper.rb

@@ -65,17 +65,19 @@ def matrix_box_image(image = nil, **args)
 
   # for matrix_box_carousels:
   # def matrix_box_images(presenter)
-  #   presenter.image_data includes context: :matrix_box where appropriate
+  #   presenter.image_data includes full_width: true where appropriate
   #   images = presenter.image_data[:images]
   #   image_args = local_assigns.
   #                except(:columns, :object, :object_counter,
   #                       :object_iteration).
   #                merge(presenter.image_data.except(:images) || {})
   #   top_img = presenter.image_data[:thumb_image] || images.first
+  #   carousel_locals = { object: object, images: images, top_img: top_img,
+  #                       thumbnails: true, **image_args }
   #
   #   tag.div(class: "thumbnail-container") do
-  #     carousel_html(object: object, images: images, top_img: top_img,
-  #                   thumbnails: false, **image_args)
+  #     # don't use a partial though, too slow. wait til we are using components
+  #     render(partial: "shared/carousel", locals: carousel_locals)
   #   end
   # end
 

app/presenters/image_presenter.rb

@@ -13,7 +13,8 @@ class ImagePresenter < BasePresenter
     :image_link_method, # needed for helper
     :lightbox_data,     # contains data passed to lightbox (incl. caption)
     :votes,             # show votes? boolean
-    :original           # show original image filename? (boolean)
+    :original,          # show original image filename? (boolean)
+    :html_id            # dom_id for broadcasts (image.id added)
 
   def initialize(image, args = {})
     super
@@ -49,7 +50,8 @@ def initialize(image, args = {})
       votes: true,
       original: false,
       is_set: true,
-      context: false # false to constrain width
+      full_width: false, # false to constrain width
+      id_prefix: "interactive_image"
     }
     args = default_args.merge(args)
     img_urls = image&.all_urls || Image.all_urls(image_id)
@@ -98,6 +100,7 @@ def args_to_presenter(image, image_id, img_urls, args)
     self.image_link_method = args[:link_method]
     self.votes = args[:votes]
     self.original = args[:original]
+    self.html_id = "#{args[:id_prefix]}_#{image_id}"
   end
 
   def sizing_info_to_presenter(image, args)
@@ -112,7 +115,7 @@ def sizing_info_to_presenter(image, args)
     img_padding = "133.33" if img_padding.to_i > 133 # default for tall
     self.proportion = img_padding
 
-    if args[:context] == :matrix_box
+    if args[:full_width] == true
       self.width = false
     else
       # Constrain width to expected dimensions for img size (not layout)

app/presenters/matrix_box_presenter.rb

@@ -79,7 +79,7 @@ def image_to_presenter(image)
       # for matrix_box_carousels:
       # images: [image],
       image_link: image.show_link_args,
-      context: :matrix_box
+      full_width: true
     }
   end
 
@@ -112,7 +112,7 @@ def observation_to_presenter(observation)
       # thumb_image: thumb_image,
       image_link: observation.show_link_args, # false for thumb thru images
       obs: observation,
-      context: :matrix_box
+      full_width: true
     }
   end
 
@@ -140,7 +140,7 @@ def user_to_presenter(user)
       # images: [user.image_id],
       image_link: user.show_link_args,
       votes: false,
-      context: :matrix_box
+      full_width: true
     }
   end
 
@@ -164,7 +164,7 @@ def figure_out_rss_log_target_images(target)
       # images: images,
       # image_link: target.show_link_args,
       obs: obs_data(target),
-      context: :matrix_box
+      full_width: true
     }
   end
 

app/views/controllers/contributors/_contributor.erb

@@ -10,7 +10,7 @@ columns ||= "col-xs-12 col-sm-6 col-md-4 col-lg-3"
       <%= if user.image_id
         tag.div(class: "thumbnail-container") do
           interactive_image(user.image_id, image_link: user_path(user.id),
-                            votes: false, context: :matrix_box)
+                            votes: false, full_width: true)
         end
       end %>
       <%= tag.div(class: "panel-body") do

app/views/controllers/glossary_terms/show.html.erb

@@ -13,7 +13,10 @@ add_tab_set(glossary_term_show_tabs(term: @glossary_term, user: @user))
   </div>
   <% if @glossary_term.thumb_image %>
     <div class="col-sm-4">
-      <%= interactive_image(@glossary_term.thumb_image, size: :medium, votes: true) %>
+      <%= interactive_image(
+        @glossary_term.thumb_image, size: :medium, votes: true,
+                                    id_prefix: "glossary_term_image"
+      ) %>
     </div>
   <% end %>
 </div><!--.row-->
@@ -28,7 +31,9 @@ add_tab_set(glossary_term_show_tabs(term: @glossary_term, user: @user))
 <div class="row">
   <% @other_images.each do |image|  %>
     <div class="col-sm-4">
-      <%= panel_block do interactive_image(image, votes: true) end %>
+      <%= panel_block do
+        interactive_image(image, votes: true, id_prefix: "glossary_term_image")
+      end %>
     </div>
   <% end %>
 </div><!--.row-->

app/views/controllers/images/show/_image_panel.html.erb

@@ -26,12 +26,7 @@
   </div><!--.panel-heading-->
   <div class="panel-body">
 
-    <%= interactive_image(@image,
-                          size: :huge,
-                          image_link: {},
-                          img_class: "huge-image",
-                          theater_on_click: true,
-                          votes: false) %>
+    <%= interactive_image(@image, **image_show_presenter_args) %>
 
     <div class="mt-3 text-center">
       <% if User.current %>

app/views/controllers/info/site_stats.html.erb

@@ -11,9 +11,9 @@ add_tab_set(info_site_stats_tabs)
     if obs_length > 0 %>
       <% @observations[0,3].each do |obs| %>
         <div class="pb-1">
-          <%= interactive_image(obs.thumb_image,
-            image_link: observation_path(obs.id),
-            votes: true) %><br/><br/>
+          <%= interactive_image(
+            obs.thumb_image, image_link: observation_path(obs.id), votes: true
+          ) %><br/><br/>
         </div>
       <% end %>
     <% end %>
@@ -37,9 +37,9 @@ add_tab_set(info_site_stats_tabs)
   <div class="hidden-xs col-md-3">
     <% if obs_length > 3 %>
       <% @observations[3,3].each do |obs| %>
-        <%= interactive_image(obs.thumb_image,
-          image_link: observation_path(obs.id),
-          votes: true) %><br/><br/>
+        <%= interactive_image(
+          obs.thumb_image, image_link: observation_path(obs.id), votes: true
+        ) %><br/><br/>
       <% end %>
     <% end %>
   </div>

app/views/controllers/names/show/_confident_observations_panel.html.erb

@@ -8,6 +8,6 @@ carousel_locals = {
 <%= if @best_images&.length&.positive?
   tag.div(class: "panel panel-default",
           id: "name_confident_observations") do
-    carousel_html(**carousel_locals)
+    render(partial: "shared/carousel", locals: carousel_locals)
   end
 end %>

app/views/controllers/observations/namings/suggestions/show.html.erb

@@ -55,9 +55,10 @@ add_tab_set(naming_suggestion_tabs(obs: @observation))
             <% end %>
           </td>
           <td>
-            <%= interactive_image(sugg.image_obs.thumb_image,
-                          image_link: image_path(id: sugg.image_obs.id)) \
-                          if sugg.image_obs.present? %>
+            <%= interactive_image(
+              sugg.image_obs.thumb_image,
+              image_link: image_path(id: sugg.image_obs.id)
+            ) if sugg.image_obs.present? %>
         </td>
       </tr>
     <% end %>