L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-62411 qtype_ddmarker: fix issues found during review

Browse Source 
* Editing the coordinate string now correctly updates the image only if
  the coordinates are valid.
* ... and does the right thing if the number of handles has changed.
* Dragging the shape handles is now constrained so that you cannot
  create a shape that goes beyond the edge of the image.
* The PHP validation that the shape is within the imagehas also been
  fixed.
* For Mac users, CMD+Click duplicates the handle like CTRL+Click.
* Ensure the shape being edited is always on top of the otehrs.
This commit is contained in:
Tim Hunt
2018-10-22 18:40:02 +01:00
parent ed7e30fa5c
commit 00f09d8f5c
6 changed files with 155 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
question/type/ddmarker/amd/build/form.min.js
Vendored
+1 -1
View File
File diff suppressed because one or more lines are too long
question/type/ddmarker/amd/build/shapes.min.js
Vendored
+1 -1
View File
File diff suppressed because one or more lines are too long
question/type/ddmarker/amd/src/form.js
+43 -9
View File
@@ -42,13 +42,32 @@ define(['jquery', 'core/dragdrop', 'qtype_ddmarker/shapes'], function($, dragDro
/**
* Update the coordinates from a particular string.
*
* @param {SVGElement} [svg] the SVG element that is the preview.
*/
DropZoneManager.prototype.updateCoordinatesFromForm = function() {
var coordinates = this.getCoordinates();
DropZoneManager.prototype.updateCoordinatesFromForm = function(svg) {
var coordinates = this.getCoordinates(),
currentNumPoints = this.shape.getType() === 'polygon' && this.shape.points.length;
if (this.shape.getCoordinates() === coordinates) {
return;
}
if (this.shape.parse(coordinates)) {
if (!this.shape.parse(coordinates)) {
// Invalid coordinates. Don't update the preview.
return;
}
if (this.shape.getType() === 'polygon' && currentNumPoints !== this.shape.points.length) {
// Polygon, and size has changed.
var currentyActive = this.isActive();
this.removeFromSvg();
if (svg) {
this.addToSvg(svg);
if (currentyActive) {
this.setActive();
}
}
} else {
// Simple update.
this.updateSvgEl();
}
};
@@ -165,6 +184,8 @@ define(['jquery', 'core/dragdrop', 'qtype_ddmarker/shapes'], function($, dragDro
}
// Move handle.
// The shape + its label are the first two children of svgEl.
// Then come the move handle followed by the edit handles.
this.svgEl.childNodes[2].setAttribute('cx', handles.moveHandle.x);
this.svgEl.childNodes[2].setAttribute('cy', handles.moveHandle.y);
@@ -188,6 +209,11 @@ define(['jquery', 'core/dragdrop', 'qtype_ddmarker/shapes'], function($, dragDro
* Set this drop zone as being edited.
*/
DropZoneManager.prototype.setActive = function() {
// Move this one to last, so that it is always on top.
// (Otherwise the handles may not be able to receive events.)
var parent = this.svgEl.parentNode;
parent.removeChild(this.svgEl);
parent.appendChild(this.svgEl);
this.svgEl.setAttribute('class', this.svgEl.getAttribute('class') + ' active');
};
@@ -244,9 +270,13 @@ define(['jquery', 'core/dragdrop', 'qtype_ddmarker/shapes'], function($, dragDro
var movingDropZone = this,
lastX = info.x,
lastY = info.y,
dragProxy = this.makeDragProxy(info.x, info.y);
dragProxy = this.makeDragProxy(info.x, info.y),
bgImg = $('fieldset#id_previewareaheader .dropbackground'),
maxX = bgImg.width(),
maxY = bgImg.height();
dragDrop.start(e, $(dragProxy), function(pageX, pageY) {
movingDropZone.shape.move(pageX - lastX, pageY - lastY);
movingDropZone.shape.move(pageX - lastX, pageY - lastY, maxX, maxY);
lastX = pageX;
lastY = pageY;
movingDropZone.updateSvgEl();
@@ -269,7 +299,7 @@ define(['jquery', 'core/dragdrop', 'qtype_ddmarker/shapes'], function($, dragDro
}
// For polygons, CTRL + drag adds a new point.
if (this.shape.getType() === 'polygon' && e.ctrlKey) {
if (this.shape.getType() === 'polygon' && (e.ctrlKey || e.metaKey)) {
this.shape.addNewPointAfter(handleIndex);
this.removeFromSvg();
this.addToSvg(svg);
@@ -279,9 +309,13 @@ define(['jquery', 'core/dragdrop', 'qtype_ddmarker/shapes'], function($, dragDro
var changingDropZone = this,
lastX = info.x,
lastY = info.y,
dragProxy = this.makeDragProxy(info.x, info.y);
dragProxy = this.makeDragProxy(info.x, info.y),
bgImg = $('fieldset#id_previewareaheader .dropbackground'),
maxX = bgImg.width(),
maxY = bgImg.height();
dragDrop.start(e, $(dragProxy), function(pageX, pageY) {
changingDropZone.shape.edit(handleIndex, pageX - lastX, pageY - lastY);
changingDropZone.shape.edit(handleIndex, pageX - lastX, pageY - lastY, maxX, maxY);
lastX = pageX;
lastY = pageY;
changingDropZone.updateSvgEl();
@@ -473,7 +507,7 @@ define(['jquery', 'core/dragdrop', 'qtype_ddmarker/shapes'], function($, dragDro
break;
case 'coords':
dropZone.updateCoordinatesFromForm();
dropZone.updateCoordinatesFromForm(dragDropForm.form.getSvg());
break;
case 'choice':
question/type/ddmarker/amd/src/shapes.js
+104 -11
View File
@@ -138,9 +138,11 @@ define(function() {
*
* @param {int} dx x offset.
* @param {int} dy y offset.
* @param {int} maxX ensure that after editing, the shape lies between 0 and maxX on the x-axis.
* @param {int} maxY ensure that after editing, the shape lies between 0 and maxX on the y-axis.
*/
Shape.prototype.move = function(dx, dy) {
this.centre.move(dx, dy);
Shape.prototype.move = function(dx, dy, maxX, maxY) {
void (maxY);
};
/**
@@ -149,9 +151,11 @@ define(function() {
* @param {int} handleIndex which handle was moved.
* @param {int} dx x offset.
* @param {int} dy y offset.
* @param {int} maxX ensure that after editing, the shape lies between 0 and maxX on the x-axis.
* @param {int} maxY ensure that after editing, the shape lies between 0 and maxX on the y-axis.
*/
Shape.prototype.edit = function(handleIndex, dx, dy) {
void (dy);
Shape.prototype.edit = function(handleIndex, dx, dy, maxX, maxY) {
void (maxY);
};
/**
@@ -259,7 +263,7 @@ define(function() {
};
Circle.prototype.parse = function(coordinates) {
if (!coordinates.match(/\d+,\d+;\d+/)) {
if (!coordinates.match(/^\d+,\d+;\d+$/)) {
return false;
}
@@ -269,9 +273,31 @@ define(function() {
return true;
};
Circle.prototype.edit = function(handleIndex, dx, dy) {
Circle.prototype.move = function(dx, dy, maxX, maxY) {
this.centre.move(dx, dy);
if (this.centre.x < this.radius) {
this.centre.x = this.radius;
}
if (this.centre.x > maxX - this.radius) {
this.centre.x = maxX - this.radius;
}
if (this.centre.y < this.radius) {
this.centre.y = this.radius;
}
if (this.centre.y > maxY - this.radius) {
this.centre.y = maxY - this.radius;
}
};
Circle.prototype.edit = function(handleIndex, dx, dy, maxX, maxY) {
this.radius += dx;
void (dy);
var limit = Math.min(this.centre.x, this.centre.y, maxX - this.centre.x, maxY - this.centre.y);
if (this.radius > limit) {
this.radius = limit;
}
if (this.radius < -limit) {
this.radius = -limit;
}
};
/**
@@ -357,7 +383,7 @@ define(function() {
};
Rectangle.prototype.parse = function(coordinates) {
if (!coordinates.match(/\d+,\d+;\d+,\d+/)) {
if (!coordinates.match(/^\d+,\d+;\d+,\d+$/)) {
return false;
}
@@ -369,9 +395,37 @@ define(function() {
return true;
};
Rectangle.prototype.edit = function(handleIndex, dx, dy) {
Rectangle.prototype.move = function(dx, dy, maxX, maxY) {
this.centre.move(dx, dy);
if (this.centre.x < 0) {
this.centre.x = 0;
}
if (this.centre.x > maxX - this.width) {
this.centre.x = maxX - this.width;
}
if (this.centre.y < 0) {
this.centre.y = 0;
}
if (this.centre.y > maxY - this.height) {
this.centre.y = maxY - this.height;
}
};
Rectangle.prototype.edit = function(handleIndex, dx, dy, maxX, maxY) {
this.width += dx;
this.height += dy;
if (this.width < -this.centre.x) {
this.width = -this.centre.x;
}
if (this.width > maxX - this.centre.x) {
this.width = maxX - this.centre.x;
}
if (this.height < -this.centre.y) {
this.height = -this.centre.y;
}
if (this.height > maxY - this.centre.y) {
this.height = maxY - this.centre.y;
}
};
/**
@@ -452,7 +506,7 @@ define(function() {
};
Polygon.prototype.parse = function(coordinates) {
if (!coordinates.match(/(?:\d+,\d+)?(?:;\d+,\d+)*/)) {
if (!coordinates.match(/^\d+,\d+(?:;\d+,\d+)*$/)) {
return false;
}
@@ -470,8 +524,47 @@ define(function() {
return true;
};
Polygon.prototype.edit = function(handleIndex, dx, dy) {
Polygon.prototype.move = function(dx, dy, maxX, maxY) {
this.centre.move(dx, dy);
var bbXMin = maxX,
bbXMax = 0,
bbYMin = maxY,
bbYMax = 0;
// Computer centre.
for (var i = 0; i < this.points.length; i++) {
bbXMin = Math.min(bbXMin, this.points[i].x);
bbXMax = Math.max(bbXMax, this.points[i].x);
bbYMin = Math.min(bbYMin, this.points[i].y);
bbYMax = Math.max(bbYMax, this.points[i].y);
}
if (this.centre.x < -bbXMin) {
this.centre.x = -bbXMin;
}
if (this.centre.x > maxX - bbXMax) {
this.centre.x = maxX - bbXMax;
}
if (this.centre.y < -bbYMin) {
this.centre.y = -bbYMin;
}
if (this.centre.y > maxY - bbYMax) {
this.centre.y = maxY - bbYMax;
}
};
Polygon.prototype.edit = function(handleIndex, dx, dy, maxX, maxY) {
this.points[handleIndex].move(dx, dy);
if (this.points[handleIndex].x < -this.centre.x) {
this.points[handleIndex].x = -this.centre.x;
}
if (this.points[handleIndex].x > maxX - this.centre.x) {
this.points[handleIndex].x = maxX - this.centre.x;
}
if (this.points[handleIndex].y < -this.centre.y) {
this.points[handleIndex].y = -this.centre.y;
}
if (this.points[handleIndex].y > maxY - this.centre.y) {
this.points[handleIndex].y = maxY - this.centre.y;
}
};
/**
question/type/ddmarker/lang/en/qtype_ddmarker.php
+1 -1
View File
@@ -42,7 +42,7 @@ First selecting a shape (circle, rectangle or polygon) will add a new drop zone
Editing a shape starts with a click on the shape in the preview to show the editing handles. You can move the shape using the center handle, or adjust the shape\'s dimensions with the vertex handles.
For polygons only, holding the control button while clicking on a vertex handle will add a new vertex to the polygon. Please keep a polygon shape as simple as possible, without crossing lines.
For polygons only, holding the control button (command button on a Mac) while clicking on a vertex handle will add a new vertex to the polygon. Please keep a polygon shape as simple as possible, without crossing lines.
For information the three shapes use coordinates in this way:<br />
* Circle: centre_x, centre_y; radius<br />for example: <code>80,100;50</code><br />
question/type/ddmarker/shapes.php
+5 -3
View File
@@ -42,7 +42,8 @@ abstract class qtype_ddmarker_shape {
}
public function inside_width_height($widthheight) {
foreach ($this->outlying_coords_to_test() as $coordsxy) {
if ($coordsxy[0] > $widthheight[0] || $coordsxy[1] > $widthheight[1]) {
if ($coordsxy[0] < 0 || $coordsxy[0] > $widthheight[0] ||
$coordsxy[1] < 0 || $coordsxy[1] > $widthheight[1]) {
return false;
}
}
@@ -236,7 +237,7 @@ class qtype_ddmarker_shape_rectangle extends qtype_ddmarker_shape {
}
protected function outlying_coords_to_test() {
return array($this->xleft + $this->width, $this->ytop + $this->height);
return [[$this->xleft, $this->ytop], [$this->xleft + $this->width, $this->ytop + $this->height]];
}
public function is_point_in_shape($xy) {
return $this->is_point_in_bounding_box($xy, array($this->xleft, $this->ytop),
@@ -296,7 +297,8 @@ class qtype_ddmarker_shape_circle extends qtype_ddmarker_shape {
}
protected function outlying_coords_to_test() {
return array($this->xcentre + $this->radius, $this->ycentre + $this->radius);
return [[$this->xcentre - $this->radius, $this->ycentre - $this->radius],
[$this->xcentre + $this->radius, $this->ycentre + $this->radius]];
}
public function is_point_in_shape($xy) {