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

Multiple-image-export #693

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+88 −10
Open

Multiple-image-export #693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cadc9b3
Export improved
divyabaid16 Jun 24, 2019
e9fc4dc
Changes made
divyabaid16 Jun 25, 2019
d52d6fb
Changes in unit test made
divyabaid16 Jun 25, 2019
a72d22b
Test passing
divyabaid16 Jun 26, 2019
ae11bae
Opacity to uplaced images added
divyabaid16 Jul 4, 2019
3582af8
Error resolved
divyabaid16 Jul 10, 2019
d7e55ef
Rubocop fix
divyabaid16 Jul 10, 2019
e0f73c8
Rubocop fix
divyabaid16 Jul 10, 2019
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
6 changes: 5 additions & 1 deletion app/controllers/maps_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,13 @@ def images

# run the export
def export
Export.create!(map_id: params[:id], bands_string: 'default bands_string')
warpable_ids = params[:id].split(',')
warpable_ids.shift
@map = Map.find_by(id: params[:id])
# puts @map
if logged_in? || Rails.env.development? || verify_recaptcha(model: @map, message: "ReCAPTCHA thinks you're not a human!")
render plain: @map.run_export(current_user, params[:resolution].to_f)
render plain: @map.run_export(warpable_ids, current_user, params[:resolution].to_f)
else
render plain: 'You must be logged in to export, unless the map is anonymous.'
end
Expand Down
11 changes: 8 additions & 3 deletions app/models/map.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,14 @@ def grouped_images_histogram(binsize)
end

# we'll eventually replace this with a JavaScript call to initiate an external export process:
def run_export(user, resolution)
def run_export(warpable_ids, user, resolution)
key = APP_CONFIG ? APP_CONFIG["google_maps_api_key"] : "AIzaSyAOLUQngEmJv0_zcG1xkGq-CXIPpLQY8iQ"

warpables = if warpable_ids == ''
placed_warpables
else
Warpable.find(warpable_ids)
end
# warpables = Warpable.find(warpable_ids) || placed_warpables
new_export = Export.new(map_id: id) unless export

Exporter.run_export(user,
Expand All @@ -246,7 +251,7 @@ def run_export(user, resolution)
slug,
Rails.root.to_s,
average_scale,
placed_warpables,
warpables,
key)
end

Expand Down
77 changes: 73 additions & 4 deletions app/views/maps/_sidebar_exports.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,41 @@
<% end %>

<% if logged_in? || @map.anonymous? %>
<div>
<% if @map.warpables.empty? %>
<p id="no-images">No images are uploaded</p>
<% else %>
<p>Select the images to be exported:</p>
<%end%>
<% @map.warpables.each do |warpable| %>
<div style="display: inline-grid;" id="warpable-<%= warpable.id %>">
<% if warpable.placed? %>
<p class="warpable export-warpable" style="margin-right: 20px;">
<% else %>
<p class="warpable" rel="tooltip" title="This image has not yet been placed on the map." style="margin-right: 20px; opacity: 0.5; cursor: not-allowed;">
<% end %>
<%= image_tag warpable.image.url(:thumb),
:'data-warpable-id' => warpable.id,
:'data-map-id' => @map.id,
:'id' => "warpable-img-#{warpable.id}"
%>
</p>
</div>
<% end %>
</div>

<div class="progress"<% unless @map.exporting? %> style="display:none;"<% end %>>
<div class="progress-bar progress-bar-striped progress-bar-animated active" role="progressbar" aria-valuenow="100" aria-valuemin="0" aria-valuemax="100" style="width: 100%">
<span class="" id="export_progress"><%= @map.export ? @map.export.status : "Loading..." %></span>
</div>
</div>

<div style="margin-top: 30px;">
<p>
<button id="begin-export" class="btn btn-lg btn-primary"<% if @map.export && @map.exporting? %> style="display:none;"<% end %>>Start export</button>
<button <% unless @map.export && @map.exporting? %>style="display:none;"<% end %> id="cancel-export" class="btn btn-lg btn-outline-secondary"><i class="fas fa-times"></i> Cancel export</button>
</p>

</div>
<% if @map.exporting? && ((@map.export.updated_at-DateTime.now)/1.hours.to_i).abs > 4 %>
<p class="alert alert-warning alert-toolong">
This export is taking a while; consider cancelling it and following the tips linked to below.
Expand Down Expand Up @@ -81,6 +111,35 @@
</div>
</div>
<script>
var exp1 = 0;
// //var arr = [];
// var b = $('.export-warpable').length;
// console.log(b);
// var k = [0, 0, 0];
// console.log(k);
$('.export-warpable').click(function() {
var warpable_id = $(this).children('img').attr("data-warpable-id");
console.log(warpable_id);
if(exp1==0){
$(this).css('border', "solid 2px red");
$(this).parent().addClass('export-image');
exp1 = 1;
}
else{
$(this).css('border', "none");
$(this).parent().removeClass('export-image');
exp1 = 0;
}
});
Comment on lines +114 to +133
Copy link
Member

Choose a reason for hiding this comment

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

Just got around to reviewing this! So this code assumes that you're only toggling the border on one image. If you select 1, and then select a 2nd, the 2nd won't get a border unless you click it twice. You could refactor maybe to something like this:

$('.export-warpable').click(function() {
    if ($(this).css('border')[0] === '0') {
      $(this).css('border', 'solid 2px red');
      $(this).parent().addClass('export-image');
    }
    else {
      $(this).css('border', 'none');
      $(this).parent().removeClass('export-image');
    }
});
  • mind the double vs single quotes and { spacing
  • but better to move the css logic under the export-image class in css, and then all you have to do is toggle the class
  • remove console.logs and comments

function check_export(){
if($('.export-image').length > 0){
$('#begin-export').attr('disabled',false).attr('title','Start Export');
}
else{
$('#begin-export').attr('disabled',true).attr('title','Select images to export');
}
};

Comment on lines +134 to +142
Copy link
Member

Choose a reason for hiding this comment

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

This function is not being called anywhere, so doesn't work. Also disabled would be a prop and to change the text use html('text')

var exporting, exportResolutionSlider
(function(){
exportResolutionSlider = $("#export-resolution").ionRangeSlider({
Expand Down Expand Up @@ -142,7 +201,16 @@ var exporting, exportResolutionSlider
})

$('#begin-export').click(function() {
if (images.length < 30 || confirm('This map has over 30 images; this is a lot! Please read the tips on exporting maps listed below the Export button.')) {
var arr = []
//var a = $('.export-image').length;
//console.log(a);
$('.export-image').each(function(i,obj){
var warpable_id = $(this).children('p').children('img').attr("data-warpable-id");
arr.push(warpable_id);
})
//console.log(arr);
//var arr = [101,102,103]
if (1 || confirm('This map has over 30 images; this is a lot! Please read the tips on exporting maps listed below the Export button.')) {
$('.alert-toolong').hide()
$('.progress').show()
$('.progress-bar').addClass('active')
Expand All @@ -159,8 +227,9 @@ var exporting, exportResolutionSlider
$('#export_progress').html("starting export")
$('.map-exports').html('')
$.ajax({
url: '/maps/export/<%= @map.id %>?authenticity_token=<%= form_authenticity_token %>',
method: 'POST',
//url: '/maps/export/<%= @map.id %>,101,102,103,104,105',
url: '/maps/export/<%= @map.id %>,'+arr,
method: 'GET',
data: {
<% if Rails.env.production? && !logged_in? && @map.anonymous? %>
'g-recaptcha-response': grecaptcha.getResponse(),
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
get 'map/:id', to: redirect('/maps/%{id}')
get 'embed/:id', to: 'maps#embed'
post 'maps/:map_id/warpables', to: 'images#create' # deprecate this in favor of resourceful route below; this is just to override maps/:id
post 'maps/export/:id', to: 'maps#export'
get 'maps/export', to: 'maps#export'
post 'maps/:id', to: 'maps#export'

get 'import/:name', to: 'images#import' # this was for auto-adding images via URL
Expand Down
2 changes: 1 addition & 1 deletion test/models/map_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class MapTest < ActiveSupport::TestCase
assert_equal 0, village.average_cm_per_pixel

resolution = 20
assert_not_nil map.run_export(users(:quentin), resolution) #map.average_cm_per_pixel)
assert_not_nil map.run_export('', users(:quentin), resolution) #map.average_cm_per_pixel)

# main issue will be that it creates and continuously updates an Export model.
# we could shift this to a polling model, either on the client side (eliminating the Export model)
Expand Down