From 20d8aed25c1a981718b4fe66a411d76585fcd435 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Tue, 29 Nov 2022 19:18:01 +0530
Subject: [PATCH 01/18] Reduce question text font size in review mode.

---
 .../fhir/catalog/DemoQuestionnaireFragment.kt |   4 +-
 ...QuestionnaireItemHeaderViewInReviewMode.kt |  46 ++++
 ...eQuestionAnswerDisplayViewHolderFactory.kt |   2 +-
 ...estionnaire_item_header_in_review_mode.xml |  99 +++++++++
 ...naire_item_simple_question_answer_view.xml |   2 +-
 datacapture/src/main/res/values/attrs.xml     |   3 +
 datacapture/src/main/res/values/styles.xml    |   6 +-
 ...tionnaireItemHeaderViewInReviewModeTest.kt | 196 ++++++++++++++++++
 8 files changed, 354 insertions(+), 4 deletions(-)
 create mode 100644 datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewMode.kt
 create mode 100644 datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml
 create mode 100644 datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewModeTest.kt

diff --git a/catalog/src/main/java/com/google/android/fhir/catalog/DemoQuestionnaireFragment.kt b/catalog/src/main/java/com/google/android/fhir/catalog/DemoQuestionnaireFragment.kt
index 0cb1bcb7e7..9bc1aee22f 100644
--- a/catalog/src/main/java/com/google/android/fhir/catalog/DemoQuestionnaireFragment.kt
+++ b/catalog/src/main/java/com/google/android/fhir/catalog/DemoQuestionnaireFragment.kt
@@ -40,6 +40,7 @@ import androidx.navigation.fragment.navArgs
 import com.google.android.fhir.catalog.ModalBottomSheetFragment.Companion.BUNDLE_ERROR_KEY
 import com.google.android.fhir.catalog.ModalBottomSheetFragment.Companion.REQUEST_ERROR_KEY
 import com.google.android.fhir.datacapture.QuestionnaireFragment
+import com.google.android.fhir.datacapture.QuestionnaireFragment.Companion.EXTRA_ENABLE_REVIEW_PAGE
 import com.google.android.fhir.datacapture.QuestionnaireFragment.Companion.SUBMIT_REQUEST_KEY
 import com.google.android.material.card.MaterialCardView
 import kotlinx.coroutines.launch
@@ -152,7 +153,8 @@ class DemoQuestionnaireFragment : Fragment() {
             args =
               bundleOf(
                 QuestionnaireFragment.EXTRA_QUESTIONNAIRE_JSON_STRING to
-                  viewModel.getQuestionnaireJson()
+                  viewModel.getQuestionnaireJson(),
+                EXTRA_ENABLE_REVIEW_PAGE to true
               )
           )
         }
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewMode.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewMode.kt
new file mode 100644
index 0000000000..72430bec90
--- /dev/null
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewMode.kt
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2022 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.android.fhir.datacapture.views
+
+import android.content.Context
+import android.util.AttributeSet
+import android.view.LayoutInflater
+import android.widget.LinearLayout
+import android.widget.TextView
+import com.google.android.fhir.datacapture.R
+import com.google.android.fhir.datacapture.localizedInstructionsSpanned
+import com.google.android.fhir.datacapture.localizedPrefixSpanned
+import org.hl7.fhir.r4.model.Questionnaire
+
+internal class QuestionnaireItemHeaderViewInReviewMode(context: Context, attrs: AttributeSet?) :
+  LinearLayout(context, attrs) {
+
+  init {
+    LayoutInflater.from(context)
+      .inflate(R.layout.questionnaire_item_header_in_review_mode, this, true)
+  }
+
+  fun bind(questionnaireItem: Questionnaire.QuestionnaireItemComponent) {
+    val prefix = findViewById<TextView>(R.id.prefix)
+    val question = findViewById<TextView>(R.id.question)
+    val hint = findViewById<TextView>(R.id.hint)
+    prefix.updateTextAndVisibility(questionnaireItem.localizedPrefixSpanned)
+    updateQuestionText(question, questionnaireItem)
+    hint.updateTextAndVisibility(questionnaireItem.localizedInstructionsSpanned)
+    visibility = getViewGroupVisibility(prefix, question, hint)
+  }
+}
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
index 4ad3b041d0..f2144ae394 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
@@ -34,7 +34,7 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
   QuestionnaireItemViewHolderFactory(R.layout.questionnaire_item_simple_question_answer_view) {
   override fun getQuestionnaireItemViewHolderDelegate() =
     object : QuestionnaireItemViewHolderDelegate {
-      private lateinit var header: QuestionnaireItemHeaderView
+      private lateinit var header: QuestionnaireItemHeaderViewInReviewMode
       private lateinit var flyOverTextView: TextView
       private lateinit var answerTextView: TextView
       private lateinit var divider: MaterialDivider
diff --git a/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml b/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml
new file mode 100644
index 0000000000..dfff796306
--- /dev/null
+++ b/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml
@@ -0,0 +1,99 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<androidx.constraintlayout.widget.ConstraintLayout
+    xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:app="http://schemas.android.com/apk/res-auto"
+    xmlns:tools="http://schemas.android.com/tools"
+    android:layout_width="match_parent"
+    android:layout_height="wrap_content"
+>
+
+  <TextView
+        android:id="@+id/prefix"
+        style="?attr/questionnaireReviewModeQuestionTextStyle"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:paddingEnd="@dimen/prefix_padding_end"
+        app:layout_constraintEnd_toStartOf="@+id/question"
+        app:layout_constraintStart_toStartOf="parent"
+        app:layout_constraintTop_toTopOf="parent"
+    />
+
+  <TextView
+        android:id="@+id/question"
+        style="?attr/questionnaireReviewModeQuestionTextStyle"
+        android:layout_width="0dp"
+        android:layout_height="wrap_content"
+        app:layout_constraintEnd_toEndOf="parent"
+        app:layout_constraintStart_toEndOf="@id/prefix"
+        app:layout_constraintTop_toTopOf="parent"
+    />
+
+  <LinearLayout
+        android:id="@+id/helpContainer"
+        android:layout_width="0dp"
+        android:layout_height="wrap_content"
+        android:layout_marginTop="@dimen/instructions_top_margin"
+        android:orientation="horizontal"
+        app:layout_constraintTop_toBottomOf="@id/question"
+        app:layout_constraintStart_toStartOf="parent"
+    >
+
+    <TextView
+            android:id="@+id/hint"
+            style="?attr/questionnaireSubtitleTextStyle"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:layout_weight=".7"
+            android:layout_gravity="center_vertical"
+            tools:text="Sample helper text"
+        />
+    <Button
+            style="?attr/questionnaireHelpButtonStyle"
+            android:id="@+id/helpButton"
+            android:layout_width="@dimen/help_button_width"
+            android:layout_height="@dimen/help_button_height"
+            android:layout_gravity="center_vertical"
+            android:layout_weight=".3"
+            android:layout_marginStart="@dimen/help_button_margin_start"
+            app:icon="@drawable/ic_help_48px"
+        />
+  </LinearLayout>
+
+  <com.google.android.material.card.MaterialCardView
+        android:id="@+id/helpCardView"
+        android:layout_width="match_parent"
+        android:layout_height="wrap_content"
+        android:layout_marginTop="@dimen/help_card_view_margin_top"
+        android:visibility="gone"
+        app:layout_constraintBottom_toBottomOf="parent"
+        app:layout_constraintTop_toBottomOf="@id/helpContainer"
+        app:layout_constraintStart_toStartOf="parent"
+    >
+    <LinearLayout
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            android:orientation="vertical"
+            android:background="?attr/colorSurfaceVariant"
+        >
+      <TextView
+                style="?attr/questionnaireHelpHeaderStyle"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:layout_marginHorizontal="@dimen/help_header_horizontal_margin"
+                android:layout_marginTop="@dimen/help_card_view_margin_top"
+                android:layout_marginBottom="@dimen/help_header_margin_bottom"
+                android:text="@string/help"
+            />
+      <TextView
+                android:id="@+id/helpText"
+                style="?attr/questionnaireHelpTextStyle"
+                android:layout_marginHorizontal="@dimen/help_header_horizontal_margin"
+                android:layout_marginBottom="@dimen/help_margin_bottom"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                tools:text="Sample helper text"
+            />
+    </LinearLayout>
+  </com.google.android.material.card.MaterialCardView>
+
+</androidx.constraintlayout.widget.ConstraintLayout>
diff --git a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
index 23ffc9464e..88931f4197 100644
--- a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
+++ b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
@@ -22,7 +22,7 @@
     android:layout_marginTop="@dimen/item_margin_vertical"
     android:orientation="vertical"
 >
-    <com.google.android.fhir.datacapture.views.QuestionnaireItemHeaderView
+    <com.google.android.fhir.datacapture.views.QuestionnaireItemHeaderViewInReviewMode
         android:id="@+id/header"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
diff --git a/datacapture/src/main/res/values/attrs.xml b/datacapture/src/main/res/values/attrs.xml
index be8e551290..3e48cf2859 100644
--- a/datacapture/src/main/res/values/attrs.xml
+++ b/datacapture/src/main/res/values/attrs.xml
@@ -33,6 +33,9 @@
     <!--   Style for answer text. -->
     <attr name="questionnaireAnswerTextStyle" format="reference" />
 
+    <!--   Style for question text in review mode -->
+    <attr name="questionnaireReviewModeQuestionTextStyle" format="reference" />
+
     <!--   Style for radio buttons. -->
     <attr name="questionnaireRadioButtonStyle" format="reference" />
     <!--   Style for checkbox buttons. -->
diff --git a/datacapture/src/main/res/values/styles.xml b/datacapture/src/main/res/values/styles.xml
index d6371b4b1b..863c3133a4 100644
--- a/datacapture/src/main/res/values/styles.xml
+++ b/datacapture/src/main/res/values/styles.xml
@@ -43,7 +43,11 @@
       <item
             name="questionnaireQuestionTextStyle"
         >@style/TextAppearance.Material3.TitleMedium</item>
-        <item
+
+      <item
+            name="questionnaireReviewModeQuestionTextStyle"
+        >@style/TextAppearance.Material3.TitleMedium</item>
+      <item
             name="questionnaireSubtitleTextStyle"
         >@style/TextAppearance.Material3.BodyMedium</item>
       <item
diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewModeTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewModeTest.kt
new file mode 100644
index 0000000000..428e7509b0
--- /dev/null
+++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewModeTest.kt
@@ -0,0 +1,196 @@
+/*
+ * Copyright 2022 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.android.fhir.datacapture.views
+
+import android.view.View
+import android.widget.FrameLayout
+import android.widget.TextView
+import androidx.core.view.isVisible
+import com.google.android.fhir.datacapture.DisplayItemControlType
+import com.google.android.fhir.datacapture.EXTENSION_DISPLAY_CATEGORY_SYSTEM
+import com.google.android.fhir.datacapture.EXTENSION_DISPLAY_CATEGORY_URL
+import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_SYSTEM
+import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_URL
+import com.google.android.fhir.datacapture.INSTRUCTIONS
+import com.google.android.fhir.datacapture.R
+import com.google.common.truth.Truth.assertThat
+import org.hl7.fhir.r4.model.CodeableConcept
+import org.hl7.fhir.r4.model.Coding
+import org.hl7.fhir.r4.model.Extension
+import org.hl7.fhir.r4.model.Questionnaire
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+
+@RunWith(RobolectricTestRunner::class)
+class QuestionnaireItemHeaderViewInReviewModeTest {
+  private val parent =
+    FrameLayout(
+      RuntimeEnvironment.getApplication().apply { setTheme(R.style.Theme_Material3_DayNight) }
+    )
+  private val view = QuestionnaireItemHeaderViewInReviewMode(parent.context, null)
+
+  @Test
+  fun shouldShowPrefix() {
+    view.bind(Questionnaire.QuestionnaireItemComponent().apply { prefix = "Prefix?" })
+
+    assertThat(view.findViewById<TextView>(R.id.prefix).isVisible).isTrue()
+    assertThat(view.findViewById<TextView>(R.id.prefix).text.toString()).isEqualTo("Prefix?")
+  }
+
+  @Test
+  fun shouldHidePrefix() {
+    view.bind(Questionnaire.QuestionnaireItemComponent().apply { prefix = "" })
+
+    assertThat(view.findViewById<TextView>(R.id.prefix).isVisible).isFalse()
+  }
+
+  @Test
+  fun shouldShowQuestion() {
+    view.bind(
+      Questionnaire.QuestionnaireItemComponent().apply {
+        repeats = true
+        text = "Question?"
+      }
+    )
+
+    assertThat(view.findViewById<TextView>(R.id.question).text.toString()).isEqualTo("Question?")
+  }
+
+  @Test
+  fun `shows instructions`() {
+    view.bind(
+      Questionnaire.QuestionnaireItemComponent().apply {
+        item =
+          listOf(
+            Questionnaire.QuestionnaireItemComponent().apply {
+              linkId = "nested-display-question"
+              text = "subtitle text"
+              extension = listOf(displayCategoryExtensionWithInstructionsCode)
+              type = Questionnaire.QuestionnaireItemType.DISPLAY
+            }
+          )
+      }
+    )
+
+    assertThat(view.findViewById<TextView>(R.id.hint).isVisible).isTrue()
+    assertThat(view.findViewById<TextView>(R.id.hint).text.toString()).isEqualTo("subtitle text")
+  }
+
+  @Test
+  fun `hides instructions`() {
+    view.bind(
+      Questionnaire.QuestionnaireItemComponent().apply {
+        item =
+          listOf(
+            Questionnaire.QuestionnaireItemComponent().apply {
+              linkId = "nested-display-question"
+              type = Questionnaire.QuestionnaireItemType.DISPLAY
+            }
+          )
+      }
+    )
+
+    assertThat(view.findViewById<TextView>(R.id.hint).visibility).isEqualTo(View.GONE)
+  }
+
+  @Test
+  fun `shows headerItem view`() {
+    view.bind(
+      Questionnaire.QuestionnaireItemComponent().apply {
+        item =
+          listOf(
+            Questionnaire.QuestionnaireItemComponent().apply {
+              linkId = "nested-display-question"
+              text = "subtitle text"
+              extension = listOf(displayCategoryExtensionWithInstructionsCode)
+              type = Questionnaire.QuestionnaireItemType.DISPLAY
+            }
+          )
+      }
+    )
+
+    assertThat(view.visibility).isEqualTo(View.VISIBLE)
+  }
+
+  @Test
+  fun shouldHideHeaderView() {
+    view.bind(Questionnaire.QuestionnaireItemComponent())
+
+    assertThat(view.visibility).isEqualTo(View.GONE)
+  }
+
+  @Test
+  fun `shows * at the end of question text`() {
+    view.bind(
+      Questionnaire.QuestionnaireItemComponent().apply {
+        text = "Question?"
+        required = true
+      }
+    )
+
+    assertThat(view.findViewById<TextView>(R.id.question).text.toString()).isEqualTo("Question? *")
+  }
+
+  @Test
+  fun `does not show * at the end of question text`() {
+    view.bind(Questionnaire.QuestionnaireItemComponent().apply { text = "Question?" })
+
+    assertThat(view.findViewById<TextView>(R.id.question).text.toString()).isEqualTo("Question?")
+  }
+
+  @Test
+  fun `shows * even if question text is missing`() {
+    view.bind(Questionnaire.QuestionnaireItemComponent().apply { required = true })
+
+    assertThat(view.findViewById<TextView>(R.id.question).text.toString())
+      .isEqualTo(view.context.applicationContext.resources.getString(R.string.space_asterisk))
+  }
+
+  private val displayCategoryExtensionWithInstructionsCode =
+    Extension().apply {
+      url = EXTENSION_DISPLAY_CATEGORY_URL
+      setValue(
+        CodeableConcept().apply {
+          coding =
+            listOf(
+              Coding().apply {
+                code = INSTRUCTIONS
+                system = EXTENSION_DISPLAY_CATEGORY_SYSTEM
+              }
+            )
+        }
+      )
+    }
+
+  private val itemControlExtensionWithHelpCode =
+    Extension().apply {
+      url = EXTENSION_ITEM_CONTROL_URL
+      setValue(
+        CodeableConcept().apply {
+          coding =
+            listOf(
+              Coding().apply {
+                code = DisplayItemControlType.HELP.extensionCode
+                system = EXTENSION_ITEM_CONTROL_SYSTEM
+              }
+            )
+        }
+      )
+    }
+}

From 8de63312988810966f5efe48972b7238c079095e Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Wed, 30 Nov 2022 11:34:40 +0530
Subject: [PATCH 02/18] Do not show help button in review mode.

---
 ...estionnaire_item_header_in_review_mode.xml | 74 ++-----------------
 1 file changed, 7 insertions(+), 67 deletions(-)

diff --git a/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml b/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml
index dfff796306..b2848d7bf1 100644
--- a/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml
+++ b/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml
@@ -6,7 +6,6 @@
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
 >
-
   <TextView
         android:id="@+id/prefix"
         style="?attr/questionnaireReviewModeQuestionTextStyle"
@@ -17,7 +16,6 @@
         app:layout_constraintStart_toStartOf="parent"
         app:layout_constraintTop_toTopOf="parent"
     />
-
   <TextView
         android:id="@+id/question"
         style="?attr/questionnaireReviewModeQuestionTextStyle"
@@ -27,73 +25,15 @@
         app:layout_constraintStart_toEndOf="@id/prefix"
         app:layout_constraintTop_toTopOf="parent"
     />
-
-  <LinearLayout
-        android:id="@+id/helpContainer"
-        android:layout_width="0dp"
+  <TextView
+        android:id="@+id/hint"
+        style="?attr/questionnaireSubtitleTextStyle"
+        android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_marginTop="@dimen/instructions_top_margin"
-        android:orientation="horizontal"
+        android:layout_gravity="center_vertical"
+        tools:text="Sample helper text"
         app:layout_constraintTop_toBottomOf="@id/question"
         app:layout_constraintStart_toStartOf="parent"
-    >
-
-    <TextView
-            android:id="@+id/hint"
-            style="?attr/questionnaireSubtitleTextStyle"
-            android:layout_width="wrap_content"
-            android:layout_height="wrap_content"
-            android:layout_weight=".7"
-            android:layout_gravity="center_vertical"
-            tools:text="Sample helper text"
-        />
-    <Button
-            style="?attr/questionnaireHelpButtonStyle"
-            android:id="@+id/helpButton"
-            android:layout_width="@dimen/help_button_width"
-            android:layout_height="@dimen/help_button_height"
-            android:layout_gravity="center_vertical"
-            android:layout_weight=".3"
-            android:layout_marginStart="@dimen/help_button_margin_start"
-            app:icon="@drawable/ic_help_48px"
-        />
-  </LinearLayout>
-
-  <com.google.android.material.card.MaterialCardView
-        android:id="@+id/helpCardView"
-        android:layout_width="match_parent"
-        android:layout_height="wrap_content"
-        android:layout_marginTop="@dimen/help_card_view_margin_top"
-        android:visibility="gone"
-        app:layout_constraintBottom_toBottomOf="parent"
-        app:layout_constraintTop_toBottomOf="@id/helpContainer"
-        app:layout_constraintStart_toStartOf="parent"
-    >
-    <LinearLayout
-            android:layout_width="match_parent"
-            android:layout_height="wrap_content"
-            android:orientation="vertical"
-            android:background="?attr/colorSurfaceVariant"
-        >
-      <TextView
-                style="?attr/questionnaireHelpHeaderStyle"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                android:layout_marginHorizontal="@dimen/help_header_horizontal_margin"
-                android:layout_marginTop="@dimen/help_card_view_margin_top"
-                android:layout_marginBottom="@dimen/help_header_margin_bottom"
-                android:text="@string/help"
-            />
-      <TextView
-                android:id="@+id/helpText"
-                style="?attr/questionnaireHelpTextStyle"
-                android:layout_marginHorizontal="@dimen/help_header_horizontal_margin"
-                android:layout_marginBottom="@dimen/help_margin_bottom"
-                android:layout_width="wrap_content"
-                android:layout_height="wrap_content"
-                tools:text="Sample helper text"
-            />
-    </LinearLayout>
-  </com.google.android.material.card.MaterialCardView>
-
+    />
 </androidx.constraintlayout.widget.ConstraintLayout>

From 61ec1253edbf035482d7f048add520b9801e91b8 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Wed, 30 Nov 2022 18:04:55 +0530
Subject: [PATCH 03/18] Add error icon if text is not answered and set error
 color to text if value is not answered.

---
 ...eQuestionAnswerDisplayViewHolderFactory.kt | 43 +++++++++++++++++++
 .../views/QuestionnaireItemViewItem.kt        |  3 ++
 .../src/main/res/drawable/ic_error_48px.xml   | 14 ++++++
 ...naire_item_simple_question_answer_view.xml | 29 ++++++++++---
 datacapture/src/main/res/values/dimens.xml    |  1 +
 datacapture/src/main/res/values/styles.xml    |  4 +-
 ...stionAnswerDisplayViewHolderFactoryTest.kt | 41 ++++++++++++++++++
 7 files changed, 126 insertions(+), 9 deletions(-)
 create mode 100644 datacapture/src/main/res/drawable/ic_error_48px.xml

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
index f2144ae394..84ef2b3e01 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
@@ -16,10 +16,14 @@
 
 package com.google.android.fhir.datacapture.views
 
+import android.content.res.Resources
+import android.util.TypedValue
 import android.view.View
 import android.view.View.GONE
 import android.view.View.VISIBLE
 import android.widget.TextView
+import androidx.annotation.AttrRes
+import androidx.core.graphics.ColorUtils
 import com.google.android.fhir.datacapture.R
 import com.google.android.fhir.datacapture.localizedFlyoverSpanned
 import com.google.android.fhir.datacapture.validation.ValidationResult
@@ -37,6 +41,7 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
       private lateinit var header: QuestionnaireItemHeaderViewInReviewMode
       private lateinit var flyOverTextView: TextView
       private lateinit var answerTextView: TextView
+      private lateinit var errorIcon: View
       private lateinit var divider: MaterialDivider
       override lateinit var questionnaireItemViewItem: QuestionnaireItemViewItem
 
@@ -45,6 +50,7 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
         flyOverTextView = itemView.findViewById(R.id.flyover_text_view)
         answerTextView = itemView.findViewById(R.id.answer_text_view)
         divider = itemView.findViewById(R.id.text_divider)
+        errorIcon = itemView.findViewById(R.id.error_icon_in_review_mode)
       }
 
       override fun bind(questionnaireItemViewItem: QuestionnaireItemViewItem) {
@@ -69,8 +75,24 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
               else -> VISIBLE
             }
           text = questionnaireItemViewItem.answerString(context)
+          setTextColorAsPerAnswer()
         }
 
+        errorIcon.visibility =
+          when (questionnaireItemViewItem.questionnaireItem.type) {
+            Questionnaire.QuestionnaireItemType.GROUP,
+            Questionnaire.QuestionnaireItemType.DISPLAY -> {
+              GONE
+            }
+            else -> {
+              if (questionnaireItemViewItem.hasAnswerString) {
+                GONE
+              } else {
+                VISIBLE
+              }
+            }
+          }
+
         divider.visibility =
           if (header.visibility == VISIBLE ||
               flyOverTextView.visibility == VISIBLE ||
@@ -85,5 +107,26 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
       override fun displayValidationResult(validationResult: ValidationResult) {}
 
       override fun setReadOnly(isReadOnly: Boolean) {}
+
+      private fun TextView.setTextColorAsPerAnswer() =
+        if (questionnaireItemViewItem.hasAnswerString) {
+          val color = context.theme.getColorIntFromMaterialAttribute(android.R.attr.textColor)
+          setTextColor(ColorUtils.setAlphaComponent(color, TEXT_COLOR_ALPHA))
+        } else {
+          val color = context.theme.getColorIntFromMaterialAttribute(android.R.attr.colorError)
+          setTextColor(color)
+        }
     }
 }
+
+private const val TEXT_COLOR_ALPHA = 0x80
+
+/**
+ * Reads @ColorInt value from android resource attribute. e.g [resId] value is
+ * [android.R.attr.colorError] to get the colorInt.
+ */
+internal fun Resources.Theme.getColorIntFromMaterialAttribute(@AttrRes resId: Int): Int {
+  val value = TypedValue()
+  resolveAttribute(resId, value, true)
+  return value.data
+}
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt
index 232b10e23c..796fef7d03 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt
@@ -150,6 +150,9 @@ data class QuestionnaireItemViewItem(
     return questionnaireResponseItem.answer.joinToString { it.displayString(context) }
   }
 
+  internal val hasAnswerString: Boolean
+    get() = questionnaireResponseItem.hasAnswer()
+
   fun isAnswerOptionSelected(
     answerOption: Questionnaire.QuestionnaireItemAnswerOptionComponent
   ): Boolean {
diff --git a/datacapture/src/main/res/drawable/ic_error_48px.xml b/datacapture/src/main/res/drawable/ic_error_48px.xml
new file mode 100644
index 0000000000..56e092bf4d
--- /dev/null
+++ b/datacapture/src/main/res/drawable/ic_error_48px.xml
@@ -0,0 +1,14 @@
+<vector
+    android:autoMirrored="true"
+    android:height="48dp"
+    android:tint="?attr/colorControlNormal"
+    android:viewportHeight="48"
+    android:viewportWidth="48"
+    android:width="48dp"
+    xmlns:android="http://schemas.android.com/apk/res/android"
+>
+    <path
+        android:fillColor="@android:color/white"
+        android:pathData="M24,34Q24.7,34 25.175,33.525Q25.65,33.05 25.65,32.35Q25.65,31.65 25.175,31.175Q24.7,30.7 24,30.7Q23.3,30.7 22.825,31.175Q22.35,31.65 22.35,32.35Q22.35,33.05 22.825,33.525Q23.3,34 24,34ZM22.65,26.35H25.65V13.7H22.65ZM24,44Q19.9,44 16.25,42.425Q12.6,40.85 9.875,38.125Q7.15,35.4 5.575,31.75Q4,28.1 4,23.95Q4,19.85 5.575,16.2Q7.15,12.55 9.875,9.85Q12.6,7.15 16.25,5.575Q19.9,4 24.05,4Q28.15,4 31.8,5.575Q35.45,7.15 38.15,9.85Q40.85,12.55 42.425,16.2Q44,19.85 44,24Q44,28.1 42.425,31.75Q40.85,35.4 38.15,38.125Q35.45,40.85 31.8,42.425Q28.15,44 24,44Z"
+    />
+</vector>
diff --git a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
index 88931f4197..ce70ccaceb 100644
--- a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
+++ b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
@@ -34,14 +34,31 @@
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
     />
-
-    <TextView
-        android:id="@+id/answer_text_view"
-        style="?attr/questionnaireAnswerTextStyle"
-        android:layout_width="match_parent"
+    <LinearLayout
+        xmlns:android="http://schemas.android.com/apk/res/android"
+        android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_marginTop="@dimen/item_margin_vertical"
-    />
+        android:orientation="horizontal"
+        android:gravity="center_vertical"
+    >
+    <TextView
+            android:id="@+id/error_icon_in_review_mode"
+            android:layout_width="20dp"
+            android:layout_height="20dp"
+            android:layout_marginEnd="@dimen/error_icon_margin_end"
+            android:background="@drawable/ic_error_48px"
+            android:backgroundTint="?attr/colorError"
+            android:visibility="gone"
+        />
+
+    <TextView
+            android:id="@+id/answer_text_view"
+            style="?attr/questionnaireAnswerTextStyle"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+        />
+    </LinearLayout>
 
     <com.google.android.material.divider.MaterialDivider
         android:id="@+id/text_divider"
diff --git a/datacapture/src/main/res/values/dimens.xml b/datacapture/src/main/res/values/dimens.xml
index d5733cd52f..e22fd695f8 100644
--- a/datacapture/src/main/res/values/dimens.xml
+++ b/datacapture/src/main/res/values/dimens.xml
@@ -40,4 +40,5 @@
     <dimen name="help_header_margin_bottom">4dp</dimen>
     <dimen name="help_margin_bottom">16dp</dimen>
     <dimen name="icon_inset">16dp</dimen>
+    <dimen name="error_icon_margin_end">4dp</dimen>
 </resources>
diff --git a/datacapture/src/main/res/values/styles.xml b/datacapture/src/main/res/values/styles.xml
index 863c3133a4..f837ba2a99 100644
--- a/datacapture/src/main/res/values/styles.xml
+++ b/datacapture/src/main/res/values/styles.xml
@@ -136,9 +136,7 @@
     <style
         name="Questionnaire.AnswerStyle"
         parent="TextAppearance.Material3.BodyMedium"
-    >
-        <item name="android:alpha">0.5</item>
-    </style>
+    ></style>
 
     <style
         name="Questionnaire.ErrorStyle"
diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
index deb68c9cbb..c28fc74683 100644
--- a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
+++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
@@ -288,4 +288,45 @@ class QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest {
     assertThat(viewHolder.itemView.findViewById<MaterialDivider>(R.id.text_divider).visibility)
       .isEqualTo(View.VISIBLE)
   }
+
+  @Test
+  fun `shows error icon if answer is not present`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply {
+          linkId = "parent-question"
+          type = Questionnaire.QuestionnaireItemType.BOOLEAN
+        },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<View>(R.id.error_icon_in_review_mode).visibility)
+      .isEqualTo(View.VISIBLE)
+  }
+
+  @Test
+  fun `does not show error icon if answer is present`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply {
+          linkId = "parent-question"
+          type = Questionnaire.QuestionnaireItemType.BOOLEAN
+        },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent()
+          .addAnswer(
+            QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+              value = BooleanType(true)
+            }
+          ),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<View>(R.id.error_icon_in_review_mode).visibility)
+      .isEqualTo(View.GONE)
+  }
 }

From a7d83a5ade2cce1b929d770e2fbe2b13167805c7 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Wed, 30 Nov 2022 18:46:30 +0530
Subject: [PATCH 04/18] spotless apply

---
 datacapture/src/main/res/values/styles.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datacapture/src/main/res/values/styles.xml b/datacapture/src/main/res/values/styles.xml
index f837ba2a99..9a04d8d331 100644
--- a/datacapture/src/main/res/values/styles.xml
+++ b/datacapture/src/main/res/values/styles.xml
@@ -136,7 +136,7 @@
     <style
         name="Questionnaire.AnswerStyle"
         parent="TextAppearance.Material3.BodyMedium"
-    ></style>
+    />
 
     <style
         name="Questionnaire.ErrorStyle"

From e9d9ce66a856474f6f0a3ef14fa4a387c2bf2d20 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Fri, 2 Dec 2022 14:52:36 +0530
Subject: [PATCH 05/18] update not answered text color.

---
 .../views/QuestionnaireItemHeaderView.kt      |  6 +++++-
 ...eQuestionAnswerDisplayViewHolderFactory.kt | 19 +++----------------
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
index a97e62f1ea..e22b7243d6 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
@@ -156,8 +156,12 @@ private fun SpannableStringBuilder.appendWithSpan(value: String, @ColorInt color
   setSpan(ForegroundColorSpan(color), start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
 }
 
+/**
+ * Reads @ColorInt value from android resource attribute. e.g [attrColor] value is
+ * [R.attr.colorError] to get the colorInt.
+ */
 @ColorInt
-private fun Context.getColorFromAttr(@AttrRes attrColor: Int): Int {
+internal fun Context.getColorFromAttr(@AttrRes attrColor: Int): Int {
   val typedValue = TypedValue()
   theme.resolveAttribute(attrColor, typedValue, true)
   return typedValue.data
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
index 84ef2b3e01..049dbc83e7 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
@@ -16,13 +16,10 @@
 
 package com.google.android.fhir.datacapture.views
 
-import android.content.res.Resources
-import android.util.TypedValue
 import android.view.View
 import android.view.View.GONE
 import android.view.View.VISIBLE
 import android.widget.TextView
-import androidx.annotation.AttrRes
 import androidx.core.graphics.ColorUtils
 import com.google.android.fhir.datacapture.R
 import com.google.android.fhir.datacapture.localizedFlyoverSpanned
@@ -110,23 +107,13 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
 
       private fun TextView.setTextColorAsPerAnswer() =
         if (questionnaireItemViewItem.hasAnswerString) {
-          val color = context.theme.getColorIntFromMaterialAttribute(android.R.attr.textColor)
+          val color = context.getColorFromAttr(android.R.attr.textColor)
           setTextColor(ColorUtils.setAlphaComponent(color, TEXT_COLOR_ALPHA))
         } else {
-          val color = context.theme.getColorIntFromMaterialAttribute(android.R.attr.colorError)
+          val color = context.getColorFromAttr(R.attr.colorError)
           setTextColor(color)
         }
     }
 }
 
-private const val TEXT_COLOR_ALPHA = 0x80
-
-/**
- * Reads @ColorInt value from android resource attribute. e.g [resId] value is
- * [android.R.attr.colorError] to get the colorInt.
- */
-internal fun Resources.Theme.getColorIntFromMaterialAttribute(@AttrRes resId: Int): Int {
-  val value = TypedValue()
-  resolveAttribute(resId, value, true)
-  return value.data
-}
+private const val TEXT_COLOR_ALPHA = 0x80 // 50%

From 0dca5affa34546f6ffc4048fe99a83110d410ea5 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Mon, 5 Dec 2022 18:08:53 +0530
Subject: [PATCH 06/18] Address review comment.

---
 .../fhir/datacapture/QuestionnaireFragment.kt |   4 +-
 .../views/QuestionnaireItemHeaderView.kt      |  98 --------------
 .../views/QuestionnaireItemHeaderViewUtil.kt  | 122 ++++++++++++++++++
 ...eQuestionAnswerDisplayViewHolderFactory.kt |   3 +-
 4 files changed, 126 insertions(+), 101 deletions(-)
 create mode 100644 datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
index 6a6db89c1c..fb5fae8391 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
@@ -276,8 +276,8 @@ open class QuestionnaireFragment : Fragment() {
     const val EXTRA_QUESTIONNAIRE_RESPONSE_JSON_URI = "questionnaire-response-uri"
 
     /**
-     * A [Boolean] extra to control if a review page is shown. By default it will be shown at the
-     * end of the questionnaire.
+     * A [Boolean] extra to control if a review page is shown. By default value is false, it will be
+     * shown at the end of the questionnaire if value is true.
      */
     const val EXTRA_ENABLE_REVIEW_PAGE = "enable-review-page"
 
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
index a452eb2bd0..af95177ffa 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
@@ -17,28 +17,15 @@
 package com.google.android.fhir.datacapture.views
 
 import android.content.Context
-import android.text.Spannable
-import android.text.SpannableStringBuilder
-import android.text.Spanned
-import android.text.style.ForegroundColorSpan
 import android.util.AttributeSet
-import android.util.TypedValue
 import android.view.LayoutInflater
-import android.view.View
 import android.view.View.GONE
 import android.view.View.VISIBLE
-import android.widget.Button
 import android.widget.LinearLayout
 import android.widget.TextView
-import androidx.annotation.AttrRes
-import androidx.annotation.ColorInt
 import com.google.android.fhir.datacapture.R
-import com.google.android.fhir.datacapture.hasHelpButton
-import com.google.android.fhir.datacapture.localizedHelpSpanned
 import com.google.android.fhir.datacapture.localizedInstructionsSpanned
 import com.google.android.fhir.datacapture.localizedPrefixSpanned
-import com.google.android.fhir.datacapture.localizedTextSpanned
-import com.google.android.material.card.MaterialCardView
 import org.hl7.fhir.r4.model.Questionnaire
 
 /** View for the prefix, question, and hint of a questionnaire item. */
@@ -81,88 +68,3 @@ internal class QuestionnaireItemHeaderView(context: Context, attrs: AttributeSet
     errorTextView.text = errorText
   }
 }
-
-internal fun TextView.updateTextAndVisibility(localizedText: Spanned? = null) {
-  text = localizedText
-  visibility =
-    if (localizedText.isNullOrEmpty()) {
-      GONE
-    } else {
-      VISIBLE
-    }
-}
-
-/** Returns [VISIBLE] if any of the [view] is visible, else returns [GONE]. */
-internal fun getViewGroupVisibility(vararg view: TextView): Int {
-  if (view.any { it.visibility == VISIBLE }) {
-    return VISIBLE
-  }
-  return GONE
-}
-
-internal fun initHelpButton(
-  view: View,
-  questionnaireItem: Questionnaire.QuestionnaireItemComponent
-) {
-  val helpButton = view.findViewById<Button>(R.id.helpButton)
-  helpButton.visibility =
-    if (questionnaireItem.hasHelpButton) {
-      VISIBLE
-    } else {
-      GONE
-    }
-  val helpCardView = view.findViewById<MaterialCardView>(R.id.helpCardView)
-  var isHelpCardViewVisible = false
-  helpButton.setOnClickListener {
-    if (isHelpCardViewVisible) {
-      isHelpCardViewVisible = false
-      helpCardView.visibility = GONE
-    } else {
-      isHelpCardViewVisible = true
-      helpCardView.visibility = VISIBLE
-    }
-  }
-
-  view
-    .findViewById<TextView>(R.id.helpText)
-    .updateTextAndVisibility(questionnaireItem.localizedHelpSpanned)
-}
-
-/**
- * Updates textview [R.id.question] with
- * [Questionnaire.QuestionnaireItemComponent.localizedTextSpanned] text and `*` if
- * [Questionnaire.QuestionnaireItemComponent.required] is true. And applies [R.attr.colorError] to
- * `*`.
- */
-internal fun updateQuestionText(
-  questionTextView: TextView,
-  questionnaireItem: Questionnaire.QuestionnaireItemComponent,
-) {
-  val builder = SpannableStringBuilder()
-  questionnaireItem.localizedTextSpanned?.let { builder.append(it) }
-  if (questionnaireItem.required) {
-    builder.appendWithSpan(
-      questionTextView.context.applicationContext.getString(R.string.space_asterisk),
-      questionTextView.context.getColorFromAttr(R.attr.colorError)
-    )
-  }
-  questionTextView.updateTextAndVisibility(builder)
-}
-
-private fun SpannableStringBuilder.appendWithSpan(value: String, @ColorInt color: Int) {
-  val start = length
-  append(value)
-  val end = length
-  setSpan(ForegroundColorSpan(color), start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
-}
-
-/**
- * Reads @ColorInt value from android resource attribute. e.g [attrColor] value is
- * [R.attr.colorError] to get the colorInt.
- */
-@ColorInt
-internal fun Context.getColorFromAttr(@AttrRes attrColor: Int): Int {
-  val typedValue = TypedValue()
-  theme.resolveAttribute(attrColor, typedValue, true)
-  return typedValue.data
-}
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
new file mode 100644
index 0000000000..7b55dbb859
--- /dev/null
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
@@ -0,0 +1,122 @@
+/*
+ * Copyright 2022 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.android.fhir.datacapture.views
+
+import android.content.Context
+import android.text.Spannable
+import android.text.SpannableStringBuilder
+import android.text.Spanned
+import android.text.style.ForegroundColorSpan
+import android.util.TypedValue
+import android.view.View
+import android.view.View.GONE
+import android.view.View.VISIBLE
+import android.widget.Button
+import android.widget.TextView
+import androidx.annotation.AttrRes
+import androidx.annotation.ColorInt
+import com.google.android.fhir.datacapture.R
+import com.google.android.fhir.datacapture.hasHelpButton
+import com.google.android.fhir.datacapture.localizedHelpSpanned
+import com.google.android.fhir.datacapture.localizedTextSpanned
+import com.google.android.material.card.MaterialCardView
+import org.hl7.fhir.r4.model.Questionnaire
+
+/**
+ * Updates textview [R.id.question] with
+ * [Questionnaire.QuestionnaireItemComponent.localizedTextSpanned] text and `*` if
+ * [Questionnaire.QuestionnaireItemComponent.required] is true. And applies [R.attr.colorError] to
+ * `*`.
+ */
+internal fun updateQuestionText(
+  questionTextView: TextView,
+  questionnaireItem: Questionnaire.QuestionnaireItemComponent,
+) {
+  val builder = SpannableStringBuilder()
+  questionnaireItem.localizedTextSpanned?.let { builder.append(it) }
+  if (questionnaireItem.required) {
+    builder.appendWithSpan(
+      questionTextView.context.applicationContext.getString(R.string.space_asterisk),
+      questionTextView.context.getColorFromAttr(R.attr.colorError)
+    )
+  }
+  questionTextView.updateTextAndVisibility(builder)
+}
+
+internal fun TextView.updateTextAndVisibility(localizedText: Spanned? = null) {
+  text = localizedText
+  visibility =
+    if (localizedText.isNullOrEmpty()) {
+      GONE
+    } else {
+      VISIBLE
+    }
+}
+
+/** Returns [VISIBLE] if any of the [view] is visible, else returns [GONE]. */
+internal fun getViewGroupVisibility(vararg view: TextView): Int {
+  if (view.any { it.visibility == VISIBLE }) {
+    return VISIBLE
+  }
+  return GONE
+}
+
+internal fun initHelpButton(
+  view: View,
+  questionnaireItem: Questionnaire.QuestionnaireItemComponent
+) {
+  val helpButton = view.findViewById<Button>(R.id.helpButton)
+  helpButton.visibility =
+    if (questionnaireItem.hasHelpButton) {
+      VISIBLE
+    } else {
+      GONE
+    }
+  val helpCardView = view.findViewById<MaterialCardView>(R.id.helpCardView)
+  var isHelpCardViewVisible = false
+  helpButton.setOnClickListener {
+    if (isHelpCardViewVisible) {
+      isHelpCardViewVisible = false
+      helpCardView.visibility = GONE
+    } else {
+      isHelpCardViewVisible = true
+      helpCardView.visibility = VISIBLE
+    }
+  }
+
+  view
+    .findViewById<TextView>(R.id.helpText)
+    .updateTextAndVisibility(questionnaireItem.localizedHelpSpanned)
+}
+
+private fun SpannableStringBuilder.appendWithSpan(value: String, @ColorInt color: Int) {
+  val start = length
+  append(value)
+  val end = length
+  setSpan(ForegroundColorSpan(color), start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
+}
+
+/**
+ * Reads @ColorInt value from android resource attribute. e.g [attrColor] value is
+ * [R.attr.colorError] to get the colorInt.
+ */
+@ColorInt
+internal fun Context.getColorFromAttr(@AttrRes attrColor: Int): Int {
+  val typedValue = TypedValue()
+  theme.resolveAttribute(attrColor, typedValue, true)
+  return typedValue.data
+}
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
index 049dbc83e7..a51b010d9f 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
@@ -29,7 +29,8 @@ import org.hl7.fhir.r4.model.Questionnaire
 
 /**
  * This view is a container that contains the question and answer obtained from
- * questionnaireItemViewItem [QuestionnaireItemViewItem].
+ * questionnaireItemViewItem [QuestionnaireItemViewItem]. It is used in review mode to review the
+ * answer of the question.
  */
 internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
   QuestionnaireItemViewHolderFactory(R.layout.questionnaire_item_simple_question_answer_view) {

From b1616764d85598c608c2931ad5d1f21978d68b2d Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Mon, 5 Dec 2022 18:25:06 +0530
Subject: [PATCH 07/18] Remove test which is not required.

---
 ...QuestionAnswerDisplayViewHolderFactoryTest.kt | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
index 896f71d26b..c28fc74683 100644
--- a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
+++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
@@ -25,7 +25,6 @@ import com.google.android.fhir.datacapture.DisplayItemControlType
 import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_SYSTEM
 import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_URL
 import com.google.android.fhir.datacapture.R
-import com.google.android.fhir.datacapture.validation.NotValidated
 import com.google.android.fhir.datacapture.validation.Valid
 import com.google.android.material.divider.MaterialDivider
 import com.google.common.truth.Truth.assertThat
@@ -290,21 +289,6 @@ class QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest {
       .isEqualTo(View.VISIBLE)
   }
 
-  @Test
-  fun `hides error textview in the header`() {
-    viewHolder.bind(
-      QuestionnaireItemViewItem(
-        Questionnaire.QuestionnaireItemComponent(),
-        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
-        validationResult = NotValidated,
-        answersChangedCallback = { _, _, _ -> },
-      )
-    )
-
-    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.error_text_at_header).visibility)
-      .isEqualTo(View.GONE)
-  }
-
   @Test
   fun `shows error icon if answer is not present`() {
     viewHolder.bind(

From 05b8c7a354e46f9d4b8f2f5d6b31928b61408fdb Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Wed, 4 Jan 2023 18:46:54 +0530
Subject: [PATCH 08/18] Address review comments.

---
 .../views/QuestionnaireGroupTypeHeaderView.kt |   7 +-
 .../views/QuestionnaireItemHeaderView.kt      |   9 +-
 ...QuestionnaireItemHeaderViewInReviewMode.kt |  46 ----
 .../views/QuestionnaireItemHeaderViewUtil.kt  |  25 +++
 ...eQuestionAnswerDisplayViewHolderFactory.kt |  21 +-
 ...estionnaire_item_header_in_review_mode.xml |  39 ----
 ...naire_item_simple_question_answer_view.xml |  38 +++-
 ...tionnaireItemHeaderViewInReviewModeTest.kt | 196 -----------------
 ...stionAnswerDisplayViewHolderFactoryTest.kt | 208 ++++++++++++++++++
 9 files changed, 300 insertions(+), 289 deletions(-)
 delete mode 100644 datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewMode.kt
 delete mode 100644 datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml
 delete mode 100644 datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewModeTest.kt

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireGroupTypeHeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireGroupTypeHeaderView.kt
index d25233281f..7519c397fe 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireGroupTypeHeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireGroupTypeHeaderView.kt
@@ -37,7 +37,12 @@ internal class QuestionnaireGroupTypeHeaderView(context: Context, attrs: Attribu
     val prefix = findViewById<TextView>(R.id.prefix)
     val question = findViewById<TextView>(R.id.question)
     val hint = findViewById<TextView>(R.id.hint)
-    initHelpButton(this, questionnaireItem)
+    initHelpButton(
+      findViewById(R.id.helpButton),
+      findViewById(R.id.helpCardView),
+      findViewById(R.id.helpText),
+      questionnaireItem
+    )
     prefix.updateTextAndVisibility(questionnaireItem.localizedPrefixSpanned)
     updateQuestionText(question, questionnaireItem)
     hint.updateTextAndVisibility(questionnaireItem.localizedInstructionsSpanned)
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
index af95177ffa..2003bd165d 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
@@ -19,8 +19,6 @@ package com.google.android.fhir.datacapture.views
 import android.content.Context
 import android.util.AttributeSet
 import android.view.LayoutInflater
-import android.view.View.GONE
-import android.view.View.VISIBLE
 import android.widget.LinearLayout
 import android.widget.TextView
 import com.google.android.fhir.datacapture.R
@@ -45,7 +43,12 @@ internal class QuestionnaireItemHeaderView(context: Context, attrs: AttributeSet
     prefix.updateTextAndVisibility(questionnaireItem.localizedPrefixSpanned)
     updateQuestionText(question, questionnaireItem)
     hint.updateTextAndVisibility(questionnaireItem.localizedInstructionsSpanned)
-    initHelpButton(this, questionnaireItem)
+    initHelpButton(
+      findViewById(R.id.helpButton),
+      findViewById(R.id.helpCardView),
+      findViewById(R.id.helpText),
+      questionnaireItem
+    )
     //   Make the entire view GONE if there is nothing to show. This is to avoid an empty row in the
     // questionnaire.
     visibility = getViewGroupVisibility(prefix, question, hint)
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewMode.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewMode.kt
deleted file mode 100644
index 72430bec90..0000000000
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewMode.kt
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
- * Copyright 2022 Google LLC
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *       http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.google.android.fhir.datacapture.views
-
-import android.content.Context
-import android.util.AttributeSet
-import android.view.LayoutInflater
-import android.widget.LinearLayout
-import android.widget.TextView
-import com.google.android.fhir.datacapture.R
-import com.google.android.fhir.datacapture.localizedInstructionsSpanned
-import com.google.android.fhir.datacapture.localizedPrefixSpanned
-import org.hl7.fhir.r4.model.Questionnaire
-
-internal class QuestionnaireItemHeaderViewInReviewMode(context: Context, attrs: AttributeSet?) :
-  LinearLayout(context, attrs) {
-
-  init {
-    LayoutInflater.from(context)
-      .inflate(R.layout.questionnaire_item_header_in_review_mode, this, true)
-  }
-
-  fun bind(questionnaireItem: Questionnaire.QuestionnaireItemComponent) {
-    val prefix = findViewById<TextView>(R.id.prefix)
-    val question = findViewById<TextView>(R.id.question)
-    val hint = findViewById<TextView>(R.id.hint)
-    prefix.updateTextAndVisibility(questionnaireItem.localizedPrefixSpanned)
-    updateQuestionText(question, questionnaireItem)
-    hint.updateTextAndVisibility(questionnaireItem.localizedInstructionsSpanned)
-    visibility = getViewGroupVisibility(prefix, question, hint)
-  }
-}
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
index 7b55dbb859..df05faf12d 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
@@ -103,6 +103,31 @@ internal fun initHelpButton(
     .updateTextAndVisibility(questionnaireItem.localizedHelpSpanned)
 }
 
+internal fun initHelpButton(
+  helpButton: Button,
+  helpCardView: MaterialCardView,
+  helpText: TextView,
+  questionnaireItem: Questionnaire.QuestionnaireItemComponent
+) {
+  helpButton.visibility =
+    if (questionnaireItem.hasHelpButton) {
+      VISIBLE
+    } else {
+      GONE
+    }
+  var isHelpCardViewVisible = false
+  helpButton.setOnClickListener {
+    if (isHelpCardViewVisible) {
+      isHelpCardViewVisible = false
+      helpCardView.visibility = GONE
+    } else {
+      isHelpCardViewVisible = true
+      helpCardView.visibility = VISIBLE
+    }
+  }
+  helpText.updateTextAndVisibility(questionnaireItem.localizedHelpSpanned)
+}
+
 private fun SpannableStringBuilder.appendWithSpan(value: String, @ColorInt color: Int) {
   val start = length
   append(value)
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
index a51b010d9f..07a60d6268 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
@@ -20,9 +20,12 @@ import android.view.View
 import android.view.View.GONE
 import android.view.View.VISIBLE
 import android.widget.TextView
+import androidx.constraintlayout.widget.ConstraintLayout
 import androidx.core.graphics.ColorUtils
 import com.google.android.fhir.datacapture.R
 import com.google.android.fhir.datacapture.localizedFlyoverSpanned
+import com.google.android.fhir.datacapture.localizedInstructionsSpanned
+import com.google.android.fhir.datacapture.localizedPrefixSpanned
 import com.google.android.fhir.datacapture.validation.ValidationResult
 import com.google.android.material.divider.MaterialDivider
 import org.hl7.fhir.r4.model.Questionnaire
@@ -36,11 +39,14 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
   QuestionnaireItemViewHolderFactory(R.layout.questionnaire_item_simple_question_answer_view) {
   override fun getQuestionnaireItemViewHolderDelegate() =
     object : QuestionnaireItemViewHolderDelegate {
-      private lateinit var header: QuestionnaireItemHeaderViewInReviewMode
       private lateinit var flyOverTextView: TextView
       private lateinit var answerTextView: TextView
       private lateinit var errorIcon: View
       private lateinit var divider: MaterialDivider
+      private lateinit var prefix: TextView
+      private lateinit var question: TextView
+      private lateinit var hint: TextView
+      private lateinit var header: ConstraintLayout
       override lateinit var questionnaireItemViewItem: QuestionnaireItemViewItem
 
       override fun init(itemView: View) {
@@ -49,10 +55,21 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
         answerTextView = itemView.findViewById(R.id.answer_text_view)
         divider = itemView.findViewById(R.id.text_divider)
         errorIcon = itemView.findViewById(R.id.error_icon_in_review_mode)
+        prefix = itemView.findViewById(R.id.prefix)
+        question = itemView.findViewById(R.id.question)
+        hint = itemView.findViewById(R.id.hint)
       }
 
       override fun bind(questionnaireItemViewItem: QuestionnaireItemViewItem) {
-        header.bind(questionnaireItemViewItem.questionnaireItem)
+        prefix.updateTextAndVisibility(
+          questionnaireItemViewItem.questionnaireItem.localizedPrefixSpanned
+        )
+        updateQuestionText(question, questionnaireItemViewItem.questionnaireItem)
+        hint.updateTextAndVisibility(
+          questionnaireItemViewItem.questionnaireItem.localizedInstructionsSpanned
+        )
+        header.visibility = getViewGroupVisibility(prefix, question, hint)
+
         val localizedFlyoverSpanned =
           questionnaireItemViewItem.questionnaireItem.localizedFlyoverSpanned
         flyOverTextView.apply {
diff --git a/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml b/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml
deleted file mode 100644
index b2848d7bf1..0000000000
--- a/datacapture/src/main/res/layout/questionnaire_item_header_in_review_mode.xml
+++ /dev/null
@@ -1,39 +0,0 @@
-<?xml version="1.0" encoding="utf-8" ?>
-<androidx.constraintlayout.widget.ConstraintLayout
-    xmlns:android="http://schemas.android.com/apk/res/android"
-    xmlns:app="http://schemas.android.com/apk/res-auto"
-    xmlns:tools="http://schemas.android.com/tools"
-    android:layout_width="match_parent"
-    android:layout_height="wrap_content"
->
-  <TextView
-        android:id="@+id/prefix"
-        style="?attr/questionnaireReviewModeQuestionTextStyle"
-        android:layout_width="wrap_content"
-        android:layout_height="wrap_content"
-        android:paddingEnd="@dimen/prefix_padding_end"
-        app:layout_constraintEnd_toStartOf="@+id/question"
-        app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toTopOf="parent"
-    />
-  <TextView
-        android:id="@+id/question"
-        style="?attr/questionnaireReviewModeQuestionTextStyle"
-        android:layout_width="0dp"
-        android:layout_height="wrap_content"
-        app:layout_constraintEnd_toEndOf="parent"
-        app:layout_constraintStart_toEndOf="@id/prefix"
-        app:layout_constraintTop_toTopOf="parent"
-    />
-  <TextView
-        android:id="@+id/hint"
-        style="?attr/questionnaireSubtitleTextStyle"
-        android:layout_width="wrap_content"
-        android:layout_height="wrap_content"
-        android:layout_marginTop="@dimen/instructions_top_margin"
-        android:layout_gravity="center_vertical"
-        tools:text="Sample helper text"
-        app:layout_constraintTop_toBottomOf="@id/question"
-        app:layout_constraintStart_toStartOf="parent"
-    />
-</androidx.constraintlayout.widget.ConstraintLayout>
diff --git a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
index ce70ccaceb..b2a7dbb1e4 100644
--- a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
+++ b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
@@ -22,11 +22,45 @@
     android:layout_marginTop="@dimen/item_margin_vertical"
     android:orientation="vertical"
 >
-    <com.google.android.fhir.datacapture.views.QuestionnaireItemHeaderViewInReviewMode
+    <androidx.constraintlayout.widget.ConstraintLayout
+        xmlns:android="http://schemas.android.com/apk/res/android"
+        xmlns:app="http://schemas.android.com/apk/res-auto"
+        xmlns:tools="http://schemas.android.com/tools"
         android:id="@+id/header"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
-    />
+    >
+        <TextView
+            android:id="@+id/prefix"
+            style="?attr/questionnaireReviewModeQuestionTextStyle"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:paddingEnd="@dimen/prefix_padding_end"
+            app:layout_constraintEnd_toStartOf="@+id/question"
+            app:layout_constraintStart_toStartOf="parent"
+            app:layout_constraintTop_toTopOf="parent"
+        />
+        <TextView
+            android:id="@+id/question"
+            style="?attr/questionnaireReviewModeQuestionTextStyle"
+            android:layout_width="0dp"
+            android:layout_height="wrap_content"
+            app:layout_constraintEnd_toEndOf="parent"
+            app:layout_constraintStart_toEndOf="@id/prefix"
+            app:layout_constraintTop_toTopOf="parent"
+        />
+        <TextView
+            android:id="@+id/hint"
+            style="?attr/questionnaireSubtitleTextStyle"
+            android:layout_width="wrap_content"
+            android:layout_height="wrap_content"
+            android:layout_marginTop="@dimen/instructions_top_margin"
+            android:layout_gravity="center_vertical"
+            tools:text="Sample helper text"
+            app:layout_constraintTop_toBottomOf="@id/question"
+            app:layout_constraintStart_toStartOf="parent"
+        />
+    </androidx.constraintlayout.widget.ConstraintLayout>
 
     <TextView
         android:id="@+id/flyover_text_view"
diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewModeTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewModeTest.kt
deleted file mode 100644
index 428e7509b0..0000000000
--- a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewInReviewModeTest.kt
+++ /dev/null
@@ -1,196 +0,0 @@
-/*
- * Copyright 2022 Google LLC
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *       http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.google.android.fhir.datacapture.views
-
-import android.view.View
-import android.widget.FrameLayout
-import android.widget.TextView
-import androidx.core.view.isVisible
-import com.google.android.fhir.datacapture.DisplayItemControlType
-import com.google.android.fhir.datacapture.EXTENSION_DISPLAY_CATEGORY_SYSTEM
-import com.google.android.fhir.datacapture.EXTENSION_DISPLAY_CATEGORY_URL
-import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_SYSTEM
-import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_URL
-import com.google.android.fhir.datacapture.INSTRUCTIONS
-import com.google.android.fhir.datacapture.R
-import com.google.common.truth.Truth.assertThat
-import org.hl7.fhir.r4.model.CodeableConcept
-import org.hl7.fhir.r4.model.Coding
-import org.hl7.fhir.r4.model.Extension
-import org.hl7.fhir.r4.model.Questionnaire
-import org.junit.Test
-import org.junit.runner.RunWith
-import org.robolectric.RobolectricTestRunner
-import org.robolectric.RuntimeEnvironment
-
-@RunWith(RobolectricTestRunner::class)
-class QuestionnaireItemHeaderViewInReviewModeTest {
-  private val parent =
-    FrameLayout(
-      RuntimeEnvironment.getApplication().apply { setTheme(R.style.Theme_Material3_DayNight) }
-    )
-  private val view = QuestionnaireItemHeaderViewInReviewMode(parent.context, null)
-
-  @Test
-  fun shouldShowPrefix() {
-    view.bind(Questionnaire.QuestionnaireItemComponent().apply { prefix = "Prefix?" })
-
-    assertThat(view.findViewById<TextView>(R.id.prefix).isVisible).isTrue()
-    assertThat(view.findViewById<TextView>(R.id.prefix).text.toString()).isEqualTo("Prefix?")
-  }
-
-  @Test
-  fun shouldHidePrefix() {
-    view.bind(Questionnaire.QuestionnaireItemComponent().apply { prefix = "" })
-
-    assertThat(view.findViewById<TextView>(R.id.prefix).isVisible).isFalse()
-  }
-
-  @Test
-  fun shouldShowQuestion() {
-    view.bind(
-      Questionnaire.QuestionnaireItemComponent().apply {
-        repeats = true
-        text = "Question?"
-      }
-    )
-
-    assertThat(view.findViewById<TextView>(R.id.question).text.toString()).isEqualTo("Question?")
-  }
-
-  @Test
-  fun `shows instructions`() {
-    view.bind(
-      Questionnaire.QuestionnaireItemComponent().apply {
-        item =
-          listOf(
-            Questionnaire.QuestionnaireItemComponent().apply {
-              linkId = "nested-display-question"
-              text = "subtitle text"
-              extension = listOf(displayCategoryExtensionWithInstructionsCode)
-              type = Questionnaire.QuestionnaireItemType.DISPLAY
-            }
-          )
-      }
-    )
-
-    assertThat(view.findViewById<TextView>(R.id.hint).isVisible).isTrue()
-    assertThat(view.findViewById<TextView>(R.id.hint).text.toString()).isEqualTo("subtitle text")
-  }
-
-  @Test
-  fun `hides instructions`() {
-    view.bind(
-      Questionnaire.QuestionnaireItemComponent().apply {
-        item =
-          listOf(
-            Questionnaire.QuestionnaireItemComponent().apply {
-              linkId = "nested-display-question"
-              type = Questionnaire.QuestionnaireItemType.DISPLAY
-            }
-          )
-      }
-    )
-
-    assertThat(view.findViewById<TextView>(R.id.hint).visibility).isEqualTo(View.GONE)
-  }
-
-  @Test
-  fun `shows headerItem view`() {
-    view.bind(
-      Questionnaire.QuestionnaireItemComponent().apply {
-        item =
-          listOf(
-            Questionnaire.QuestionnaireItemComponent().apply {
-              linkId = "nested-display-question"
-              text = "subtitle text"
-              extension = listOf(displayCategoryExtensionWithInstructionsCode)
-              type = Questionnaire.QuestionnaireItemType.DISPLAY
-            }
-          )
-      }
-    )
-
-    assertThat(view.visibility).isEqualTo(View.VISIBLE)
-  }
-
-  @Test
-  fun shouldHideHeaderView() {
-    view.bind(Questionnaire.QuestionnaireItemComponent())
-
-    assertThat(view.visibility).isEqualTo(View.GONE)
-  }
-
-  @Test
-  fun `shows * at the end of question text`() {
-    view.bind(
-      Questionnaire.QuestionnaireItemComponent().apply {
-        text = "Question?"
-        required = true
-      }
-    )
-
-    assertThat(view.findViewById<TextView>(R.id.question).text.toString()).isEqualTo("Question? *")
-  }
-
-  @Test
-  fun `does not show * at the end of question text`() {
-    view.bind(Questionnaire.QuestionnaireItemComponent().apply { text = "Question?" })
-
-    assertThat(view.findViewById<TextView>(R.id.question).text.toString()).isEqualTo("Question?")
-  }
-
-  @Test
-  fun `shows * even if question text is missing`() {
-    view.bind(Questionnaire.QuestionnaireItemComponent().apply { required = true })
-
-    assertThat(view.findViewById<TextView>(R.id.question).text.toString())
-      .isEqualTo(view.context.applicationContext.resources.getString(R.string.space_asterisk))
-  }
-
-  private val displayCategoryExtensionWithInstructionsCode =
-    Extension().apply {
-      url = EXTENSION_DISPLAY_CATEGORY_URL
-      setValue(
-        CodeableConcept().apply {
-          coding =
-            listOf(
-              Coding().apply {
-                code = INSTRUCTIONS
-                system = EXTENSION_DISPLAY_CATEGORY_SYSTEM
-              }
-            )
-        }
-      )
-    }
-
-  private val itemControlExtensionWithHelpCode =
-    Extension().apply {
-      url = EXTENSION_ITEM_CONTROL_URL
-      setValue(
-        CodeableConcept().apply {
-          coding =
-            listOf(
-              Coding().apply {
-                code = DisplayItemControlType.HELP.extensionCode
-                system = EXTENSION_ITEM_CONTROL_SYSTEM
-              }
-            )
-        }
-      )
-    }
-}
diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
index c28fc74683..ff6de7a675 100644
--- a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
+++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
@@ -20,16 +20,22 @@ import android.app.Application
 import android.view.View
 import android.widget.FrameLayout
 import android.widget.TextView
+import androidx.core.view.isVisible
 import androidx.test.core.app.ApplicationProvider
 import com.google.android.fhir.datacapture.DisplayItemControlType
+import com.google.android.fhir.datacapture.EXTENSION_DISPLAY_CATEGORY_SYSTEM
+import com.google.android.fhir.datacapture.EXTENSION_DISPLAY_CATEGORY_URL
 import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_SYSTEM
 import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_URL
+import com.google.android.fhir.datacapture.INSTRUCTIONS
 import com.google.android.fhir.datacapture.R
 import com.google.android.fhir.datacapture.validation.Valid
 import com.google.android.material.divider.MaterialDivider
 import com.google.common.truth.Truth.assertThat
 import org.hl7.fhir.r4.model.BooleanType
 import org.hl7.fhir.r4.model.CodeableConcept
+import org.hl7.fhir.r4.model.Coding
+import org.hl7.fhir.r4.model.Extension
 import org.hl7.fhir.r4.model.Questionnaire
 import org.hl7.fhir.r4.model.QuestionnaireResponse
 import org.junit.Test
@@ -329,4 +335,206 @@ class QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest {
     assertThat(viewHolder.itemView.findViewById<View>(R.id.error_icon_in_review_mode).visibility)
       .isEqualTo(View.GONE)
   }
+
+  @Test
+  fun `shows prefix text`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply { prefix = "Prefix?" },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.prefix).isVisible).isTrue()
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.prefix).text.toString())
+      .isEqualTo("Prefix?")
+  }
+
+  @Test
+  fun `hides prefix text`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply { prefix = "" },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.prefix).isVisible).isFalse()
+  }
+
+  @Test
+  fun `shows question text`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply {
+          repeats = true
+          text = "Question?"
+        },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.question).text.toString())
+      .isEqualTo("Question?")
+  }
+
+  @Test
+  fun `shows instructions`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply {
+          item =
+            listOf(
+              Questionnaire.QuestionnaireItemComponent().apply {
+                linkId = "nested-display-question"
+                text = "subtitle text"
+                extension = listOf(displayCategoryExtensionWithInstructionsCode)
+                type = Questionnaire.QuestionnaireItemType.DISPLAY
+              }
+            )
+        },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.hint).isVisible).isTrue()
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.hint).text.toString())
+      .isEqualTo("subtitle text")
+  }
+
+  @Test
+  fun `hides instructions`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply {
+          item =
+            listOf(
+              Questionnaire.QuestionnaireItemComponent().apply {
+                linkId = "nested-display-question"
+                type = Questionnaire.QuestionnaireItemType.DISPLAY
+              }
+            )
+        },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.hint).visibility)
+      .isEqualTo(View.GONE)
+  }
+
+  @Test
+  fun `shows headerItem view`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply {
+          item =
+            listOf(
+              Questionnaire.QuestionnaireItemComponent().apply {
+                linkId = "nested-display-question"
+                text = "subtitle text"
+                extension = listOf(displayCategoryExtensionWithInstructionsCode)
+                type = Questionnaire.QuestionnaireItemType.DISPLAY
+              }
+            )
+        },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.visibility).isEqualTo(View.VISIBLE)
+  }
+
+  @Test
+  fun `hides headerItem view`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent(),
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.question).visibility)
+      .isEqualTo(View.GONE)
+  }
+
+  @Test
+  fun `shows * at the end of question text`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply {
+          text = "Question?"
+          required = true
+        },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.question).text.toString())
+      .isEqualTo("Question? *")
+  }
+
+  @Test
+  fun `does not show * at the end of question text`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply { text = "Question?" },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.question).text.toString())
+      .isEqualTo("Question?")
+  }
+
+  @Test
+  fun `shows * even if question text is missing`() {
+    viewHolder.bind(
+      QuestionnaireItemViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply { required = true },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.question).text.toString())
+      .isEqualTo(
+        viewHolder.itemView.context.applicationContext.resources.getString(R.string.space_asterisk)
+      )
+  }
+
+  private val displayCategoryExtensionWithInstructionsCode =
+    Extension().apply {
+      url = EXTENSION_DISPLAY_CATEGORY_URL
+      setValue(
+        CodeableConcept().apply {
+          coding =
+            listOf(
+              Coding().apply {
+                code = INSTRUCTIONS
+                system = EXTENSION_DISPLAY_CATEGORY_SYSTEM
+              }
+            )
+        }
+      )
+    }
 }

From fa9258088bc3cf273194b11242b2b4c42dcbccdd Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Tue, 10 Jan 2023 11:55:54 +0530
Subject: [PATCH 09/18] Address review comments.

---
 .../fhir/datacapture/QuestionnaireFragment.kt |  5 +-
 .../views/QuestionnaireGroupTypeHeaderView.kt | 10 +--
 .../views/QuestionnaireItemHeaderView.kt      | 10 +--
 .../views/QuestionnaireItemHeaderViewUtil.kt  | 58 ++++-------------
 ...eQuestionAnswerDisplayViewHolderFactory.kt | 65 +++++++------------
 .../views/QuestionnaireItemViewItem.kt        |  2 +-
 ...naire_item_simple_question_answer_view.xml | 24 ++++---
 datacapture/src/main/res/values/attrs.xml     | 13 +++-
 datacapture/src/main/res/values/dimens.xml    |  3 +
 datacapture/src/main/res/values/styles.xml    | 30 +++++++--
 ...stionAnswerDisplayViewHolderFactoryTest.kt |  5 +-
 11 files changed, 104 insertions(+), 121 deletions(-)

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
index 56aac325ee..a1a17575dc 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
@@ -277,10 +277,7 @@ open class QuestionnaireFragment : Fragment() {
      */
     const val EXTRA_QUESTIONNAIRE_RESPONSE_JSON_URI = "questionnaire-response-uri"
 
-    /**
-     * A [Boolean] extra to control if a review page is shown. By default value is false, it will be
-     * shown at the end of the questionnaire if value is true.
-     */
+    /** A [Boolean] extra to enable the review page. Default to false. */
     const val EXTRA_ENABLE_REVIEW_PAGE = "enable-review-page"
 
     /**
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireGroupTypeHeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireGroupTypeHeaderView.kt
index 7519c397fe..bb3a0cfd9d 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireGroupTypeHeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireGroupTypeHeaderView.kt
@@ -37,15 +37,15 @@ internal class QuestionnaireGroupTypeHeaderView(context: Context, attrs: Attribu
     val prefix = findViewById<TextView>(R.id.prefix)
     val question = findViewById<TextView>(R.id.question)
     val hint = findViewById<TextView>(R.id.hint)
-    initHelpButton(
-      findViewById(R.id.helpButton),
-      findViewById(R.id.helpCardView),
-      findViewById(R.id.helpText),
+    initHelpViews(
+      helpButton = findViewById(R.id.helpButton),
+      helpCardView = findViewById(R.id.helpCardView),
+      helpTextView = findViewById(R.id.helpText),
       questionnaireItem
     )
     prefix.updateTextAndVisibility(questionnaireItem.localizedPrefixSpanned)
     updateQuestionText(question, questionnaireItem)
     hint.updateTextAndVisibility(questionnaireItem.localizedInstructionsSpanned)
-    visibility = getViewGroupVisibility(prefix, question, hint)
+    visibility = headerViewVisibility(prefix, question, hint)
   }
 }
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
index 2003bd165d..5ec5fd5bbe 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
@@ -43,15 +43,15 @@ internal class QuestionnaireItemHeaderView(context: Context, attrs: AttributeSet
     prefix.updateTextAndVisibility(questionnaireItem.localizedPrefixSpanned)
     updateQuestionText(question, questionnaireItem)
     hint.updateTextAndVisibility(questionnaireItem.localizedInstructionsSpanned)
-    initHelpButton(
-      findViewById(R.id.helpButton),
-      findViewById(R.id.helpCardView),
-      findViewById(R.id.helpText),
+    initHelpViews(
+      helpButton = findViewById(R.id.helpButton),
+      helpCardView = findViewById(R.id.helpCardView),
+      helpTextView = findViewById(R.id.helpText),
       questionnaireItem
     )
     //   Make the entire view GONE if there is nothing to show. This is to avoid an empty row in the
     // questionnaire.
-    visibility = getViewGroupVisibility(prefix, question, hint)
+    visibility = headerViewVisibility(prefix, question, hint)
   }
 
   /**
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
index df05faf12d..c2d147ee8b 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
@@ -22,7 +22,6 @@ import android.text.SpannableStringBuilder
 import android.text.Spanned
 import android.text.style.ForegroundColorSpan
 import android.util.TypedValue
-import android.view.View
 import android.view.View.GONE
 import android.view.View.VISIBLE
 import android.widget.Button
@@ -39,8 +38,7 @@ import org.hl7.fhir.r4.model.Questionnaire
 /**
  * Updates textview [R.id.question] with
  * [Questionnaire.QuestionnaireItemComponent.localizedTextSpanned] text and `*` if
- * [Questionnaire.QuestionnaireItemComponent.required] is true. And applies [R.attr.colorError] to
- * `*`.
+ * [Questionnaire.QuestionnaireItemComponent.required] is true.
  */
 internal fun updateQuestionText(
   questionTextView: TextView,
@@ -49,10 +47,7 @@ internal fun updateQuestionText(
   val builder = SpannableStringBuilder()
   questionnaireItem.localizedTextSpanned?.let { builder.append(it) }
   if (questionnaireItem.required) {
-    builder.appendWithSpan(
-      questionTextView.context.applicationContext.getString(R.string.space_asterisk),
-      questionTextView.context.getColorFromAttr(R.attr.colorError)
-    )
+    builder.append(questionTextView.context.applicationContext.getString(R.string.space_asterisk))
   }
   questionTextView.updateTextAndVisibility(builder)
 }
@@ -68,45 +63,17 @@ internal fun TextView.updateTextAndVisibility(localizedText: Spanned? = null) {
 }
 
 /** Returns [VISIBLE] if any of the [view] is visible, else returns [GONE]. */
-internal fun getViewGroupVisibility(vararg view: TextView): Int {
+internal fun headerViewVisibility(vararg view: TextView): Int {
   if (view.any { it.visibility == VISIBLE }) {
     return VISIBLE
   }
   return GONE
 }
 
-internal fun initHelpButton(
-  view: View,
-  questionnaireItem: Questionnaire.QuestionnaireItemComponent
-) {
-  val helpButton = view.findViewById<Button>(R.id.helpButton)
-  helpButton.visibility =
-    if (questionnaireItem.hasHelpButton) {
-      VISIBLE
-    } else {
-      GONE
-    }
-  val helpCardView = view.findViewById<MaterialCardView>(R.id.helpCardView)
-  var isHelpCardViewVisible = false
-  helpButton.setOnClickListener {
-    if (isHelpCardViewVisible) {
-      isHelpCardViewVisible = false
-      helpCardView.visibility = GONE
-    } else {
-      isHelpCardViewVisible = true
-      helpCardView.visibility = VISIBLE
-    }
-  }
-
-  view
-    .findViewById<TextView>(R.id.helpText)
-    .updateTextAndVisibility(questionnaireItem.localizedHelpSpanned)
-}
-
-internal fun initHelpButton(
+internal fun initHelpViews(
   helpButton: Button,
   helpCardView: MaterialCardView,
-  helpText: TextView,
+  helpTextView: TextView,
   questionnaireItem: Questionnaire.QuestionnaireItemComponent
 ) {
   helpButton.visibility =
@@ -115,17 +82,14 @@ internal fun initHelpButton(
     } else {
       GONE
     }
-  var isHelpCardViewVisible = false
   helpButton.setOnClickListener {
-    if (isHelpCardViewVisible) {
-      isHelpCardViewVisible = false
-      helpCardView.visibility = GONE
-    } else {
-      isHelpCardViewVisible = true
-      helpCardView.visibility = VISIBLE
-    }
+    helpCardView.visibility =
+      when (helpCardView.visibility) {
+        VISIBLE -> GONE
+        else -> VISIBLE
+      }
   }
-  helpText.updateTextAndVisibility(questionnaireItem.localizedHelpSpanned)
+  helpTextView.updateTextAndVisibility(questionnaireItem.localizedHelpSpanned)
 }
 
 private fun SpannableStringBuilder.appendWithSpan(value: String, @ColorInt color: Int) {
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
index 07a60d6268..d8057e1a1d 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
@@ -21,7 +21,6 @@ import android.view.View.GONE
 import android.view.View.VISIBLE
 import android.widget.TextView
 import androidx.constraintlayout.widget.ConstraintLayout
-import androidx.core.graphics.ColorUtils
 import com.google.android.fhir.datacapture.R
 import com.google.android.fhir.datacapture.localizedFlyoverSpanned
 import com.google.android.fhir.datacapture.localizedInstructionsSpanned
@@ -40,8 +39,8 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
   override fun getQuestionnaireItemViewHolderDelegate() =
     object : QuestionnaireItemViewHolderDelegate {
       private lateinit var flyOverTextView: TextView
-      private lateinit var answerTextView: TextView
-      private lateinit var errorIcon: View
+      private lateinit var notAnsweredView: View
+      private lateinit var answerView: TextView
       private lateinit var divider: MaterialDivider
       private lateinit var prefix: TextView
       private lateinit var question: TextView
@@ -52,12 +51,12 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
       override fun init(itemView: View) {
         header = itemView.findViewById(R.id.header)
         flyOverTextView = itemView.findViewById(R.id.flyover_text_view)
-        answerTextView = itemView.findViewById(R.id.answer_text_view)
         divider = itemView.findViewById(R.id.text_divider)
-        errorIcon = itemView.findViewById(R.id.error_icon_in_review_mode)
         prefix = itemView.findViewById(R.id.prefix)
         question = itemView.findViewById(R.id.question)
         hint = itemView.findViewById(R.id.hint)
+        notAnsweredView = itemView.findViewById(R.id.not_answered_view)
+        answerView = itemView.findViewById(R.id.answer_text_view)
       }
 
       override fun bind(questionnaireItemViewItem: QuestionnaireItemViewItem) {
@@ -68,7 +67,7 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
         hint.updateTextAndVisibility(
           questionnaireItemViewItem.questionnaireItem.localizedInstructionsSpanned
         )
-        header.visibility = getViewGroupVisibility(prefix, question, hint)
+        header.visibility = headerViewVisibility(prefix, question, hint)
 
         val localizedFlyoverSpanned =
           questionnaireItemViewItem.questionnaireItem.localizedFlyoverSpanned
@@ -82,36 +81,31 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
           text = localizedFlyoverSpanned
         }
 
-        answerTextView.apply {
-          visibility =
-            when (questionnaireItemViewItem.questionnaireItem.type) {
-              Questionnaire.QuestionnaireItemType.GROUP,
-              Questionnaire.QuestionnaireItemType.DISPLAY -> GONE
-              else -> VISIBLE
-            }
-          text = questionnaireItemViewItem.answerString(context)
-          setTextColorAsPerAnswer()
-        }
-
-        errorIcon.visibility =
-          when (questionnaireItemViewItem.questionnaireItem.type) {
-            Questionnaire.QuestionnaireItemType.GROUP,
-            Questionnaire.QuestionnaireItemType.DISPLAY -> {
-              GONE
-            }
-            else -> {
-              if (questionnaireItemViewItem.hasAnswerString) {
-                GONE
-              } else {
-                VISIBLE
-              }
+        when (questionnaireItemViewItem.questionnaireItem.type) {
+          Questionnaire.QuestionnaireItemType.GROUP,
+          Questionnaire.QuestionnaireItemType.DISPLAY -> {
+            notAnsweredView.visibility = GONE
+            answerView.visibility = GONE
+          }
+          else -> {
+            if (questionnaireItemViewItem.hasAnswer) {
+              notAnsweredView.visibility = GONE
+              answerView.visibility = VISIBLE
+              answerView.text = questionnaireItemViewItem.answerString(answerView.context)
+            } else {
+              notAnsweredView.visibility = VISIBLE
+              answerView.visibility = GONE
+              notAnsweredView.findViewById<TextView>(R.id.error_text_view).text =
+                questionnaireItemViewItem.answerString(answerView.context)
             }
           }
+        }
 
         divider.visibility =
           if (header.visibility == VISIBLE ||
               flyOverTextView.visibility == VISIBLE ||
-              answerTextView.visibility == VISIBLE
+              answerView.visibility == VISIBLE ||
+              notAnsweredView.visibility == VISIBLE
           ) {
             VISIBLE
           } else {
@@ -122,16 +116,5 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
       override fun displayValidationResult(validationResult: ValidationResult) {}
 
       override fun setReadOnly(isReadOnly: Boolean) {}
-
-      private fun TextView.setTextColorAsPerAnswer() =
-        if (questionnaireItemViewItem.hasAnswerString) {
-          val color = context.getColorFromAttr(android.R.attr.textColor)
-          setTextColor(ColorUtils.setAlphaComponent(color, TEXT_COLOR_ALPHA))
-        } else {
-          val color = context.getColorFromAttr(R.attr.colorError)
-          setTextColor(color)
-        }
     }
 }
-
-private const val TEXT_COLOR_ALPHA = 0x80 // 50%
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt
index 3574c075c5..d4c7b63756 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt
@@ -150,7 +150,7 @@ data class QuestionnaireItemViewItem(
     return questionnaireResponseItem.answer.joinToString { it.value.displayString(context) }
   }
 
-  internal val hasAnswerString: Boolean
+  internal val hasAnswer: Boolean
     get() = questionnaireResponseItem.hasAnswer()
 
   fun isAnswerOptionSelected(
diff --git a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
index b2a7dbb1e4..874162dd4d 100644
--- a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
+++ b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
@@ -61,7 +61,6 @@
             app:layout_constraintStart_toStartOf="parent"
         />
     </androidx.constraintlayout.widget.ConstraintLayout>
-
     <TextView
         android:id="@+id/flyover_text_view"
         style="?attr/questionnaireSubtitleTextStyle"
@@ -70,32 +69,39 @@
     />
     <LinearLayout
         xmlns:android="http://schemas.android.com/apk/res/android"
+        android:id="@+id/not_answered_view"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_marginTop="@dimen/item_margin_vertical"
         android:orientation="horizontal"
         android:gravity="center_vertical"
     >
-    <TextView
+        <View
             android:id="@+id/error_icon_in_review_mode"
-            android:layout_width="20dp"
-            android:layout_height="20dp"
+            android:layout_width="@dimen/error_icon_width"
+            android:layout_height="@dimen/error_icon_height"
             android:layout_marginEnd="@dimen/error_icon_margin_end"
             android:background="@drawable/ic_error_48px"
             android:backgroundTint="?attr/colorError"
-            android:visibility="gone"
         />
 
-    <TextView
-            android:id="@+id/answer_text_view"
-            style="?attr/questionnaireAnswerTextStyle"
+        <TextView
+            android:id="@+id/error_text_view"
+            style="?attr/questionnaireReviewModeNotAnsweredTextStyle"
+            android:textColor="?attr/colorError"
             android:layout_width="wrap_content"
             android:layout_height="wrap_content"
         />
     </LinearLayout>
-
+    <TextView
+        android:id="@+id/answer_text_view"
+        style="?attr/questionnaireReviewModeAnswerTextStyle"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+    />
     <com.google.android.material.divider.MaterialDivider
         android:id="@+id/text_divider"
+        style="?attr/questionnaireReviewModeItemDividerStyle"
         android:layout_height="wrap_content"
         android:layout_marginTop="@dimen/item_margin_vertical"
         android:layout_width="match_parent"
diff --git a/datacapture/src/main/res/values/attrs.xml b/datacapture/src/main/res/values/attrs.xml
index 321f0a3988..572cc5b48c 100644
--- a/datacapture/src/main/res/values/attrs.xml
+++ b/datacapture/src/main/res/values/attrs.xml
@@ -30,12 +30,19 @@
     <attr name="questionnaireHelpTextStyle" format="reference" />
     <!--   Style for help button -->
     <attr name="questionnaireHelpButtonStyle" format="reference" />
-    <!--   Style for answer text. -->
-    <attr name="questionnaireAnswerTextStyle" format="reference" />
-
     <!--   Style for question text in review mode -->
     <attr name="questionnaireReviewModeQuestionTextStyle" format="reference" />
 
+    <!--   Style for answer text. -->
+    <attr name="questionnaireReviewModeAnswerTextStyle" format="reference" />
+    <!--   Style for not answered text. -->
+    <attr
+        name="questionnaireReviewModeNotAnsweredTextStyle"
+        format="reference"
+    />
+
+    <attr name="questionnaireReviewModeItemDividerStyle" format="reference" />
+
     <!--   Style for radio buttons. -->
     <attr name="questionnaireRadioButtonStyle" format="reference" />
     <!--   Style for checkbox buttons. -->
diff --git a/datacapture/src/main/res/values/dimens.xml b/datacapture/src/main/res/values/dimens.xml
index c3a10b0414..d2f3f442a2 100644
--- a/datacapture/src/main/res/values/dimens.xml
+++ b/datacapture/src/main/res/values/dimens.xml
@@ -44,4 +44,7 @@
     <!--  dimens for padding between text and icon in the radio button  -->
     <dimen name="padding_between_text_and_icon">16dp</dimen>
     <dimen name="padding_after_text">24dp</dimen>
+    <!--  dimens for error icon width and height on review summary page.  -->
+    <dimen name="error_icon_width">20dp</dimen>
+    <dimen name="error_icon_height">20dp</dimen>
 </resources>
diff --git a/datacapture/src/main/res/values/styles.xml b/datacapture/src/main/res/values/styles.xml
index 4a3d25456f..7f31ba5c3b 100644
--- a/datacapture/src/main/res/values/styles.xml
+++ b/datacapture/src/main/res/values/styles.xml
@@ -60,8 +60,14 @@
             name="questionnaireHelpButtonStyle"
         >@style/Widget.MaterialComponents.Button.TextButton.Icon</item>
         <item
-            name="questionnaireAnswerTextStyle"
-        >@style/Questionnaire.AnswerStyle</item>
+            name="questionnaireReviewModeAnswerTextStyle"
+        >@style/Questionnaire.ReviewModeAnswerStyle</item>
+      <item
+            name="questionnaireReviewModeNotAnsweredTextStyle"
+        >@style/Questionnaire.ReviewModeNotAnsweredTextStyle</item>
+      <item
+            name="questionnaireReviewModeItemDividerStyle"
+        >@style/Questionnaire.ReviewModeMaterialDividerStyle</item>
         <item
             name="questionnaireCheckBoxStyle"
         >@style/Questionnaire.CheckBoxStyle
@@ -143,9 +149,25 @@
     </style>
 
     <style
-        name="Questionnaire.AnswerStyle"
+        name="Questionnaire.ReviewModeAnswerStyle"
+        parent="TextAppearance.Material3.BodyMedium"
+    >
+      <item name="android:alpha">0.5</item>
+    </style>
+
+    <style
+        name="Questionnaire.ReviewModeNotAnsweredTextStyle"
         parent="TextAppearance.Material3.BodyMedium"
-    />
+    >
+         <item name="android:textColor">?attr/colorError</item>
+    </style>
+
+    <style
+        name="Questionnaire.ReviewModeMaterialDividerStyle"
+        parent="Widget.Material3.MaterialDivider"
+    >
+      <item name="android:alpha">0.5</item>
+    </style>
 
     <style
         name="Questionnaire.ErrorStyle"
diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
index ff6de7a675..7ec2559246 100644
--- a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
+++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
@@ -19,6 +19,7 @@ package com.google.android.fhir.datacapture.views
 import android.app.Application
 import android.view.View
 import android.widget.FrameLayout
+import android.widget.LinearLayout
 import android.widget.TextView
 import androidx.core.view.isVisible
 import androidx.test.core.app.ApplicationProvider
@@ -174,7 +175,7 @@ class QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest {
       )
     )
 
-    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.answer_text_view).text)
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.error_text_view).text)
       .isEqualTo(
         ApplicationProvider.getApplicationContext<Application>().getString(R.string.not_answered)
       )
@@ -332,7 +333,7 @@ class QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest {
       )
     )
 
-    assertThat(viewHolder.itemView.findViewById<View>(R.id.error_icon_in_review_mode).visibility)
+    assertThat(viewHolder.itemView.findViewById<LinearLayout>(R.id.not_answered_view).visibility)
       .isEqualTo(View.GONE)
   }
 

From 87721624e1dbdebf14530f81cba812c619f11a68 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Tue, 10 Jan 2023 15:33:07 +0530
Subject: [PATCH 10/18] Address review comment.

---
 .../fhir/catalog/DemoQuestionnaireFragment.kt |  4 +--
 .../views/QuestionnaireItemHeaderViewUtil.kt  | 26 +------------------
 ...eQuestionAnswerDisplayViewHolderFactory.kt | 22 +++++++++++-----
 ...naire_item_simple_question_answer_view.xml |  1 -
 datacapture/src/main/res/values/attrs.xml     |  6 ++---
 5 files changed, 20 insertions(+), 39 deletions(-)

diff --git a/catalog/src/main/java/com/google/android/fhir/catalog/DemoQuestionnaireFragment.kt b/catalog/src/main/java/com/google/android/fhir/catalog/DemoQuestionnaireFragment.kt
index 9bc1aee22f..0cb1bcb7e7 100644
--- a/catalog/src/main/java/com/google/android/fhir/catalog/DemoQuestionnaireFragment.kt
+++ b/catalog/src/main/java/com/google/android/fhir/catalog/DemoQuestionnaireFragment.kt
@@ -40,7 +40,6 @@ import androidx.navigation.fragment.navArgs
 import com.google.android.fhir.catalog.ModalBottomSheetFragment.Companion.BUNDLE_ERROR_KEY
 import com.google.android.fhir.catalog.ModalBottomSheetFragment.Companion.REQUEST_ERROR_KEY
 import com.google.android.fhir.datacapture.QuestionnaireFragment
-import com.google.android.fhir.datacapture.QuestionnaireFragment.Companion.EXTRA_ENABLE_REVIEW_PAGE
 import com.google.android.fhir.datacapture.QuestionnaireFragment.Companion.SUBMIT_REQUEST_KEY
 import com.google.android.material.card.MaterialCardView
 import kotlinx.coroutines.launch
@@ -153,8 +152,7 @@ class DemoQuestionnaireFragment : Fragment() {
             args =
               bundleOf(
                 QuestionnaireFragment.EXTRA_QUESTIONNAIRE_JSON_STRING to
-                  viewModel.getQuestionnaireJson(),
-                EXTRA_ENABLE_REVIEW_PAGE to true
+                  viewModel.getQuestionnaireJson()
               )
           )
         }
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
index c2d147ee8b..0a157c6144 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
@@ -16,18 +16,12 @@
 
 package com.google.android.fhir.datacapture.views
 
-import android.content.Context
-import android.text.Spannable
 import android.text.SpannableStringBuilder
 import android.text.Spanned
-import android.text.style.ForegroundColorSpan
-import android.util.TypedValue
 import android.view.View.GONE
 import android.view.View.VISIBLE
 import android.widget.Button
 import android.widget.TextView
-import androidx.annotation.AttrRes
-import androidx.annotation.ColorInt
 import com.google.android.fhir.datacapture.R
 import com.google.android.fhir.datacapture.hasHelpButton
 import com.google.android.fhir.datacapture.localizedHelpSpanned
@@ -36,7 +30,7 @@ import com.google.android.material.card.MaterialCardView
 import org.hl7.fhir.r4.model.Questionnaire
 
 /**
- * Updates textview [R.id.question] with
+ * Updates textview [questionTextView] with
  * [Questionnaire.QuestionnaireItemComponent.localizedTextSpanned] text and `*` if
  * [Questionnaire.QuestionnaireItemComponent.required] is true.
  */
@@ -91,21 +85,3 @@ internal fun initHelpViews(
   }
   helpTextView.updateTextAndVisibility(questionnaireItem.localizedHelpSpanned)
 }
-
-private fun SpannableStringBuilder.appendWithSpan(value: String, @ColorInt color: Int) {
-  val start = length
-  append(value)
-  val end = length
-  setSpan(ForegroundColorSpan(color), start, end, Spannable.SPAN_EXCLUSIVE_EXCLUSIVE)
-}
-
-/**
- * Reads @ColorInt value from android resource attribute. e.g [attrColor] value is
- * [R.attr.colorError] to get the colorInt.
- */
-@ColorInt
-internal fun Context.getColorFromAttr(@AttrRes attrColor: Int): Int {
-  val typedValue = TypedValue()
-  theme.resolveAttribute(attrColor, typedValue, true)
-  return typedValue.data
-}
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
index d8057e1a1d..83b5ad2619 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
@@ -89,14 +89,9 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
           }
           else -> {
             if (questionnaireItemViewItem.hasAnswer) {
-              notAnsweredView.visibility = GONE
-              answerView.visibility = VISIBLE
-              answerView.text = questionnaireItemViewItem.answerString(answerView.context)
+              showAnswerView(questionnaireItemViewItem)
             } else {
-              notAnsweredView.visibility = VISIBLE
-              answerView.visibility = GONE
-              notAnsweredView.findViewById<TextView>(R.id.error_text_view).text =
-                questionnaireItemViewItem.answerString(answerView.context)
+              showNotAnsweredView(questionnaireItemViewItem)
             }
           }
         }
@@ -113,6 +108,19 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
           }
       }
 
+      private fun showAnswerView(questionnaireItemViewItem: QuestionnaireItemViewItem) {
+        notAnsweredView.visibility = GONE
+        answerView.visibility = VISIBLE
+        answerView.text = questionnaireItemViewItem.answerString(answerView.context)
+      }
+
+      private fun showNotAnsweredView(questionnaireItemViewItem: QuestionnaireItemViewItem) {
+        answerView.visibility = GONE
+        notAnsweredView.visibility = VISIBLE
+        notAnsweredView.findViewById<TextView>(R.id.error_text_view).text =
+          questionnaireItemViewItem.answerString(answerView.context)
+      }
+
       override fun displayValidationResult(validationResult: ValidationResult) {}
 
       override fun setReadOnly(isReadOnly: Boolean) {}
diff --git a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
index 874162dd4d..186e0f5579 100644
--- a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
+++ b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
@@ -77,7 +77,6 @@
         android:gravity="center_vertical"
     >
         <View
-            android:id="@+id/error_icon_in_review_mode"
             android:layout_width="@dimen/error_icon_width"
             android:layout_height="@dimen/error_icon_height"
             android:layout_marginEnd="@dimen/error_icon_margin_end"
diff --git a/datacapture/src/main/res/values/attrs.xml b/datacapture/src/main/res/values/attrs.xml
index 572cc5b48c..2f5fe62468 100644
--- a/datacapture/src/main/res/values/attrs.xml
+++ b/datacapture/src/main/res/values/attrs.xml
@@ -33,14 +33,14 @@
     <!--   Style for question text in review mode -->
     <attr name="questionnaireReviewModeQuestionTextStyle" format="reference" />
 
-    <!--   Style for answer text. -->
+    <!--   Style for answer text in review mode -->
     <attr name="questionnaireReviewModeAnswerTextStyle" format="reference" />
-    <!--   Style for not answered text. -->
+    <!--   Style for not answered text in review mode -->
     <attr
         name="questionnaireReviewModeNotAnsweredTextStyle"
         format="reference"
     />
-
+    <!--   Style for item divider in review mode -->
     <attr name="questionnaireReviewModeItemDividerStyle" format="reference" />
 
     <!--   Style for radio buttons. -->

From 81ac57e42064a9df93c789fb34bd13e27ed61cdc Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Tue, 10 Jan 2023 16:07:08 +0530
Subject: [PATCH 11/18] Fix failed test.

---
 ...eItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
index 7ec2559246..45247c53f3 100644
--- a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
+++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
@@ -297,7 +297,7 @@ class QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest {
   }
 
   @Test
-  fun `shows error icon if answer is not present`() {
+  fun `shows not answered view if answer is not present`() {
     viewHolder.bind(
       QuestionnaireItemViewItem(
         Questionnaire.QuestionnaireItemComponent().apply {
@@ -310,12 +310,12 @@ class QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest {
       )
     )
 
-    assertThat(viewHolder.itemView.findViewById<View>(R.id.error_icon_in_review_mode).visibility)
+    assertThat(viewHolder.itemView.findViewById<LinearLayout>(R.id.not_answered_view).visibility)
       .isEqualTo(View.VISIBLE)
   }
 
   @Test
-  fun `does not show error icon if answer is present`() {
+  fun `does not show not answered view if answer is present`() {
     viewHolder.bind(
       QuestionnaireItemViewItem(
         Questionnaire.QuestionnaireItemComponent().apply {

From 22697004681aaa45e180af42ba644b32e8c1c608 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Wed, 1 Mar 2023 15:14:13 +0530
Subject: [PATCH 12/18] Address review comments.

---
 .../fhir/datacapture/QuestionnaireFragment.kt |  5 ++-
 .../datacapture/QuestionnaireViewModel.kt     | 18 +++++++++-
 ...eQuestionAnswerDisplayViewHolderFactory.kt | 36 +++++++++++++------
 ...naire_item_simple_question_answer_view.xml | 15 ++++----
 ...stionAnswerDisplayViewHolderFactoryTest.kt |  4 +--
 5 files changed, 57 insertions(+), 21 deletions(-)

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
index b003545142..8d586ec3ca 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
@@ -108,7 +108,10 @@ open class QuestionnaireFragment : Fragment() {
     reviewModeEditButton.setOnClickListener { viewModel.setReviewMode(false) }
 
     val reviewModeButton = view.findViewById<View>(R.id.review_mode_button)
-    reviewModeButton.setOnClickListener { viewModel.setReviewMode(true) }
+    reviewModeButton.setOnClickListener {
+      viewModel.setReviewMode(true)
+      viewModel.validateQuestionnaireAndUpdateUI()
+    }
 
     questionnaireEditRecyclerView.adapter = questionnaireItemEditAdapter
     val linearLayoutManager = LinearLayoutManager(view.context)
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
index 884a800abe..4d381a9f14 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
@@ -94,6 +94,18 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
 
   /** The current questionnaire response as questions are being answered. */
   private val questionnaireResponse: QuestionnaireResponse
+  //
+  //  /**
+  //   * True if the user has tapped the next/previous pagination buttons on the current page. This
+  // is
+  //   * needed to avoid spewing validation errors before any questions are answered.
+  //   */
+  //  private var isPaginationButtonPressed = false
+  //
+  //  /** Forces response validation each time [getQuestionnaireAdapterItems] is called. */
+  //  private var hasPressedSubmitButton = false
+  //
+  //  private var hasPressedReviewButton = false
 
   init {
     when {
@@ -203,6 +215,8 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
   /** Forces response validation each time [getQuestionnaireAdapterItems] is called. */
   private var hasPressedSubmitButton = false
 
+  private var hasPressedReviewButton = false
+
   /**
    * Map of [QuestionnaireResponseItemAnswerComponent] for
    * [Questionnaire.QuestionnaireItemComponent]s that are disabled now. The answers will be used to
@@ -303,6 +317,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
       .also { result ->
         if (result.values.flatten().filterIsInstance<Invalid>().isNotEmpty()) {
           hasPressedSubmitButton = true
+          hasPressedReviewButton = true
           modificationCount.update { it + 1 }
         }
       }
@@ -619,7 +634,8 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
     val validationResult =
       if (modifiedQuestionnaireResponseItemSet.contains(questionnaireResponseItem) ||
           isPaginationButtonPressed ||
-          hasPressedSubmitButton
+          hasPressedSubmitButton ||
+          hasPressedReviewButton
       ) {
         QuestionnaireResponseItemValidator.validate(
           questionnaireItem,
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
index 83b5ad2619..061c213745 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
@@ -16,6 +16,7 @@
 
 package com.google.android.fhir.datacapture.views
 
+import android.util.Log
 import android.view.View
 import android.view.View.GONE
 import android.view.View.VISIBLE
@@ -25,6 +26,7 @@ import com.google.android.fhir.datacapture.R
 import com.google.android.fhir.datacapture.localizedFlyoverSpanned
 import com.google.android.fhir.datacapture.localizedInstructionsSpanned
 import com.google.android.fhir.datacapture.localizedPrefixSpanned
+import com.google.android.fhir.datacapture.validation.Invalid
 import com.google.android.fhir.datacapture.validation.ValidationResult
 import com.google.android.material.divider.MaterialDivider
 import org.hl7.fhir.r4.model.Questionnaire
@@ -39,7 +41,7 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
   override fun getQuestionnaireItemViewHolderDelegate() =
     object : QuestionnaireItemViewHolderDelegate {
       private lateinit var flyOverTextView: TextView
-      private lateinit var notAnsweredView: View
+      private lateinit var errorView: View
       private lateinit var answerView: TextView
       private lateinit var divider: MaterialDivider
       private lateinit var prefix: TextView
@@ -55,11 +57,17 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
         prefix = itemView.findViewById(R.id.prefix)
         question = itemView.findViewById(R.id.question)
         hint = itemView.findViewById(R.id.hint)
-        notAnsweredView = itemView.findViewById(R.id.not_answered_view)
+        errorView = itemView.findViewById(R.id.error_view)
         answerView = itemView.findViewById(R.id.answer_text_view)
       }
 
       override fun bind(questionnaireItemViewItem: QuestionnaireItemViewItem) {
+        Log.d(
+          "REVIEW",
+          (questionnaireItemViewItem.validationResult as? Invalid)
+            ?.getSingleStringValidationMessage()
+            ?: ""
+        )
         prefix.updateTextAndVisibility(
           questionnaireItemViewItem.questionnaireItem.localizedPrefixSpanned
         )
@@ -84,14 +92,22 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
         when (questionnaireItemViewItem.questionnaireItem.type) {
           Questionnaire.QuestionnaireItemType.GROUP,
           Questionnaire.QuestionnaireItemType.DISPLAY -> {
-            notAnsweredView.visibility = GONE
+            errorView.visibility = GONE
             answerView.visibility = GONE
           }
           else -> {
-            if (questionnaireItemViewItem.hasAnswer) {
-              showAnswerView(questionnaireItemViewItem)
+            //            if (questionnaireItemViewItem.hasAnswer) {
+            //              showAnswerView(questionnaireItemViewItem)
+            //            } else {
+            //              showNotAnsweredView(questionnaireItemViewItem)
+            //            }
+            answerView.text = questionnaireItemViewItem.answerString(answerView.context)
+            if (questionnaireItemViewItem.validationResult is Invalid) {
+              errorView.findViewById<TextView>(R.id.error_text_view).text =
+                questionnaireItemViewItem.validationResult.getSingleStringValidationMessage()
+              errorView.visibility = VISIBLE
             } else {
-              showNotAnsweredView(questionnaireItemViewItem)
+              errorView.visibility = GONE
             }
           }
         }
@@ -100,7 +116,7 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
           if (header.visibility == VISIBLE ||
               flyOverTextView.visibility == VISIBLE ||
               answerView.visibility == VISIBLE ||
-              notAnsweredView.visibility == VISIBLE
+              errorView.visibility == VISIBLE
           ) {
             VISIBLE
           } else {
@@ -109,15 +125,15 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
       }
 
       private fun showAnswerView(questionnaireItemViewItem: QuestionnaireItemViewItem) {
-        notAnsweredView.visibility = GONE
+        errorView.visibility = GONE
         answerView.visibility = VISIBLE
         answerView.text = questionnaireItemViewItem.answerString(answerView.context)
       }
 
       private fun showNotAnsweredView(questionnaireItemViewItem: QuestionnaireItemViewItem) {
         answerView.visibility = GONE
-        notAnsweredView.visibility = VISIBLE
-        notAnsweredView.findViewById<TextView>(R.id.error_text_view).text =
+        errorView.visibility = VISIBLE
+        errorView.findViewById<TextView>(R.id.error_text_view).text =
           questionnaireItemViewItem.answerString(answerView.context)
       }
 
diff --git a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
index 186e0f5579..6239b4372f 100644
--- a/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
+++ b/datacapture/src/main/res/layout/questionnaire_item_simple_question_answer_view.xml
@@ -67,14 +67,21 @@
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
     />
+    <TextView
+        android:id="@+id/answer_text_view"
+        style="?attr/questionnaireReviewModeAnswerTextStyle"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+    />
     <LinearLayout
         xmlns:android="http://schemas.android.com/apk/res/android"
-        android:id="@+id/not_answered_view"
+        android:id="@+id/error_view"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_marginTop="@dimen/item_margin_vertical"
         android:orientation="horizontal"
         android:gravity="center_vertical"
+        android:visibility="gone"
     >
         <View
             android:layout_width="@dimen/error_icon_width"
@@ -92,12 +99,6 @@
             android:layout_height="wrap_content"
         />
     </LinearLayout>
-    <TextView
-        android:id="@+id/answer_text_view"
-        style="?attr/questionnaireReviewModeAnswerTextStyle"
-        android:layout_width="wrap_content"
-        android:layout_height="wrap_content"
-    />
     <com.google.android.material.divider.MaterialDivider
         android:id="@+id/text_divider"
         style="?attr/questionnaireReviewModeItemDividerStyle"
diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
index adc84bc061..ed8b3d2c10 100644
--- a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
+++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest.kt
@@ -311,7 +311,7 @@ class QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest {
       )
     )
 
-    assertThat(viewHolder.itemView.findViewById<LinearLayout>(R.id.not_answered_view).visibility)
+    assertThat(viewHolder.itemView.findViewById<LinearLayout>(R.id.error_view).visibility)
       .isEqualTo(View.VISIBLE)
   }
 
@@ -334,7 +334,7 @@ class QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactoryTest {
       )
     )
 
-    assertThat(viewHolder.itemView.findViewById<LinearLayout>(R.id.not_answered_view).visibility)
+    assertThat(viewHolder.itemView.findViewById<LinearLayout>(R.id.error_view).visibility)
       .isEqualTo(View.GONE)
   }
 

From 04be654a06331d7c7b4457e538f08ce531d3ab6d Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Wed, 1 Mar 2023 15:30:44 +0530
Subject: [PATCH 13/18] Refactored the code

---
 .../fhir/datacapture/QuestionnaireViewModel.kt       | 12 ------------
 ...emSimpleQuestionAnswerDisplayViewHolderFactory.kt | 12 ------------
 2 files changed, 24 deletions(-)

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
index 4d381a9f14..8ecae607c6 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
@@ -94,18 +94,6 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
 
   /** The current questionnaire response as questions are being answered. */
   private val questionnaireResponse: QuestionnaireResponse
-  //
-  //  /**
-  //   * True if the user has tapped the next/previous pagination buttons on the current page. This
-  // is
-  //   * needed to avoid spewing validation errors before any questions are answered.
-  //   */
-  //  private var isPaginationButtonPressed = false
-  //
-  //  /** Forces response validation each time [getQuestionnaireAdapterItems] is called. */
-  //  private var hasPressedSubmitButton = false
-  //
-  //  private var hasPressedReviewButton = false
 
   init {
     when {
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
index 061c213745..8d89afa46c 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory.kt
@@ -16,7 +16,6 @@
 
 package com.google.android.fhir.datacapture.views
 
-import android.util.Log
 import android.view.View
 import android.view.View.GONE
 import android.view.View.VISIBLE
@@ -62,12 +61,6 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
       }
 
       override fun bind(questionnaireItemViewItem: QuestionnaireItemViewItem) {
-        Log.d(
-          "REVIEW",
-          (questionnaireItemViewItem.validationResult as? Invalid)
-            ?.getSingleStringValidationMessage()
-            ?: ""
-        )
         prefix.updateTextAndVisibility(
           questionnaireItemViewItem.questionnaireItem.localizedPrefixSpanned
         )
@@ -96,11 +89,6 @@ internal object QuestionnaireItemSimpleQuestionAnswerDisplayViewHolderFactory :
             answerView.visibility = GONE
           }
           else -> {
-            //            if (questionnaireItemViewItem.hasAnswer) {
-            //              showAnswerView(questionnaireItemViewItem)
-            //            } else {
-            //              showNotAnsweredView(questionnaireItemViewItem)
-            //            }
             answerView.text = questionnaireItemViewItem.answerString(answerView.context)
             if (questionnaireItemViewItem.validationResult is Invalid) {
               errorView.findViewById<TextView>(R.id.error_text_view).text =

From 5797150b567cfa9d11763e4fe0702a3822ec7734 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Wed, 1 Mar 2023 18:16:26 +0530
Subject: [PATCH 14/18] Refactore the file name.

---
 .../MoreHeaderViews.kt}                       | 18 +------------
 .../fhir/datacapture/views/GroupHeaderView.kt |  3 +++
 .../fhir/datacapture/views/HeaderView.kt      |  3 +++
 .../factories/ReviewViewHolderFactory.kt      | 25 ++++++-------------
 4 files changed, 14 insertions(+), 35 deletions(-)
 rename datacapture/src/main/java/com/google/android/fhir/datacapture/{views/QuestionnaireItemHeaderViewUtil.kt => extensions/MoreHeaderViews.kt} (74%)

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt
similarity index 74%
rename from datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
rename to datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt
index be64503b4f..6dcb5e4d12 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderViewUtil.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt
@@ -14,9 +14,8 @@
  * limitations under the License.
  */
 
-package com.google.android.fhir.datacapture.views
+package com.google.android.fhir.datacapture.extensions
 
-import android.text.SpannableStringBuilder
 import android.text.Spanned
 import android.view.View.GONE
 import android.view.View.VISIBLE
@@ -24,24 +23,9 @@ import android.widget.Button
 import android.widget.TextView
 import com.google.android.fhir.datacapture.hasHelpButton
 import com.google.android.fhir.datacapture.localizedHelpSpanned
-import com.google.android.fhir.datacapture.localizedTextSpanned
 import com.google.android.material.card.MaterialCardView
 import org.hl7.fhir.r4.model.Questionnaire
 
-/**
- * Updates textview [questionTextView] with
- * [Questionnaire.QuestionnaireItemComponent.localizedTextSpanned] text and `*` if
- * [Questionnaire.QuestionnaireItemComponent.required] is true.
- */
-internal fun updateQuestionText(
-  questionTextView: TextView,
-  questionnaireItem: Questionnaire.QuestionnaireItemComponent,
-) {
-  val builder = SpannableStringBuilder()
-  questionnaireItem.localizedTextSpanned?.let { builder.append(it) }
-  questionTextView.updateTextAndVisibility(builder)
-}
-
 internal fun TextView.updateTextAndVisibility(localizedText: Spanned? = null) {
   text = localizedText
   visibility =
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt
index 73b1f03d61..6e790ec9e8 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt
@@ -22,6 +22,9 @@ import android.view.LayoutInflater
 import android.widget.LinearLayout
 import android.widget.TextView
 import com.google.android.fhir.datacapture.R
+import com.google.android.fhir.datacapture.extensions.headerViewVisibility
+import com.google.android.fhir.datacapture.extensions.initHelpViews
+import com.google.android.fhir.datacapture.extensions.updateTextAndVisibility
 import com.google.android.fhir.datacapture.localizedInstructionsSpanned
 import com.google.android.fhir.datacapture.localizedPrefixSpanned
 import com.google.android.fhir.datacapture.localizedTextSpanned
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt
index 42da2cc590..70b332bdee 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt
@@ -22,6 +22,9 @@ import android.view.LayoutInflater
 import android.widget.LinearLayout
 import android.widget.TextView
 import com.google.android.fhir.datacapture.R
+import com.google.android.fhir.datacapture.extensions.headerViewVisibility
+import com.google.android.fhir.datacapture.extensions.initHelpViews
+import com.google.android.fhir.datacapture.extensions.updateTextAndVisibility
 import com.google.android.fhir.datacapture.localizedInstructionsSpanned
 import com.google.android.fhir.datacapture.localizedPrefixSpanned
 import com.google.android.fhir.datacapture.localizedTextSpanned
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt
index 7ed5822891..d4ab019eca 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt
@@ -22,14 +22,14 @@ import android.view.View.VISIBLE
 import android.widget.TextView
 import androidx.constraintlayout.widget.ConstraintLayout
 import com.google.android.fhir.datacapture.R
+import com.google.android.fhir.datacapture.extensions.headerViewVisibility
+import com.google.android.fhir.datacapture.extensions.updateTextAndVisibility
 import com.google.android.fhir.datacapture.localizedFlyoverSpanned
 import com.google.android.fhir.datacapture.localizedInstructionsSpanned
 import com.google.android.fhir.datacapture.localizedPrefixSpanned
+import com.google.android.fhir.datacapture.localizedTextSpanned
 import com.google.android.fhir.datacapture.validation.Invalid
 import com.google.android.fhir.datacapture.views.QuestionnaireViewItem
-import com.google.android.fhir.datacapture.views.headerViewVisibility
-import com.google.android.fhir.datacapture.views.updateQuestionText
-import com.google.android.fhir.datacapture.views.updateTextAndVisibility
 import com.google.android.material.divider.MaterialDivider
 import org.hl7.fhir.r4.model.Questionnaire
 
@@ -65,7 +65,9 @@ internal object ReviewViewHolderFactory : QuestionnaireItemViewHolderFactory(R.l
         prefix.updateTextAndVisibility(
           questionnaireViewItem.questionnaireItem.localizedPrefixSpanned
         )
-        updateQuestionText(question, questionnaireViewItem.questionnaireItem)
+        question.updateTextAndVisibility(
+          questionnaireViewItem.questionnaireItem.localizedTextSpanned
+        )
         hint.updateTextAndVisibility(
           questionnaireViewItem.questionnaireItem.localizedInstructionsSpanned
         )
@@ -82,7 +84,6 @@ internal object ReviewViewHolderFactory : QuestionnaireItemViewHolderFactory(R.l
             }
           text = localizedFlyoverSpanned
         }
-
         when (questionnaireViewItem.questionnaireItem.type) {
           Questionnaire.QuestionnaireItemType.GROUP,
           Questionnaire.QuestionnaireItemType.DISPLAY -> {
@@ -91,6 +92,7 @@ internal object ReviewViewHolderFactory : QuestionnaireItemViewHolderFactory(R.l
           }
           else -> {
             answerView.text = questionnaireViewItem.answerString(answerView.context)
+            answerView.visibility = VISIBLE
             if (questionnaireViewItem.validationResult is Invalid) {
               errorView.findViewById<TextView>(R.id.error_text_view).text =
                 questionnaireViewItem.validationResult.getSingleStringValidationMessage()
@@ -113,19 +115,6 @@ internal object ReviewViewHolderFactory : QuestionnaireItemViewHolderFactory(R.l
           }
       }
 
-      private fun showAnswerView(questionnaireItemViewItem: QuestionnaireViewItem) {
-        errorView.visibility = GONE
-        answerView.visibility = VISIBLE
-        answerView.text = questionnaireItemViewItem.answerString(answerView.context)
-      }
-
-      private fun showNotAnsweredView(questionnaireItemViewItem: QuestionnaireViewItem) {
-        answerView.visibility = GONE
-        errorView.visibility = VISIBLE
-        errorView.findViewById<TextView>(R.id.error_text_view).text =
-          questionnaireItemViewItem.answerString(answerView.context)
-      }
-
       override fun setReadOnly(isReadOnly: Boolean) {}
     }
 }

From 6cf983e8ce83aec65f74c55276ee03c6c269bc07 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Thu, 2 Mar 2023 11:02:40 +0530
Subject: [PATCH 15/18] Fix local unit tests.

---
 .../views/factories/ReviewViewHolderFactoryTest.kt    | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactoryTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactoryTest.kt
index b4bbee0d7f..e386985478 100644
--- a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactoryTest.kt
+++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactoryTest.kt
@@ -30,7 +30,7 @@ import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_SYSTEM
 import com.google.android.fhir.datacapture.EXTENSION_ITEM_CONTROL_URL
 import com.google.android.fhir.datacapture.INSTRUCTIONS
 import com.google.android.fhir.datacapture.R
-import com.google.android.fhir.datacapture.validation.NotValidated
+import com.google.android.fhir.datacapture.validation.Invalid
 import com.google.android.fhir.datacapture.validation.Valid
 import com.google.android.fhir.datacapture.views.QuestionnaireViewItem
 import com.google.android.material.divider.MaterialDivider
@@ -176,7 +176,7 @@ class ReviewViewHolderFactoryTest {
       )
     )
 
-    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.error_text_view).text)
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.answer_text_view).text)
       .isEqualTo(
         ApplicationProvider.getApplicationContext<Application>().getString(R.string.not_answered)
       )
@@ -304,15 +304,16 @@ class ReviewViewHolderFactoryTest {
         Questionnaire.QuestionnaireItemComponent().apply {
           linkId = "parent-question"
           type = Questionnaire.QuestionnaireItemType.BOOLEAN
+          required = true
         },
         QuestionnaireResponse.QuestionnaireResponseItemComponent(),
-        validationResult = NotValidated,
+        validationResult = Invalid(listOf("Missing answer for required field")),
         answersChangedCallback = { _, _, _, _ -> },
       )
     )
 
-    assertThat(viewHolder.itemView.findViewById<LinearLayout>(R.id.error_view).visibility)
-      .isEqualTo(View.VISIBLE)
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.error_text_view).text)
+      .isEqualTo("Missing answer for required field")
   }
 
   @Test

From 5c87587518857282ed120111e382f684e84906a8 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Thu, 2 Mar 2023 17:38:53 +0530
Subject: [PATCH 16/18] Address review comment.

---
 .../android/fhir/datacapture/QuestionnaireViewModel.kt   | 5 +----
 .../fhir/datacapture/extensions/MoreHeaderViews.kt       | 9 +++++++--
 .../android/fhir/datacapture/views/GroupHeaderView.kt    | 4 ++--
 .../google/android/fhir/datacapture/views/HeaderView.kt  | 4 ++--
 .../views/factories/ReviewViewHolderFactory.kt           | 4 ++--
 5 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
index 840b0bab5b..a2b08faf40 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
@@ -207,8 +207,6 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
   /** Forces response validation each time [getQuestionnaireAdapterItems] is called. */
   private var hasPressedSubmitButton = false
 
-  private var hasPressedReviewButton = false
-
   /**
    * Map of [QuestionnaireResponseItemAnswerComponent] for
    * [Questionnaire.QuestionnaireItemComponent]s that are disabled now. The answers will be used to
@@ -314,7 +312,6 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
       .also { result ->
         if (result.values.flatten().filterIsInstance<Invalid>().isNotEmpty()) {
           hasPressedSubmitButton = true
-          hasPressedReviewButton = true
           modificationCount.update { it + 1 }
         }
       }
@@ -616,7 +613,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
       if (modifiedQuestionnaireResponseItemSet.contains(questionnaireResponseItem) ||
           isPaginationButtonPressed ||
           hasPressedSubmitButton ||
-          hasPressedReviewButton
+          isInReviewModeFlow.value
       ) {
         QuestionnaireResponseItemValidator.validate(
           questionnaireItem,
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt
index 6dcb5e4d12..a0e80f273c 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreHeaderViews.kt
@@ -36,14 +36,19 @@ internal fun TextView.updateTextAndVisibility(localizedText: Spanned? = null) {
     }
 }
 
-/** Returns [VISIBLE] if any of the [view] is visible, else returns [GONE]. */
-internal fun headerViewVisibility(vararg view: TextView): Int {
+/** Returns [VISIBLE] if any of the [view] is visible, [GONE] otherwise. */
+internal fun getHeaderViewVisibility(vararg view: TextView): Int {
   if (view.any { it.visibility == VISIBLE }) {
     return VISIBLE
   }
   return GONE
 }
 
+/**
+ * Initializes the text for [helpTextView] with instructions on how to use the feature, and sets the
+ * visibility and click listener for the [helpButton] to allow users to access the help information
+ * and toggles the visibility for view [helpCardView].
+ */
 internal fun initHelpViews(
   helpButton: Button,
   helpCardView: MaterialCardView,
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt
index 6e790ec9e8..b149345343 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/GroupHeaderView.kt
@@ -22,7 +22,7 @@ import android.view.LayoutInflater
 import android.widget.LinearLayout
 import android.widget.TextView
 import com.google.android.fhir.datacapture.R
-import com.google.android.fhir.datacapture.extensions.headerViewVisibility
+import com.google.android.fhir.datacapture.extensions.getHeaderViewVisibility
 import com.google.android.fhir.datacapture.extensions.initHelpViews
 import com.google.android.fhir.datacapture.extensions.updateTextAndVisibility
 import com.google.android.fhir.datacapture.localizedInstructionsSpanned
@@ -50,6 +50,6 @@ internal class GroupHeaderView(context: Context, attrs: AttributeSet?) :
     prefix.updateTextAndVisibility(questionnaireItem.localizedPrefixSpanned)
     question.updateTextAndVisibility(questionnaireItem.localizedTextSpanned)
     hint.updateTextAndVisibility(questionnaireItem.localizedInstructionsSpanned)
-    visibility = headerViewVisibility(prefix, question, hint)
+    visibility = getHeaderViewVisibility(prefix, question, hint)
   }
 }
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt
index 70b332bdee..79fef23e70 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/HeaderView.kt
@@ -22,7 +22,7 @@ import android.view.LayoutInflater
 import android.widget.LinearLayout
 import android.widget.TextView
 import com.google.android.fhir.datacapture.R
-import com.google.android.fhir.datacapture.extensions.headerViewVisibility
+import com.google.android.fhir.datacapture.extensions.getHeaderViewVisibility
 import com.google.android.fhir.datacapture.extensions.initHelpViews
 import com.google.android.fhir.datacapture.extensions.updateTextAndVisibility
 import com.google.android.fhir.datacapture.localizedInstructionsSpanned
@@ -54,7 +54,7 @@ internal class HeaderView(context: Context, attrs: AttributeSet?) : LinearLayout
     )
     //   Make the entire view GONE if there is nothing to show. This is to avoid an empty row in the
     // questionnaire.
-    visibility = headerViewVisibility(prefix, question, hint)
+    visibility = getHeaderViewVisibility(prefix, question, hint)
   }
 
   /**
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt
index d4ab019eca..acb262ae40 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactory.kt
@@ -22,7 +22,7 @@ import android.view.View.VISIBLE
 import android.widget.TextView
 import androidx.constraintlayout.widget.ConstraintLayout
 import com.google.android.fhir.datacapture.R
-import com.google.android.fhir.datacapture.extensions.headerViewVisibility
+import com.google.android.fhir.datacapture.extensions.getHeaderViewVisibility
 import com.google.android.fhir.datacapture.extensions.updateTextAndVisibility
 import com.google.android.fhir.datacapture.localizedFlyoverSpanned
 import com.google.android.fhir.datacapture.localizedInstructionsSpanned
@@ -71,7 +71,7 @@ internal object ReviewViewHolderFactory : QuestionnaireItemViewHolderFactory(R.l
         hint.updateTextAndVisibility(
           questionnaireViewItem.questionnaireItem.localizedInstructionsSpanned
         )
-        header.visibility = headerViewVisibility(prefix, question, hint)
+        header.visibility = getHeaderViewVisibility(prefix, question, hint)
 
         val localizedFlyoverSpanned =
           questionnaireViewItem.questionnaireItem.localizedFlyoverSpanned

From 3c9f7ca951cc459a787dbba01f0f0f9ce0041d75 Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Thu, 2 Mar 2023 18:10:06 +0530
Subject: [PATCH 17/18] Add more unit tests.

---
 .../factories/ReviewViewHolderFactoryTest.kt  | 47 ++++++++++++++++++-
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactoryTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactoryTest.kt
index e386985478..56ee0409ee 100644
--- a/datacapture/src/test/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactoryTest.kt
+++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/views/factories/ReviewViewHolderFactoryTest.kt
@@ -298,7 +298,50 @@ class ReviewViewHolderFactoryTest {
   }
 
   @Test
-  fun `shows not answered view if answer is not present`() {
+  fun `shows answer text if question is answered`() {
+    viewHolder.bind(
+      QuestionnaireViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply {
+          linkId = "parent-question"
+          type = Questionnaire.QuestionnaireItemType.BOOLEAN
+          required = true
+        },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent()
+          .addAnswer(
+            QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+              value = BooleanType(true)
+            }
+          ),
+        validationResult = Valid,
+        answersChangedCallback = { _, _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.answer_text_view).text)
+      .isEqualTo("Yes")
+  }
+
+  @Test
+  fun `shows default text if question is not answered`() {
+    viewHolder.bind(
+      QuestionnaireViewItem(
+        Questionnaire.QuestionnaireItemComponent().apply {
+          linkId = "parent-question"
+          type = Questionnaire.QuestionnaireItemType.BOOLEAN
+          required = true
+        },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent(),
+        validationResult = Invalid(listOf("Missing answer for required field")),
+        answersChangedCallback = { _, _, _, _ -> },
+      )
+    )
+
+    assertThat(viewHolder.itemView.findViewById<TextView>(R.id.answer_text_view).text)
+      .isEqualTo("Not Answered")
+  }
+
+  @Test
+  fun `shows an error text if required question is not answered`() {
     viewHolder.bind(
       QuestionnaireViewItem(
         Questionnaire.QuestionnaireItemComponent().apply {
@@ -317,7 +360,7 @@ class ReviewViewHolderFactoryTest {
   }
 
   @Test
-  fun `does not show not answered view if answer is present`() {
+  fun `hides error view if answer is present`() {
     viewHolder.bind(
       QuestionnaireViewItem(
         Questionnaire.QuestionnaireItemComponent().apply {

From 9dde44f6f93dfdc5560140aac29be52f1e69c25b Mon Sep 17 00:00:00 2001
From: Santosh Pingle <spingle@google.com>
Date: Thu, 9 Mar 2023 11:19:30 +0530
Subject: [PATCH 18/18] Remove updateUI call from on review button click.

---
 .../google/android/fhir/datacapture/QuestionnaireFragment.kt | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
index b825a7d94f..9e9301dfc2 100644
--- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
+++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
@@ -126,10 +126,7 @@ class QuestionnaireFragment : Fragment() {
 
     val reviewModeButton =
       view.findViewById<View>(R.id.review_mode_button).apply {
-        setOnClickListener {
-          viewModel.setReviewMode(true)
-          viewModel.validateQuestionnaireAndUpdateUI()
-        }
+        setOnClickListener { viewModel.setReviewMode(true) }
       }
 
     questionnaireEditRecyclerView.adapter = questionnaireEditAdapter