Issue #9 Added 'Incorrect feedback text' support #10

supriyarajgopal-spi · 2016-09-01T07:19:50Z

'Incorrect feedback text' field is added and showScore() is modified to show (in)correct feedback per drag-and-drop question.
Also made the 'common' setting as 'false' and changed the label for 'Feedback' field so that it appears on every drag-and-drop question instead of for the whole question set

'Incorrect feedback text' field is added and showScore() is modified to show (in)correct feedback per drag-and-drop question. Also made the 'common' setting as 'false' and changed the label for 'Feedback' field so that it appears on every drag-and-drop question instead of for the whole question set

thomasmars

Good work :)

Semantics for correct and incorrect feedback comes on top of the authoring tool, these should be moved to the bottom and probably placed inside a group

I have left a couple of comments that needs to be addressed before I can merge it in.

Note that when semantics are changed/added they must also be added to all language files (it is fine however to just add them in english).

Let me know if you want some help with implementing these changes!

thomasmars · 2016-09-27T09:05:37Z

js/dragquestion.js

   */
  C.prototype.showScore = function () {
    var maxScore = this.calculateMaxScore();
    if (this.options.behaviour.singlePoint) {
      maxScore = 1;
    }
-    var scoreText = this.options.feedback.replace('@score', this.points).replace('@total', maxScore);
+	var scoreText;


Please use two whitespaces instead of tabs, as described in H5P code style document.

thomasmars · 2016-09-27T09:07:36Z

semantics.json

-    "description": "Feedback text, variables available: @score and @total. Example: 'You got @score of @total points.'",
-    "common": true
+    "description": "Feedback text when correct answer is given, variables available: @score and @total. Example: 'You got @score of @total points.'",
+    "common": false


Semantic common fields default to "false", there is no need to specify this. You can remove the "common": false line.

thomasmars · 2016-09-27T09:08:53Z

semantics.json

+  },
+  {
+    "label": "Incorrect feedback text",
+    "name": "incorrect_feedback",


JavaScript variable should be in camelCase for consistency with other variables, i.e. change to "incorrectFeedback"

thomasmars · 2016-09-27T09:12:51Z

semantics.json

+    "label": "Incorrect feedback text",
+    "name": "incorrect_feedback",
+    "type": "text",
+    "default": "Sorry, this answer is incorrect",


I think default text should tell the score. I think it is fine to keep the default text that was here previously, and let authors customize this if they want. i.e. change to "You got @score of @ToTal points".

thomasmars · 2016-09-27T09:14:19Z

semantics.json

+    "description": "Feedback text when correct answer is given, variables available: @score and @total. Example: 'You got @score of @total points.'",
+    "common": false
+  },
+  {


New semantics must be added to all language files

thomasmars · 2016-09-27T09:17:54Z

js/dragquestion.js

-    var scoreText = this.options.feedback.replace('@score', this.points).replace('@total', maxScore);
+	var scoreText;
+	if(this.points == maxScore)
+		scoreText = this.options.feedback.replace('@score', this.points).replace('@total', maxScore);


Use H5P code style guidelines for if clauses, i.e. curly braces

thomasmars · 2016-09-27T09:21:49Z

js/dragquestion.js

+	if(this.points == maxScore)
+		scoreText = this.options.feedback.replace('@score', this.points).replace('@total', maxScore);
+	else
+		scoreText = this.options.incorrect_feedback.replace('@score', this.points).replace('@total', maxScore);


incorrect_feedback is undefined for old content. It needs a default value set in JS. see extended options

supriyarajgopal-spi · 2016-09-27T10:11:40Z

Hey @thomasmars ,

Thanks so much for the feedback! I will surely work on it whenever possible and update this thread.

- Indentation - Camel casing - Changes to semantics.json - Added default Incorrect Feedback text

supriyarajgopal-spi · 2016-10-24T11:24:15Z

Hi @thomasmars ,

I have implemented the changes to the same branch. Kindly reivew and merge.

thomasmars

Looks a lot better :)

When setting a feedback text and a user want to change it for both Correct feedback and Incorrect feedback the given solution is a bit awkward, because you must change both Correct feedback and Incorrect feedback fields, now if you imagine that an author wants to change the feedback for ALL "Drag-questions" within a course presentation, then he would have to go into all "drag-question" content types and change both of these specific fields for all of the content types.

Perhaps we could implement a compromise of a solution that generally uses a common field for all "drag-question" content types, but with the possibility of overriding these with a "correct feedback text" and an "incorrect feedback text", that would default to empty, and not used if empty. You can see an example of how this would work in the "true-false" content type semantics. What do you think ?

supriyarajgopal-spi · 2016-10-25T10:55:38Z

Right. So do you mean, @thomasmars , that we could keep the default common feedback field along with these specific Correct/Incorrect feedback fields.
And then, override the common feedback field with the specific Correct feedback field if set?
I think the Incorrect feedback field is better not to be considered for overriding.

Your thoughts?

fnoks · 2016-10-26T09:52:27Z

@supriyarajgopal-spi : As I understand you, your opinion is that there is no need for making it possible to create a specific feedback when answering incorrectly. I'm thinking there might be use cases where that would be a nice feature. Do you disagree?

supriyarajgopal-spi · 2016-10-26T10:01:17Z

No @fnoks , I meant:
Suppose we implement specific Correct/Incorrect feedback fields and the common,default feedback field as well.
Now, if an author sets all the above three fields viz. specific Correct/Incorrect feedback fields and the common,default feedback field, then..the 'specific correct feedback field' should override the common feedback field.
I hope I am clear.

fnoks · 2016-10-26T10:39:17Z

@supriyarajgopal-spi : Then we do agree :)

thomasmars · 2016-11-14T10:31:58Z

So in order to move forward with merging this pull request in the following must be resolved:

Merge in master
Create a common field for feedback that will be used in general
Make the specific feedback texts for correct and wrong default to empty
Make the specific feedback texts only override the general feedback text when they are defined

More requirements

thomasmars

The following changes are required before the PR can be merged into master

Merge in master branch, resolve any conflicts
Create a common field for feedback that will be used in general
Make the specific feedback texts for correct and wrong default to empty
Make the specific feedback texts only override the general feedback text when they are defined

supriyarajgopal-spi mentioned this pull request Sep 2, 2016

Quiz type not providing 'correct answer feedback' for every question #9

Closed

thomasmars requested changes Sep 27, 2016

View reviewed changes

Implemented suggestions by H5P moderator

c3a40ff

- Indentation - Camel casing - Changes to semantics.json - Added default Incorrect Feedback text

thomasmars previously approved these changes Oct 25, 2016

View reviewed changes

thomasmars reviewed Oct 25, 2016

View reviewed changes

thomasmars requested changes Nov 17, 2016

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #9 Added 'Incorrect feedback text' support #10

Issue #9 Added 'Incorrect feedback text' support #10

supriyarajgopal-spi commented Sep 1, 2016

thomasmars left a comment

thomasmars Sep 27, 2016

thomasmars Sep 27, 2016

thomasmars Sep 27, 2016

thomasmars Sep 27, 2016

thomasmars Sep 27, 2016

thomasmars Sep 27, 2016

thomasmars Sep 27, 2016

supriyarajgopal-spi commented Sep 27, 2016

supriyarajgopal-spi commented Oct 24, 2016

thomasmars left a comment

supriyarajgopal-spi commented Oct 25, 2016 •

edited

Loading

fnoks commented Oct 26, 2016

supriyarajgopal-spi commented Oct 26, 2016

fnoks commented Oct 26, 2016

thomasmars commented Nov 14, 2016

thomasmars left a comment •

edited

Loading

Issue #9 Added 'Incorrect feedback text' support #10

Are you sure you want to change the base?

Issue #9 Added 'Incorrect feedback text' support #10

Conversation

supriyarajgopal-spi commented Sep 1, 2016

thomasmars left a comment

Choose a reason for hiding this comment

thomasmars Sep 27, 2016

Choose a reason for hiding this comment

thomasmars Sep 27, 2016

Choose a reason for hiding this comment

thomasmars Sep 27, 2016

Choose a reason for hiding this comment

thomasmars Sep 27, 2016

Choose a reason for hiding this comment

thomasmars Sep 27, 2016

Choose a reason for hiding this comment

thomasmars Sep 27, 2016

Choose a reason for hiding this comment

thomasmars Sep 27, 2016

Choose a reason for hiding this comment

supriyarajgopal-spi commented Sep 27, 2016

supriyarajgopal-spi commented Oct 24, 2016

thomasmars left a comment

Choose a reason for hiding this comment

supriyarajgopal-spi commented Oct 25, 2016 • edited Loading

fnoks commented Oct 26, 2016

supriyarajgopal-spi commented Oct 26, 2016

fnoks commented Oct 26, 2016

thomasmars commented Nov 14, 2016

thomasmars left a comment • edited Loading

Choose a reason for hiding this comment

supriyarajgopal-spi commented Oct 25, 2016 •

edited

Loading

thomasmars left a comment •

edited

Loading