Skip to content

Commit

Permalink
Added feedback [HFP-1044]
Browse files Browse the repository at this point in the history
  • Loading branch information
fnoks committed Jun 27, 2017
1 parent d1b01b1 commit 149a908
Show file tree
Hide file tree
Showing 7 changed files with 164 additions and 10 deletions.
6 changes: 3 additions & 3 deletions library.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
"description": "Drag and drop the elements into the correct drop zones.",
"contentType": "Question",
"majorVersion": 1,
"minorVersion": 10,
"patchVersion": 1,
"minorVersion": 11,
"patchVersion": 0,
"embedTypes": [
"iframe"
],
Expand Down Expand Up @@ -53,7 +53,7 @@
{
"machineName": "H5PEditor.DragQuestion",
"majorVersion": 1,
"minorVersion": 8
"minorVersion": 9
}
]
}
28 changes: 26 additions & 2 deletions semantics.json
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,9 @@
"optional": true
},
{
"name": "tip",
"name": "tipsAndFeedback",
"type": "group",
"label": "Tip",
"label": "Tips and feedback",
"importance": "low",
"optional": true,
"fields": [
Expand All @@ -310,6 +310,22 @@
"em"
],
"optional": true
},
{
"name": "feedbackOnCorrect",
"type": "text",
"label": "Message displayed on correct match",
"importance": "low",
"description": "Message will appear below the task on \"check\" if correct droppable is matched.",
"optional": true
},
{
"name": "feedbackOnIncorrect",
"type": "text",
"label": "Message displayed on incorrect match",
"importance": "low",
"description": "Message will appear below the task on \"check\" if the match is incorrect.",
"optional": true
}
]
},
Expand Down Expand Up @@ -474,5 +490,13 @@
"importance": "low",
"default": "Wrong answer",
"common": true
},
{
"name": "feedbackHeader",
"type": "text",
"label": "Header for panel containing feedback for correct/incorrect answers",
"importance": "low",
"default": "Feedback",
"common": true
}
]
75 changes: 73 additions & 2 deletions src/drag-question.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import DragUtils from './drag-utils';
import DropZone from './dropzone';
import Draggable from './draggable';

var $ = H5P.jQuery;
const $ = H5P.jQuery;

/**
* Constructor
Expand Down Expand Up @@ -36,6 +36,7 @@ function C(options, contentId, contentData) {
tipAvailable: 'Tip available',
correctAnswer: 'Correct answer',
wrongAnswer: 'Wrong answer',
feedbackHeader: 'Feedback',
question: {
settings: {
questionTitle: 'Drag and drop',
Expand Down Expand Up @@ -516,13 +517,83 @@ C.prototype.addSolutionButton = function () {
that.answered = true;
that.showAllSolutions();
that.showScore();
that.addExplanation();
var xAPIEvent = that.createXAPIEventTemplate('answered');
that.addQuestionToXAPI(xAPIEvent);
that.addResponseToXAPI(xAPIEvent);
that.trigger(xAPIEvent);
});
};

/**
* Add explanation/feedback (the part on the bottom part)
*/
C.prototype.addExplanation = function () {
const task = this.options.question.task;
var self = this;

let explanations = [];

// Go through all dropzones, and find answers:
task.dropZones.forEach((dropZone, dropZoneId) => {
const feedback = {
correct: dropZone.tipsAndFeedback.feedbackOnCorrect,
incorrect: dropZone.tipsAndFeedback.feedbackOnIncorrect
};

// Don't run this code if feedback is not configured;

This comment has been minimized.

Copy link
@timothyylim

timothyylim Jun 27, 2017

Contributor

should this be:
feedback.correct === undefined && feedback.incorrect === undefined?

if (feedback.correct === undefined && feedback.correct === undefined) {
return;
}

// Index for correct draggables
const correctElements = dropZone.correctElements;

// Find all dragables placed on this dropzone:

This comment has been minimized.

Copy link
@icc

icc Jul 19, 2017

Member

Missing semicolon.

May I suggest the linter-jshint package for atom which will highlight such mistakes?

let placedDraggables = {}
this.draggables.forEach(draggable => {
draggable.elements.forEach(dz => {
if (dz.dropZone == dropZoneId) {
// Save reference to draggable, and mark it as correct/incorrect
placedDraggables[draggable.id] = {
instance: draggable,
correct: correctElements.indexOf("" + draggable.id) !== -1
}
}
});
});

// Go through each placed draggable
Object.keys(placedDraggables).forEach(draggableId => {
const draggable = placedDraggables[draggableId];
const draggableLabel = DragUtils.strip(draggable.instance.type.params.alt || draggable.instance.type.params.text) || '?';
const dropZoneLabel = DragUtils.strip(dropZone.label);

if (draggable.correct && feedback.correct) {
explanations.push({
correct: dropZoneLabel + ' + ' + draggableLabel,
text: feedback.correct
});

draggable.instance.setFeedback(feedback.correct, dropZoneId);
}
else if (!draggable.correct && feedback.incorrect) {
explanations.push({
correct: dropZoneLabel + ' + ',
wrong: draggableLabel,
text: feedback.incorrect
});

draggable.instance.setFeedback(feedback.incorrect, dropZoneId);
}
});
});

if (explanations.length !== 0) {
this.setExplanation(explanations, this.options.feedbackHeader);
}
};

/**
* Add retry button to our container.
*/
Expand Down Expand Up @@ -748,7 +819,7 @@ C.prototype.resetTask = function () {
this.showButton('check-answer');
this.hideButton('try-again');
this.setFeedback();

this.setExplanation();
};

/**
Expand Down
12 changes: 12 additions & 0 deletions src/drag-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,16 @@ export default class DragUtils {
});
DragUtils.setElementOpacity($element, backgroundOpacity);
}

/**
* Stripping away html tags
*
* @param {string} html
* @return {string}
*/
static strip(html) {
var tmp = document.createElement('div');
tmp.innerHTML = html;
return tmp.textContent || tmp.innerText || '';
}
}
22 changes: 21 additions & 1 deletion src/draggable.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import DragUtils from './drag-utils';

var $ = H5P.jQuery;
const $ = H5P.jQuery;

export default class Draggable extends H5P.EventDispatcher {
/**
Expand Down Expand Up @@ -206,6 +206,20 @@ export default class Draggable extends H5P.EventDispatcher {
self.trigger('elementadd', element.$[0]);
}

This comment has been minimized.

Copy link
@timothyylim

timothyylim Jun 27, 2017

Contributor

Comments would be nice here :)

setFeedback(feedback, dropZoneId) {
this.elements.forEach(element => {
if (element.dropZone === dropZoneId) {
if (element.$feedback === undefined) {
element.$feedback = $('<span>', {
'class': 'h5p-hidden-read',
appendTo: element.$
});
}
element.$feedback.text(feedback);

This comment has been minimized.

Copy link
@icc

icc Jul 19, 2017

Member

Please note that the parameters that all content types get in are always HTML. Never should you use these values for text(), .innerText or Text. If you absolutely want to use Text then you must convert it first. The quotesToChars() approach does not count. You should use the browser's built in parser, e.g.

var parser = document.createElement('div');
parser.innerHTML = paramters.valuethatiwantoconvert;
var textvalue = parser.innerText;

But this is best avoided unless you're doing something special. Mistaking the text for HTML raises major security concerns.

}
});
}

/**
* Determine if element should be copied when tragging, i.e. infinity instances.
*
Expand Down Expand Up @@ -330,6 +344,12 @@ export default class Draggable extends H5P.EventDispatcher {
var self = this;

this.elements.forEach(function (draggable) {

if (draggable.$feedback) {
draggable.$feedback.remove();
delete draggable.$feedback;
}

//If the draggable is in a dropzone reset its' position and feedback.
if (draggable.dropZone !== undefined) {
var element = draggable.$;
Expand Down
4 changes: 2 additions & 2 deletions src/dropzone.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import DragUtils from './drag-utils';

var $ = H5P.jQuery;
const $ = H5P.jQuery;

export default class DropZone {

Expand All @@ -25,7 +25,7 @@ export default class DropZone {
self.width = dropZone.width;
self.height = dropZone.height;
self.backgroundOpacity = dropZone.backgroundOpacity;
self.tip = dropZone.tip;
self.tip = dropZone.tipsAndFeedback.tip || '';
self.single = dropZone.single;
self.autoAlignEnabled = dropZone.autoAlign;
self.alignables = [];
Expand Down
27 changes: 27 additions & 0 deletions upgrades.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,33 @@ H5PUpgrades['H5P.DragQuestion'] = (function ($) {
}
}
}
finished(null, parameters);
},
/**
* For dropzones:
* Moving tip from a single element group to a group consisting of tip + feedback
*/
11: function (parameters, finished) {
if (parameters.question !== undefined &&
parameters.question.task !== undefined &&
parameters.question.task.dropZones !== undefined ) {

var dropZones = parameters.question.task.dropZones;
for (var i = 0; i < dropZones.length; i++) {
var dropZone = dropZones[i];
var tip = (dropZone !== undefined && dropZone.tip !== undefined && typeof dropZone.tip === 'string') ? dropZone.tip : '';

// Create the new group-structure
delete dropZone.tip;
dropZone.tipsAndFeedback = {
tip: tip,
feedbackOnCorrect: '',
feedbackOnIncorrect: ''
}

}
}

finished(null, parameters);
}
}
Expand Down

0 comments on commit 149a908

Please sign in to comment.