refactor(MultipleChoice): simplify code (#1992)

WISE-Community · Nov 14, 2024 · ba29819 · ba29819
1 parent c6c885e
commit ba29819
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 98 deletions.
diff --git a/...ponents/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html b/...ponents/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html
@@ -22,8 +22,6 @@
   i18nMatTooltip
   matTooltip="Add Choice"
   matTooltipPosition="above"
-  i18n-aria-label
-  aria-label="Add"
 >
   <mat-icon>add</mat-icon>
 </button>
@@ -84,8 +82,6 @@
         i18n-matTooltip
         matTooltip="Move Up"
         matTooltipPosition="above"
-        i18n-aria-label
-        aria-label="Up"
       >
         <mat-icon>arrow_upward</mat-icon>
       </button>
@@ -98,8 +94,6 @@
         i18n-matTooltip
         matTooltip="Move Down"
         matTooltipPosition="above"
-        i18n-aria-label
-        aria-label="Down"
       >
         <mat-icon>arrow_downward</mat-icon>
       </button>
@@ -111,8 +105,6 @@
         i18n-matTooltip
         matTooltip="Delete"
         matTooltipPosition="above"
-        i18n-aria-label
-        aria-label="Delete"
       >
         <mat-icon>delete</mat-icon>
       </button>

diff --git a/.../components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.harness.ts b/.../components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.harness.ts
@@ -7,12 +7,14 @@ export class MultipleChoiceAuthoringHarness extends ComponentHarness {
   getAddChoiceButton = this.locatorFor(MatButtonHarness.with({ selector: '.add-choice-button' }));
   getChoices = this.locatorForAll('.choice-container');
   getDeleteChoiceButtons = this.locatorForAll(
-    MatButtonHarness.with({ selector: '[aria-label="Delete"]' })
+    MatButtonHarness.with({ selector: '[mattooltip="Delete"]' })
   );
   getMoveDownButtons = this.locatorForAll(
-    MatButtonHarness.with({ selector: '[aria-label="Down"]' })
+    MatButtonHarness.with({ selector: '[mattooltip="Move Down"]' })
+  );
+  getMoveUpButtons = this.locatorForAll(
+    MatButtonHarness.with({ selector: '[mattooltip="Move Up"]' })
   );
-  getMoveUpButtons = this.locatorForAll(MatButtonHarness.with({ selector: '[aria-label="Up"]' }));
   getMultipleAnswerRadioChoice = this.locatorFor(
     MatRadioButtonHarness.with({ label: 'Multiple Answer' })
   );

diff --git a/...mponents/multipleChoice/multiple-choice-student/multiple-choice-student.component.spec.ts b/...mponents/multipleChoice/multiple-choice-student/multiple-choice-student.component.spec.ts
@@ -212,6 +212,7 @@ function singleAnswerSingleCorrectAnswerComponentShouldShowTheFeedbackOnTheSubmi
   it('should show the feedback on the submitted choice', () => {
     selectSingleAnswerChoice(choiceId1);
     checkAnswer();
+    component.displayFeedback();
     const choice1 = getChoiceById(choiceId1);
     const choice2 = getChoiceById(choiceId2);
     const choice3 = getChoiceById(choiceId3);
@@ -261,6 +262,7 @@ function multipleAnswerComponentShouldShowTheFeedbackOnTheSubmittedChoices() {
     selectMultipleAnswerChoice(choiceId2);
     selectMultipleAnswerChoice(choiceId3);
     checkAnswer();
+    component.displayFeedback();
     const choice1 = getChoiceById(choiceId1);
     const choice2 = getChoiceById(choiceId2);
     const choice3 = getChoiceById(choiceId3);

diff --git a/...e5/components/multipleChoice/multiple-choice-student/multiple-choice-student.component.ts b/...e5/components/multipleChoice/multiple-choice-student/multiple-choice-student.component.ts
@@ -245,48 +245,22 @@ export class MultipleChoiceStudent extends ComponentStudent {
   }
 
   private hideAllFeedback(): void {
-    for (const choice of this.choices) {
-      choice.showFeedback = false;
-    }
+    this.choices.forEach((choice) => (choice.showFeedback = false));
   }
 
   checkAnswer(): void {
     if (this.component.isRadio()) {
-      this.checkSingleAnswer();
+      this.isCorrect = this.choices.some((choice) => choice.isCorrect && this.isChecked(choice.id));
     } else {
-      this.checkMultipleAnswer();
+      this.isCorrect = this.choices.every((choice) => this.isStudentChoiceValueCorrect(choice));
     }
   }
 
-  private checkSingleAnswer(): void {
-    let isCorrect = false;
-    for (const choice of this.choices) {
-      if (this.componentHasCorrectAnswer) {
-        if (choice.isCorrect && this.isChecked(choice.id)) {
-          isCorrect = true;
-        }
-      }
-      this.displayFeedbackOnChoiceIfNecessary(choice);
-    }
-    if (this.componentHasCorrectAnswer) {
-      this.isCorrect = isCorrect;
-    }
-  }
-
-  private checkMultipleAnswer(): void {
-    let isAllCorrect = true;
-    for (const choice of this.choices) {
-      if (this.componentHasCorrectAnswer) {
-        isAllCorrect &&= this.isStudentChoiceValueCorrect(choice);
-      }
-      this.displayFeedbackOnChoiceIfNecessary(choice);
-    }
-    if (this.componentHasCorrectAnswer) {
-      this.isCorrect = isAllCorrect;
-    }
+  displayFeedback(): void {
+    this.choices.forEach((choice) => this.displayFeedbackOnChoice(choice));
   }
 
-  private displayFeedbackOnChoiceIfNecessary(choice: any): void {
+  private displayFeedbackOnChoice(choice: any): void {
     if (this.showFeedback && this.isChecked(choice.id)) {
       choice.showFeedback = true;
       choice.feedbackToShow = choice.feedback;
@@ -319,7 +293,10 @@ export class MultipleChoiceStudent extends ComponentStudent {
     };
 
     if (action === 'submit') {
-      this.checkAnswer();
+      if (this.componentHasCorrectAnswer) {
+        this.checkAnswer();
+      }
+      this.displayFeedback();
       if (this.isCorrect != null) {
         studentData.isCorrect = this.isCorrect;
       }
@@ -395,19 +372,17 @@ export class MultipleChoiceStudent extends ComponentStudent {
    * @return a component state with the merged student responses
    */
   createMergedComponentState(componentStates: any[]): any[] {
-    const mergedComponentState: any = this.createNewComponentState();
-    if (componentStates != null) {
-      let mergedStudentChoices = [];
-      for (const componentState of componentStates) {
-        const studentChoices = componentState.studentData.studentChoices;
-        if (studentChoices != null && studentChoices.length > 0) {
-          mergedStudentChoices = mergedStudentChoices.concat(studentChoices);
-        }
+    let mergedStudentChoices = [];
+    for (const componentState of componentStates) {
+      const studentChoices = componentState.studentData.studentChoices;
+      if (studentChoices != null && studentChoices.length > 0) {
+        mergedStudentChoices = mergedStudentChoices.concat(studentChoices);
       }
-      mergedComponentState.studentData = {
-        studentChoices: mergedStudentChoices
-      };
     }
+    const mergedComponentState: any = this.createNewComponentState();
+    mergedComponentState.studentData = {
+      studentChoices: mergedStudentChoices
+    };
     return mergedComponentState;
   }
 

diff --git a/src/assets/wise5/components/multipleChoice/multipleChoiceService.ts b/src/assets/wise5/components/multipleChoice/multipleChoiceService.ts
@@ -35,11 +35,9 @@ export class MultipleChoiceService extends ComponentService {
     studentChoiceIds: string[],
     constraintChoiceIds: string | string[]
   ): boolean {
-    if (constraintChoiceIds instanceof Array) {
-      return arraysContainSameValues(studentChoiceIds, constraintChoiceIds);
-    } else {
-      return studentChoiceIds.includes(constraintChoiceIds);
-    }
+    return constraintChoiceIds instanceof Array
+      ? arraysContainSameValues(studentChoiceIds, constraintChoiceIds)
+      : studentChoiceIds.includes(constraintChoiceIds);
   }
 
   isCompleted(component: any, componentStates: any[], nodeEvents: any[], node: any) {
@@ -59,11 +57,8 @@ export class MultipleChoiceService extends ComponentService {
     return false;
   }
 
-  getStudentChoicesFromComponentState(componentState: any) {
-    if (componentState.studentData) {
-      return componentState.studentData.studentChoices;
-    }
-    return [];
+  private getStudentChoicesFromComponentState(componentState: any): any[] {
+    return componentState.studentData ? componentState.studentData.studentChoices : [];
   }
 
   /**
@@ -97,12 +92,7 @@ export class MultipleChoiceService extends ComponentService {
     return false;
   }
 
-  componentHasCorrectAnswer(component: any) {
-    for (const choice of component.choices) {
-      if (choice.isCorrect) {
-        return true;
-      }
-    }
-    return false;
+  componentHasCorrectAnswer(component: any): boolean {
+    return component.choices.some((choice) => choice.isCorrect);
   }
 }
diff --git a/src/messages.xlf b/src/messages.xlf
@@ -629,7 +629,7 @@
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">85</context>
+          <context context-type="linenumber">83</context>
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/summary/summary-authoring/summary-authoring.component.html</context>
@@ -688,7 +688,7 @@
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">99</context>
+          <context context-type="linenumber">95</context>
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/summary/summary-authoring/summary-authoring.component.html</context>
@@ -799,11 +799,7 @@
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">112</context>
-        </context-group>
-        <context-group purpose="location">
-          <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">115</context>
+          <context context-type="linenumber">106</context>
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/summary/summary-authoring/summary-authoring.component.html</context>
@@ -7814,10 +7810,6 @@ Click &quot;Cancel&quot; to keep the invalid JSON open so you can fix it.</sourc
           <context context-type="sourcefile">src/assets/wise5/components/match/match-authoring/match-authoring.component.html</context>
           <context context-type="linenumber">143</context>
         </context-group>
-        <context-group purpose="location">
-          <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">26</context>
-        </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/summary/summary-authoring/summary-authoring.component.html</context>
           <context context-type="linenumber">116</context>
@@ -9854,7 +9846,7 @@ Click &quot;Cancel&quot; to keep the invalid JSON open so you can fix it.</sourc
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">71</context>
+          <context context-type="linenumber">69</context>
         </context-group>
       </trans-unit>
       <trans-unit id="29242856838907f4f025afb9a0467fbe4511c055" datatype="html">
@@ -16085,10 +16077,6 @@ Are you sure you want to proceed?</source>
           <context context-type="sourcefile">src/assets/wise5/components/match/match-authoring/match-authoring.component.html</context>
           <context context-type="linenumber">186</context>
         </context-group>
-        <context-group purpose="location">
-          <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">88</context>
-        </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/summary/summary-authoring/summary-authoring.component.html</context>
           <context context-type="linenumber">163</context>
@@ -16123,10 +16111,6 @@ Are you sure you want to proceed?</source>
           <context context-type="sourcefile">src/assets/wise5/components/match/match-authoring/match-authoring.component.html</context>
           <context context-type="linenumber">200</context>
         </context-group>
-        <context-group purpose="location">
-          <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">102</context>
-        </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/summary/summary-authoring/summary-authoring.component.html</context>
           <context context-type="linenumber">177</context>
@@ -19481,7 +19465,7 @@ Warning: This will delete all existing choices and buckets in this component.</s
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">31,33</context>
+          <context context-type="linenumber">29,31</context>
         </context-group>
       </trans-unit>
       <trans-unit id="3e20a6ecefca9c87d5dedc6e425a571629d232c5" datatype="html">
@@ -19503,7 +19487,7 @@ Warning: This will delete all existing choices and buckets in this component.</s
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">43</context>
+          <context context-type="linenumber">41</context>
         </context-group>
       </trans-unit>
       <trans-unit id="b75de65916c3254e808d6757d451c2c9abe56d06" datatype="html">
@@ -19584,7 +19568,7 @@ Warning: This will delete all existing choices and buckets in this component.</s
         </context-group>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">62,64</context>
+          <context context-type="linenumber">60,62</context>
         </context-group>
       </trans-unit>
       <trans-unit id="1ba7568bba29f8d8e352f93e67002d0b548eb838" datatype="html">
@@ -19814,21 +19798,21 @@ Warning: This will delete all existing choices in this component.</source>
         <source>Choice Text</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">41</context>
+          <context context-type="linenumber">39</context>
         </context-group>
       </trans-unit>
       <trans-unit id="4488359b3a14b99b193ac292e1f16ee47766ab8b" datatype="html">
         <source>Is Correct</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">60</context>
+          <context context-type="linenumber">58</context>
         </context-group>
       </trans-unit>
       <trans-unit id="5317cde482fdb766008de3e1ae09cf0610366abc" datatype="html">
         <source>Optional</source>
         <context-group purpose="location">
           <context context-type="sourcefile">src/assets/wise5/components/multipleChoice/multiple-choice-authoring/multiple-choice-authoring.component.html</context>
-          <context context-type="linenumber">73</context>
+          <context context-type="linenumber">71</context>
         </context-group>
       </trans-unit>
       <trans-unit id="83b360aecbf5316060c382f91fb84c421743fe98" datatype="html">