Fix: Expanded feedback to add images and more control (fixes #222)

js/helpers.js

@@ -32,26 +32,26 @@ const helpers = {
     // Comparison operators
     switch (operator) {
       case '===':
-        if (value === text) return block.fn(this);
+        if (value === text) return block.fn ? block.fn(this) : true;
         break;
       case '=': case '==':
         // eslint-disable-next-line eqeqeq
-        if (value == text) return block.fn(this);
+        if (value == text) return block.fn ? block.fn(this) : true;
         break;
       case '>=':
-        if (value >= text) return block.fn(this);
+        if (value >= text) return block.fn ? block.fn(this) : true;
         break;
       case '<=':
-        if (value <= text) return block.fn(this);
+        if (value <= text) return block.fn ? block.fn(this) : true;
         break;
       case '>':
-        if (value > text) return block.fn(this);
+        if (value > text) return block.fn ? block.fn(this) : true;
         break;
       case '<':
-        if (value < text) return block.fn(this);
+        if (value < text) return block.fn ? block.fn(this) : true;
         break;
     }
-    return block.inverse(this);
+    return block.inverse ? block.inverse(this) : false;
   },
 
   math(lvalue, operator, rvalue, options) {

js/models/itemsQuestionModel.js

@@ -137,35 +137,25 @@ export default class ItemsQuestionModel extends BlendedItemsComponentQuestionMod
     return scores.slice(0, this.get('_selectable')).filter(score => score < 0).reduce((minScore, score) => (minScore += score), 0);
   }
 
-  setupFeedback() {
-    if (!this.has('_feedback')) return;
-
-    if (this.get('_isCorrect')) {
-      this.setupCorrectFeedback();
-      return;
-    }
-
-    if (this.isPartlyCorrect()) {
-      this.setupPartlyCorrectFeedback();
-      return;
-    }
-
-    // apply individual item feedback
+  getFeedback (_feedback = this.get('_feedback')) {
+    if (!_feedback) return {};
     const activeItem = this.getActiveItem();
-    if (this.isSingleSelect() && activeItem.get('feedback')) {
-      this.setupIndividualFeedback(activeItem);
-      return;
+    const activeItemFeedback = activeItem.get('feedback');
+    const isIndividualFeedback = (!this.isCorrect() && !this.isPartlyCorrect() && this.isSingleSelect() && activeItemFeedback);
+    const feedback = super.getFeedback(_feedback);
+    if (!isIndividualFeedback) return feedback;
+    if (typeof activeItemFeedback === 'string') {
+      // old style
+      return {
+        ...feedback,
+        body: activeItemFeedback || ''
+      };
     }
-
-    this.setupIncorrectFeedback();
-  }
-
-  setupIndividualFeedback(selectedItem) {
-    const json = this.toJSON();
-    this.set({
-      feedbackTitle: Handlebars.compile(this.getFeedbackTitle(this.get('_feedback')))(json),
-      feedbackMessage: Handlebars.compile(selectedItem.get('feedback'))(json)
-    });
+    // new style
+    return {
+      ...feedback,
+      ...activeItemFeedback
+    };
   }
 
   isPartlyCorrect() {

js/models/questionModel.js

@@ -150,6 +150,10 @@ class QuestionModel extends ComponentModel {
   // Should return a boolean based upon whether to question is correct or not
   isCorrect() {}
 
+  // Used by the question to determine if the question is incorrect or partly correct
+  // Should return a boolean
+  isPartlyCorrect() {}
+
   /**
    * Used to set the legacy _score property based upon the _questionWeight and correctness
    * @deprecated Please use get score, get maxScore and get minScore instead
@@ -245,63 +249,106 @@ class QuestionModel extends ComponentModel {
 
   }
 
-  // Used to setup the correct, incorrect and partly correct feedback
-  setupFeedback() {
-    if (!this.has('_feedback')) return;
-
-    if (this.get('_isCorrect')) {
-      this.setupCorrectFeedback();
-    } else if (this.isPartlyCorrect()) {
-      this.setupPartlyCorrectFeedback();
-    } else {
-      this.setupIncorrectFeedback();
-    }
-  }
-
-  // Used by the question to determine if the question is incorrect or partly correct
-  // Should return a boolean
-  isPartlyCorrect() {}
-
-  setupCorrectFeedback() {
-    this.set({
-      feedbackTitle: this.getFeedbackTitle(),
-      feedbackMessage: Handlebars.compile(this.get('_feedback').correct)(this.toJSON())
-    });
-  }
-
-  setupPartlyCorrectFeedback() {
-    const feedback = this.get('_feedback')._partlyCorrect;
+  getFeedback (_feedback = this.get('_feedback')) {
+    if (!_feedback) return {};
+    const isFinal = (this.get('_attemptsLeft') === 0);
+    const correctness = this.get('_isCorrect')
+      ? 'correct'
+      : this.isPartlyCorrect()
+        ? 'partlyCorrect'
+        : 'incorrect';
+
+    // global feedback title / _classes
+    const {
+      altTitle = Adapt.course.get('_globals')._accessibility.altFeedbackTitle || '',
+      title,
+      _classes
+    } = _feedback;
+
+    switch (correctness) {
+      case 'correct': {
+        if (typeof _feedback.correct === 'string') {
+          // old style
+          return {
+            // add higher values
+            altTitle,
+            title,
+            _classes,
+            body: _feedback.correct
+          };
+        }
+        // new style
+        const feedbackCorrect = _feedback._correct;
+        return {
+          // add higher values
+          ...feedbackCorrect || {},
+          altTitle: feedbackCorrect?.altTitle || altTitle,
+          title: feedbackCorrect?.title || title,
+          _classes: feedbackCorrect?._classes || _classes
+        };
+      }
 
-    if (feedback?.final) {
-      this.setAttemptSpecificFeedback(feedback);
-    } else {
-      this.setupIncorrectFeedback();
+      case 'partlyCorrect': {
+        if (typeof _feedback._partlyCorrect === 'object') {
+          // old style
+          return {
+            // add higher values
+            altTitle,
+            title,
+            _classes,
+            body: !isFinal
+              ? _feedback._partlyCorrect.notFinal
+              : _feedback._partlyCorrect.final
+          };
+        }
+        // new style
+        const feedbackPartlyCorrect = !isFinal ? _feedback._partlyCorrectNotFinal : _feedback._partlyCorrectFinal;
+        return {
+          // add higher values
+          ...feedbackPartlyCorrect || {},
+          altTitle: feedbackPartlyCorrect?.altTitle || altTitle,
+          title: feedbackPartlyCorrect?.title || title,
+          _classes: feedbackPartlyCorrect?._classes || _classes
+        };
+      }
+      case 'incorrect': {
+        if (typeof _feedback._incorrect === 'object') {
+          // old style
+          return {
+            // add higher values
+            altTitle,
+            title,
+            _classes,
+            body: !isFinal
+              ? _feedback._incorrect.notFinal
+              : _feedback._incorrect.final
+          };
+        }
+        // new style
+        const feedbackIncorrect = !isFinal ? _feedback._incorrectNotFinal : _feedback._incorrectFinal;
+        return {
+          // add higher values
+          ...feedbackIncorrect || {},
+          altTitle: feedbackIncorrect?.altTitle || altTitle,
+          title: feedbackIncorrect?.title || title,
+          _classes: feedbackIncorrect?._classes || _classes
+        };
+      }
     }
+    return {};
   }
 
-  setupIncorrectFeedback() {
-    this.setAttemptSpecificFeedback(this.get('_feedback')._incorrect);
-  }
-
-  setAttemptSpecificFeedback(feedback) {
-    const body = (this.get('_attemptsLeft') && feedback.notFinal) || feedback.final;
-
+  // Used to setup the correct, incorrect and partly correct feedback
+  setupFeedback() {
+    if (!this.has('_feedback')) return;
+    const { altTitle = '', title = '', body = '' } = this.getFeedback();
     this.set({
-      altFeedbackTitle: this.getAltFeedbackTitle(),
-      feedbackTitle: this.getFeedbackTitle(),
+      altFeedbackTitle: Handlebars.compile(altTitle)(this.toJSON()),
+      feedbackTitle: Handlebars.compile(title)(this.toJSON()),
       feedbackMessage: Handlebars.compile(body)(this.toJSON())
     });
   }
 
-  getAltFeedbackTitle() {
-    return this.get('_feedback').altTitle || Adapt.course.get('_globals')._accessibility.altFeedbackTitle || '';
-  }
-
-  getFeedbackTitle() {
-    const title = this.get('_feedback').title || this.get('displayTitle') || this.get('title') || '';
-    return Handlebars.compile(title)(this.toJSON());
-  }
-
   /**
    * Used to determine whether the learner is allowed to interact with the question component or not.
    * @return {Boolean}

less/core/notify.less

@@ -29,30 +29,27 @@
     .icon-warning;
   }
 
-  // Sets up full width title and 50/50 text/image split below
-  // --------------------------------------------------
-  @media (min-width: @device-width-medium) {
-    &__content.has-image &__content-inner {
+  &__popup.has-image {
+    .notify__section-inner {
       display: flex;
-      flex-direction: row;
-      flex-wrap: wrap;
-    }
-
-    &__content.has-image &__title {
-      width: 100%;
+      flex-direction: column;
+      row-gap: 1rem;
     }
 
-    &__content.has-image &__body,
-    &__content.has-image &__image-container {
-      width: 50%;
-    }
-  }
-
-  // Left aligns image in notify pop up
-  // --------------------------------------------------
-  @media (min-width: @device-width-medium) {
-    &__content.align-image-left &__content-inner {
-      flex-direction: row-reverse;
+    @media (min-width: @device-width-medium) {
+      &.align-image-left .notify__section-inner,
+      &.align-image-right .notify__section-inner {
+        flex-direction: row;
+        column-gap: 1rem;
+
+        .notify__body {
+          width: 60%;
+        }
+
+        .notify__image-container {
+          width: 40%;
+        }
+      }
     }
   }
 }

templates/notifyPopup.hbs

@@ -1,7 +1,7 @@
 {{! make the _globals object in course.json available to this template}}
 {{import_globals}}
 
-<div {{#each _attributes}}{{@key}}="{{this}}" {{/each}} class="notify__popup notify__popup-type-{{_type}}{{#if _classes}} {{_classes}}{{/if}}">
+<div {{#each _attributes}}{{@key}}="{{this}}" {{/each}} class="notify__popup notify__popup-type-{{_type}}{{#if _classes}} {{_classes}}{{/if}}{{#if _graphic._src}} has-image{{/if}}{{#if _imageAlignment}} align-image-{{_imageAlignment}}{{/if}}">
   <div class="notify__popup-inner">
 
     <div class="notify__content">
@@ -26,14 +26,50 @@
             </div>
           </div>
         {{/if}}
-
-        {{#if body}}
-        <div class="notify__body">
-          <div class="notify__body-inner">
-            {{{compile body}}}
+
+        <div class="notify__section">
+          <div class="notify__section-inner">
+
+            {{#if _graphic._src}}
+            {{#any (equals _imageAlignment 'left') (equals _imageAlignment 'top')}}
+            <div class="notify__image-container{{#if _graphic.attribution}} has-attribution{{/if}}">
+              <img class="notify__image" src="{{_graphic._src}}" {{#if _graphic.alt}}aria-label="{{{_graphic.alt}}}"{{else}}aria-hidden="true"{{/if}}>
+              {{#if _graphic.attribution}}
+              <div class="notify__attribution">
+                <div class="notify__attribution-inner">
+                  {{{_graphic.attribution}}}
+                </div>
+              </div>
+              {{/if}}
+            </div>
+            {{~/any~}}
+            {{~/if~}}
+
+            {{~#if body~}}
+            <div class="notify__body">
+              <div class="notify__body-inner">
+                {{{compile body}}}
+              </div>
+            </div>
+            {{~/if~}}
+
+            {{~#if _graphic._src~}}
+            {{~#any (equals _imageAlignment 'right') (equals _imageAlignment 'bottom')~}}
+            <div class="notify__image-container{{#if _graphic.attribution}} has-attribution{{/if}}">
+              <img class="notify__image" src="{{_graphic._src}}" {{#if _graphic.alt}}aria-label="{{{_graphic.alt}}}"{{else}}aria-hidden="true"{{/if}}>
+              {{#if _graphic.attribution}}
+              <div class="notify__attribution">
+                <div class="notify__attribution-inner">
+                  {{{_graphic.attribution}}}
+                </div>
+              </div>
+              {{/if}}
+            </div>
+            {{/any}}
+            {{/if}}
+
           </div>
         </div>
-        {{/if}}
 
         {{#equals _type "alert"}}
         <div class="notify__btn-container">