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.

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

Synch editing add ons #957

Merged
merged 4 commits into from
Aug 23, 2019
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: 8 additions & 3 deletions app/assets/javascripts/channels/concurrent_editing.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
/* Handles all the frontend interactions with action cable and the server. */

App.concurrent_editing = App.cable.subscriptions.create("ConcurrentEditingChannel", {
App.concurrent_editing = App.cable.subscriptions.create(
{
channel: "ConcurrentEditingChannel",
mapSlug: window.location.href.split("/").pop()
}, {
connected: function() {
// Called when the subscription is ready for use on the server
},
Expand All @@ -11,7 +15,7 @@ App.concurrent_editing = App.cable.subscriptions.create("ConcurrentEditingChanne

received: function(data) {
// Called when there's incoming data on the websocket for this channel
window.mapKnitter.synchronizeData(data.changes);
window.mapknitter.synchronizeData(data.changes);
},

speak: function(changes) {
Expand All @@ -20,7 +24,8 @@ App.concurrent_editing = App.cable.subscriptions.create("ConcurrentEditingChanne
* which is responsible for broadcasting the updated warpables
* to all the user's connected to the concurrent_editing channel. */
return this.perform("sync", {
changes: changes
changes: changes,
map_slug: window.location.href.split("/").pop()
});
}
});
123 changes: 119 additions & 4 deletions app/assets/javascripts/mapknitter/Map.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,11 +405,123 @@ MapKnitter.Map = MapKnitter.Class.extend({
corners [2] = y;
corners [3] = x;

console.log(corners);

layer = layers.filter(l => l._url==warpable.srcmedium)[0];
layer.setCorners(corners);

if(layer == null || layer == undefined) {
window.mapknitter.synchronizeNewAddedImage(warpable);
} else {
layer.setCorners(corners);
var index = layers.indexOf(layer);
if (index > -1) {
layers.splice(index, 1);
}
}
});

// remove images if deleted from any user's browser
layers.forEach(function(layer) {
edit = layer.editing
edit._removeToolbar();
edit.disable();
// remove from Leaflet map:
map.removeLayer(layer);
// remove from sidebar too:
$('#warpable-' + layer.warpable_id).remove();
});
},

synchronizeNewAddedImage: function(warpable) {
Copy link

Choose a reason for hiding this comment

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

Function synchronizeNewAddedImage has 69 lines of code (exceeds 25 allowed). Consider refactoring.

var wn = warpable.nodes;
bounds = [];

// only already-placed images:
if (wn.length > 0) {
var downloadEl = $('.img-download-' + warpable.id),
imgEl = $('#full-img-' + warpable.id);

downloadEl.click(function () {
Copy link

Choose a reason for hiding this comment

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

Identical blocks of code found in 2 locations. Consider refactoring.

downloadEl.html('<i class="fa fa-circle-o-notch fa-spin"></i>');

imgEl[0].onload = function () {
var height = imgEl.height(),
width = imgEl.width(),
nw = map.latLngToContainerPoint(wn[0]),
ne = map.latLngToContainerPoint(wn[1]),
se = map.latLngToContainerPoint(wn[2]),
sw = map.latLngToContainerPoint(wn[3]),
offsetX = nw.x,
offsetY = nw.y,
displayedWidth = $('#warpable-img-' + warpable.id).width(),
ratio = width / displayedWidth;

nw.x -= offsetX;
ne.x -= offsetX;
se.x -= offsetX;
sw.x -= offsetX;

nw.y -= offsetY;
ne.y -= offsetY;
se.y -= offsetY;
sw.y -= offsetY;

warpWebGl(
'full-img-' + warpable.id,
[0, 0, width, 0, width, height, 0, height],
[nw.x, nw.y, ne.x, ne.y, se.x, se.y, sw.x, sw.y],
true // trigger download
)

downloadEl.html('<i class="fa fa-download"></i>');
}

imgEl[0].src = $('.img-download-' + warpable.id).attr('data-image');
});

var corners = [
Copy link

Choose a reason for hiding this comment

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

Identical blocks of code found in 2 locations. Consider refactoring.

L.latLng(wn[0].lat, wn[0].lon),
L.latLng(wn[1].lat, wn[1].lon),
L.latLng(wn[3].lat, wn[3].lon),
L.latLng(wn[2].lat, wn[2].lon)
];

var img = L.distortableImageOverlay(warpable.srcmedium, {
corners: corners,
mode: 'lock'
}).addTo(map);

var customExports = mapknitter.customExportAction();
var imgGroup = L.distortableCollection({
actions: [customExports]
}).addTo(map);

imgGroup.addLayer(img);

/**
* TODO: toolbar may still appear outside of frame. Create a getter for toolbar corners in LDI and then include them in this calculation
*/
bounds = bounds.concat(corners);
var newImgBounds = L.latLngBounds(corners);

if (!map._initialBounds.contains(newImgBounds) && !map._initialBounds.equals(newImgBounds)) {
Copy link

Choose a reason for hiding this comment

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

Identical blocks of code found in 2 locations. Consider refactoring.

map._initialBounds.extend(newImgBounds);
mapknitter._map.flyToBounds(map._initialBounds);
}

images.push(img);
img.warpable_id = warpable.id;

if (!mapknitter.readOnly) {
Copy link

Choose a reason for hiding this comment

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

Identical blocks of code found in 2 locations. Consider refactoring.

L.DomEvent.on(img._image, {
click: mapknitter.selectImage,
dblclick: mapknitter.dblClickImage,
load: mapknitter.setupToolbar
}, img);

L.DomEvent.on(imgGroup, 'layeradd', mapknitter.setupEvents, img);
}

img.editing.disable()
}
},

saveImageIfChanged: function () {
Expand All @@ -434,7 +546,7 @@ MapKnitter.Map = MapKnitter.Class.extend({
saveImage: function () {
var img = this;
img._corner_state = JSON.stringify(img._corners); // reset change state string:
$.ajax('/images', { // send save request
$.ajax('/images/'+img.warpable_id, { // send save request
type: 'PATCH',
data: {
warpable_id: img.warpable_id,
Expand Down Expand Up @@ -474,6 +586,9 @@ MapKnitter.Map = MapKnitter.Class.extend({
beforeSend: function (e) {
$('.mk-save').removeClass('fa-check-circle fa-times-circle fa-green fa-red').addClass('fa-spinner fa-spin')
},
success: function(data) {
App.concurrent_editing.speak(data);
},
complete: function (e) {
$('.mk-save').removeClass('fa-spinner fa-spin').addClass('fa-check-circle fa-green')
// disable interactivity:
Expand Down
4 changes: 2 additions & 2 deletions app/channels/concurrent_editing_channel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class ConcurrentEditingChannel < ApplicationCable::Channel

def subscribed
# Called first to connect user to the channel.
stream_from "concurrent_editing_channel"
stream_from "concurrent_editing_channel:#{params[:mapSlug]}"
end

def unsubscribed
Expand All @@ -12,6 +12,6 @@ def unsubscribed

def sync(changes)
# Responsible for broadcasting the updated warpables or simply images to the user's connected on this channel.
ActionCable.server.broadcast 'concurrent_editing_channel', changes
ActionCable.server.broadcast "concurrent_editing_channel:#{changes['map_slug']}", changes
end
end
7 changes: 2 additions & 5 deletions app/controllers/images_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,7 @@ def update
@warpable.locked = params[:locked]
@warpable.cm_per_pixel = @warpable.get_cm_per_pixel
@warpable.save
respond_to do |format|
format.html { render html: 'success' }
format.json { render json: @warpable.map.fetch_map_data }
end
render json: @warpable.map.fetch_map_data
Copy link
Member

Choose a reason for hiding this comment

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

I solved this same thing. I guess it came from a merge request.

else
render plain: 'You must be logged in to update the image, unless the map is anonymous.'
end
Expand All @@ -114,7 +111,7 @@ def destroy
@warpable.destroy
respond_to do |format|
format.html { redirect_to @warpable.map }
format.json { render json: @warpable }
format.json { render json: @warpable.map.fetch_map_data }
end
else
flash[:error] = 'You must be logged in to delete images.'
Expand Down
2 changes: 1 addition & 1 deletion test/controllers/images_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def fetch_in_production
points = "-71.39,41.83:-71.39,41.83:-71.39,41.83:-71.39,41.83"
patch :update, params: { id: @map.id, warpable_id: @warp.id, locked: false, points: points}
assert_not_nil @warp.nodes
assert_equal "text/html", response.content_type
assert_equal "application/json", response.content_type
assert_response :success
end

Expand Down