Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lazy load images using lazyload-rails gem #8043

Merged
merged 1 commit into from
Jun 23, 2020
Merged

lazy load images using lazyload-rails gem #8043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ gem 'lemmatizer', '~> 0.2.2'
gem 'mailman', require: false
# To implement fontawesome v4.7.0
gem "font-awesome-rails"
gem "lazyload-rails"

# To convert html to markdown
gem 'reverse_markdown'
Expand Down
3 changes: 2 additions & 1 deletion app/assets/javascripts/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//
//= require jquery
//= require jquery_ujs
//= require jquery-lazyload/jquery.lazyload.js
//= require debounce/index.js
//= require bootstrap/dist/js/bootstrap.bundle.min.js
//= require bootstrap-3-typeahead/bootstrap3-typeahead.min.js
Expand Down Expand Up @@ -60,4 +61,4 @@
//= require validation.js
//= require submit_form_ajax.js
//= require urlMapHash.js
//= require spam2.js
//= require spam2.js
10 changes: 8 additions & 2 deletions app/views/dashboard/_node_default.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
<%= render partial: 'dashboard/node_moderate', locals: { node: node } %>

<% if node.main_image %>
<a class="img" href="<%= node.path %>"><img src="<%= node.main_image.path(:default) %>" style="width:100%;" /></a>
<a class="img" href="<%= node.path %>"><%= image_tag(node.main_image.path(:default), style:'width:100%;') %></a>
Copy link
Member

Choose a reason for hiding this comment

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

Hi @Tlazypanda - this seems to have disrupted the loading of some images which are "blob" type - see for example the admittedly odd ones in this view:

https://publiclab.org/embed/grid/grid:infragram-upload

See how these actually use a dataurl as the image? I know its weird but I see the image_tag method is replacing the filename with blob - can we include a workaround, perhaps, if the image path is a data url?

This is a pretty weird workflow... i feel like maybe I'm missing something!

Also, I'm trying to fix the somewhat related assets on that template (for use in embedded content), here:
36c54a2

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hey @jywarren I was thinking of writing a conditional rendering for this but am having some difficulty in defining the condition I was thinking of something like:-
<% elsif current_user.photo.url.start_with?("data:image") %> but doesn't seem to work... My understanding of the models may not be at point here can you help me out 😅

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, hmm. Are we mixing up blobs with data_urls? This is a bit mysterious! Uh, is it possible that the lazy load gem is converting a data-url into a blob?

AH! I remember, i wrote some really obscure code here. Let's see...

OMG YES -- #6931 and #6930 -- deep mysterious code! But yes, it has the data-uri-to-blob method there. That should help us!

Copy link
Member

Choose a reason for hiding this comment

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

Should we consider just turning off lazy loading for this one instance in order to recover the functionality? @TildaDares @Manasa2850 @17sushmita are any of you interested in this particular bug? Thank you all!

Copy link
Member

Choose a reason for hiding this comment

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

I was looking into this and was playing around with some modifications on the unstable but I'm not sure how to reproduce this on my local. Would love to get any help regarding this.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately I'm not sure how to reproduce... I'm not sure how the blob images are saved! Can we dig into the raw html shown on unstable to try to learn more?

Copy link
Member

Choose a reason for hiding this comment

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

My best guess here is that we should just turn off lazy loading for this one type of image.

Copy link
Member

Choose a reason for hiding this comment

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

OK this file is for the old dashboard! So ignore this particular file, we'll revert the one specifically for the new card-style display of nodes, below, i'll leave a comment.

<% elsif node.scraped_image %>
<a class="img" href="<%= node.path %>"><img src="<%= node.scraped_image %>" style="width:100%;" /></a>
<a class="img" href="<%= node.path %>"><%= image_tag(node.scraped_image, style:'width:100%;') %></a>
<% end %>

<h4><a href="<%= node.path %>"><%= node.title %></a> <% if node.status == 3 %><span style="font-size: small;" class="badge badge-success">Draft</span><% end %></h4>
Expand All @@ -17,3 +17,9 @@

</div>
</div>

<script>
$(function(){
$("img").lazyload();
});
</script>
16 changes: 11 additions & 5 deletions app/views/dashboard/_node_question.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
<div class="col-lg-6 note-container-question">
<div class="note note-pane note-question<% if node.status == 4 %> moderated<% end %>">

<div class="header-icon"><i class="fa fa-question-circle"></i> <a href="/q/<%= node.id %>"><i class="fa fa-link"></i></a></div>

<div style="overflow: hidden;">

<%= render partial: 'dashboard/node_moderate', locals: { node: node } %>

<% if node.main_image %>
<a class="img" href="<%= node.path(:question) %>"><img src="<%= node.main_image.path(:default) %>" style="width:100%;" /></a>
<a class="img" href="<%= node.path(:question) %>"><%= image_tag(node.main_image.path(:default), style:'width:100%;') %></a>
<% elsif node.scraped_image %>
<a class="img" href="<%= node.path %>"><img src="<%= node.scraped_image %>" style="width:100%;" /></a>
<a class="img" href="<%= node.path %>"><%= image_tag(node.scraped_image, style:'width:100%;') %></a>
<% end %>

<h4><a href="<%= node.path(:question) %>"><%= node.title %></a></h4>

<p class="meta"><%= translation('dashboard._node_question.question') %> <%= render partial: "dashboard/node_meta", locals: { node: node } %></p>

</div>
</div>
</div>

<script>
$(function(){
$("img").lazyload();
});
</script>
10 changes: 8 additions & 2 deletions app/views/notes/_card.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<div class="card">
<% if node.main_image %>
<a class="card-img-top img" style="overflow: hidden; height:10em;"<% if @widget %>target="_blank"<% end %> href="<%= node.path %>"><img src="<%= node.main_image.path(:default) %>" style="width:100%;"/></a>
Copy link
Member

Choose a reason for hiding this comment

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

So let's revert this entire file, only. That should solve it!

<a class="card-img-top img" style="overflow: hidden; height:10em;"<% if @widget %>target="_blank"<% end %> href="<%= node.path %>"><%= image_tag(node.main_image.path(:default), style:'width:100%;') %></a>
<% elsif node.scraped_image %>
<a class="card-img-top img" style="overflow: hidden; height:10em;" href="<%= node.path %>"><img src="<%= node.scraped_image %>" style="width:100%;" /></a>
<a class="card-img-top img" style="overflow: hidden; height:10em;" href="<%= node.path %>"><%= image_tag(node.scraped_image, style:'width:100%;') %></a>
<% else %>
<a class="imgg" style="height:10em; margin-bottom: 10px;padding: 1rem;">
<i class="fa fa-picture-o note-not aria-hidden="true" style="color: #ccc; font-size: 6em;"></i>
Expand Down Expand Up @@ -66,3 +66,9 @@
</div>
</small>
</div>

<script>
$(function(){
$("img").lazyload();
});
</script>
7 changes: 5 additions & 2 deletions app/views/notes/show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

<% if @node.main_image %>
<a class="main-image" style="max-width:100%;width:800px;" href="<%= @node.main_image.path(:original) %>">
<img style="max-height:600px;max-width:100%;" class="rounded d-none d-lg-inline" src="<%= @node.main_image.path(:large) %>" />
<img style="width:100%;" class="rounded d-lg-none" src="<%= @node.main_image.path(:large) %>" />
<%= image_tag(@node.main_image.path(:large), class:"rounded d-none d-lg-inline", style:'max-height:600px;max-width:100%;') %>
<%= image_tag(@node.main_image.path(:large), class:"rounded d-lg-none", style:'width:100%;') %>
</a>
<!--<div class="expand"><a onClick="$('.main-image').toggleClass('compressed');"><i class="fa fa-angle-down"></i></a></div>-->
<% end %>
Expand Down Expand Up @@ -138,4 +138,7 @@
$("#collapse-button").click(function(){
$(this).remove();
});
$(function(){
$("img").lazyload();
});
</script>
8 changes: 5 additions & 3 deletions app/views/questions/show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
<%= javascript_include_tag('comment_expand') %>
<%= javascript_include_tag('notes') %>
<%= javascript_include_tag('textbox_expand') %>
<script> var comments_length = <%= @node.comments.length %>; </script>
<script> var comments_length = <%= @node.comments.length %>; $(function(){
$("img").lazyload();
});</script>
<%= javascript_include_tag('question') %>
<div class="col-lg-9 note-show question-show">

<% if @node.main_image %>
<a class="main-image" style="max-width:100%;width:800px;" href="<%= @node.main_image.path(:original) %>">
<img style="max-height:600px;" class="rounded d-none d-lg-inline" src="<%= @node.main_image.path(:large) %>" />
<img style="width:100%;" class="rounded d-lg-none" src="<%= @node.main_image.path(:large) %>" />
<%= image_tag(@node.main_image.path(:large), class:"rounded d-none d-lg-inline", style:'max-height:600px;max-width:100%;') %>
<%= image_tag(@node.main_image.path(:large), class:"rounded d-lg-none", style:'width:100%;') %>
</a>
<% end %>

Expand Down
9 changes: 7 additions & 2 deletions app/views/users/list.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<div class="row card-body">
<div class="col-lg-12 nopadding">
<div class="profile-image text-center">
<img class="rounded-circle img-fluid" id="profile-photo" src="<%= user.profile_image %>" />
<%= image_tag(user.profile_image, class:'rounded-circle img-fluid', id:'profile-photo') %>
</div>
</div>
<div class="col-lg-12 nopadding">
Expand Down Expand Up @@ -169,12 +169,17 @@ h3, h2, h5 {
padding-bottom: 5px;
padding-left: 14px;
}

.sort-style{
color: #808080;
}
.selected{
text-decoration: underline;
}
</style>
<script>
$(function(){
$("img").lazyload();
});
</script>
<%= render partial: 'tag/location' %>
19 changes: 11 additions & 8 deletions app/views/users/profile.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
<%= render :partial => "map/leaflet" , locals: { lat: @map_lat, lon: @map_lon, zoom: @map_zoom, blurred: @map_blurred, topmap: true, user: @profile_user } %>
<% elsif !current_user.nil? && current_user.id == @profile_user.id %>
<div id="map_template" style="position: relative; width: 100%; display: inline-block;">
<img src="https://a.tiles.mapbox.com/v3/jywarren.map-lmrwb2em/0/0/0.png" style="height:300px; width: 100%; margin: 0; position: relative; margin-right: -10px;">
<img src="https://a.tiles.mapbox.com/v3/jywarren.map-lmrwb2em/0/0/0.png" style="height:300px; width: 100%; margin: 0; position: relative; margin-right: -10px;">
<button type='button' class='blurred-location-input btn btn-default btn-lg' onclick='addLocation()' style="position: absolute; position: absolute;top: 41% ; left: 15% ;"> <strong> Share your Location </strong> </button>
<p><i><small>Learn about <a href='https://publiclab.org/wiki/location-privacy'>privacy</a> </small></i></p>
</div>
</div>
<% end %>

<br />
Expand Down Expand Up @@ -45,14 +45,14 @@

<div class="offset-md-1 order-first order-md-last col-md-4 text-center">
<div class="text-center">
<%if @content_approved or (!current_user.nil? && current_user.id == @profile_user.id)%>
<img class="d-none d-lg-inline rounded-circle" id="profile-photo" style="width:50%;margin-bottom:10px;" src="<%= @profile_user.profile_image %>" />
<%if @content_approved or (!current_user.nil? && current_user.id == @profile_user.id)%>
<%= image_tag(@profile_user.profile_image, class:'d-none d-lg-inline rounded-circle', id:'profile-photo', style:"width:50%;margin-bottom:10px;") %>
<%end%>
<h4 class="mt-3"><strong>@<%= @profile_user.name %> <%= @profile_user.new_contributor %></strong></h4>
</div>
<div style="text-align:center;" class="d-lg-none">
<%if @content_approved or (!current_user.nil? && current_user.id == @profile_user.id)%>
<img class="rounded-circle" id="profile-photo" style="width:50%;margin-bottom:20px;" src="<%= @profile_user.profile_image(:medium) %>" />
<%if @content_approved or (!current_user.nil? && current_user.id == @profile_user.id)%>
<%= image_tag(@profile_user.profile_image(:medium), class:'rounded-circle', id:'profile-photo', style:"width:50%;margin-bottom:20px;") %>
<%end%>
<h4 class="mt-3"><strong>@<%= @profile_user.name %> <%= @profile_user.new_contributor %></strong></h4>
</div>
Expand All @@ -66,9 +66,9 @@
<% if @profile_user.status == 0 %> | <i class="fa fa-ban" style="color:#a00;"></i> <%= translation('users.profile.banned') %><% end %>
</small>
</h4>
<%if @content_approved or (!current_user.nil? && current_user.id == @profile_user.id)%>
<%if @content_approved or (!current_user.nil? && current_user.id == @profile_user.id)%>
<p><%= raw auto_link(RDiscount.new(@profile_user.bio || '').to_html, :sanitize => false) %></p>
<%end%>
<%end%>
<div class="mt-3">

<a class="btn btn-outline-secondary" id="tags-section">Tags</a>
Expand Down Expand Up @@ -273,6 +273,9 @@
<script src = "https://cdn.jsdelivr.net/npm/[email protected]/dist/frappe-charts.min.iife.js"></script>

<script type = "text/javascript">
$(function(){
$("img").lazyload();
});
//Upon clicking copy button, copyToken function is activated.
$('#copy-btn').on('click', copyToken);

Expand Down
2 changes: 1 addition & 1 deletion app/views/wiki/_header.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<% if @node.main_image && !@presentation %>
<a style="margin-bottom:10px;" href="<%= @node.main_image.path(:original) %>">
<img style="max-width:100%;max-height:600px;margin-bottom:10px;" class="rounded" src="<%= @node.main_image.path(:large) %>" />
<%= image_tag(@node.main_image.path(:large), class:"rounded", style:'max-width:100%;max-height:600px;margin-bottom:10px;') %>
</a>
<% end %>

Expand Down
3 changes: 2 additions & 1 deletion app/views/wiki/show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<% if @presentation && @node.main_image%>
<div id="main-image-presentation" style="margin-bottom:8px;">
<a href="<%= @node.main_image.path(:original) %>">
<img style="width: 100vw;margin: 0 calc((100% / 2) - 50vw);margin-top: -21px;margin-bottom: 12px;" class="rounded" src="<%= @node.main_image.path(:original) %>" />
<%= image_tag(@node.main_image.path(:original), class:"rounded", style:'width: 100vw;margin: 0 calc((100% / 2) - 50vw);margin-top: -21px;margin-bottom: 12px;') %>
</a>
</div>
<% end %>
Expand Down Expand Up @@ -64,6 +64,7 @@

<script>
(function(){
$("img").lazyload();

<% if @fancy %>
$('#content img').addClass('img-rounded');
Expand Down
9 changes: 9 additions & 0 deletions config/initializers/lazyload.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Lazyload::Rails.configure do |config|
# By default, a 1x1 grey gif is used as placeholder ("data:image/gif;base64,...").
# This can be easily customized:
# config.placeholder = "/public/img/grey.gif"

# image_tag can return lazyload-friendly images by default,
# no need to pass the { lazy: true } option
config.lazy_by_default = true
end
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@
"typeahead.js": "^0.11.1",
"typeahead.js-browserify": "Javier-Rotelli/typeahead.js-browserify#~1.0.7",
"urlhash": "^0.1.3",
"woofmark": "~4.2.0"
"woofmark": "~4.2.0",
"jquery-lazyload": "1.9.7"
},
"devDependencies": {},
"homepage": "https://github.com/publiclab/plots2",
Expand Down