Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slider Question functionality added #287

Merged
merged 15 commits into from
Mar 13, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
* Copyright 2020 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.widget.FrameLayout
import android.widget.TextView
import androidx.appcompat.view.ContextThemeWrapper
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import com.google.android.fhir.datacapture.R
import com.google.android.material.slider.Slider
import com.google.common.truth.Truth.assertThat
import com.google.fhir.r4.core.Integer
import com.google.fhir.r4.core.Questionnaire
import com.google.fhir.r4.core.QuestionnaireResponse
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith

@RunWith(AndroidJUnit4::class)
class QuestionnaireItemSliderViewHolderFactoryInstrumentedTest {
private lateinit var context: ContextThemeWrapper
private lateinit var parent: FrameLayout
private lateinit var viewHolder: QuestionnaireItemViewHolder

@Before
fun setUp() {
context = ContextThemeWrapper(
InstrumentationRegistry.getInstrumentation().targetContext,
R.style.Theme_MaterialComponents
)
parent = FrameLayout(context)
viewHolder = QuestionnaireItemSliderViewHolderFactory.create(parent)
}

@Test
fun shouldSetHeaderTextViewText() {
viewHolder.bind(
QuestionnaireItemViewItem(
Questionnaire.Item.newBuilder().apply {
text = com.google.fhir.r4.core.String.newBuilder().setValue("Question?").build()
}.build(),
QuestionnaireResponse.Item.newBuilder()
) {}
)

assertThat(viewHolder.itemView.findViewById<TextView>(R.id.slider_header).text)
.isEqualTo("Question?")
}

@Test
fun singleAnswerOrNull_noAnswer_shouldReturnNull() {
val questionnaireItemViewItem = QuestionnaireItemViewItem(
Questionnaire.Item.getDefaultInstance(),
QuestionnaireResponse.Item.newBuilder()
) {}

assertThat(questionnaireItemViewItem.singleAnswerOrNull).isNull()
}

@Test
fun singleAnswerOrNull_singleAnswer_shouldReturnSingleAnswer() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to write tests verifying that the slider is working as it is supposed to . Check the answerFalse_shouldSetCheckBoxUnchecked unit test in QuestionnaireItemCheckBoxViewHolderFactoryInstrumentedTest to get a better understanding. The assert statement should use the value returned from the viewHolder.findViewById(R.id.slider)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please comment again once done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

@jingtang10 jingtang10 Mar 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see https://github.com/google/android-fhir/blob/master/datacapture/src/androidTest/java/com/google/android/fhir/datacapture/views/QuestionnaireItemEditTextSingleLineViewHolderFactoryInstrumentedTest.kt. So there're two types of test cases, you have the ones that update the UI, and you have the ones that change the UI and update the questionnaire respoonse.

so you'll need to add at least another test case that tests the answer is set after you modify the slider.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fun singleAnswerOrNull_singleAnswer_shouldReturnSingleAnswer() {
fun shouldSetSliderValue() {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

viewHolder.bind(
QuestionnaireItemViewItem(
Questionnaire.Item.getDefaultInstance(),
QuestionnaireResponse.Item.newBuilder().apply {
addAnswer(
QuestionnaireResponse.Item.Answer.newBuilder().apply {
value = QuestionnaireResponse.Item.Answer.ValueX.newBuilder()
.setInteger(Integer.newBuilder().setValue(10).build())
.build()
}
)
}
) {}
)

assertThat(viewHolder.itemView.findViewById<Slider>(R.id.slider).value).isEqualTo(10)
}

@Test
fun singleAnswerOrNull_multipleAnswers_shouldReturnNull() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fun singleAnswerOrNull_multipleAnswers_shouldReturnNull() {
fun shouldSetSliderValueToDefault() {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

val questionnaireItemViewItem = QuestionnaireItemViewItem(
Questionnaire.Item.getDefaultInstance(),
QuestionnaireResponse.Item.newBuilder().apply {
addAnswer(
QuestionnaireResponse.Item.Answer.newBuilder().apply {
value = QuestionnaireResponse.Item.Answer.ValueX.newBuilder()
.setInteger(Integer.newBuilder().setValue(10).build())
.build()
}
)
addAnswer(
QuestionnaireResponse.Item.Answer.newBuilder().apply {
value = QuestionnaireResponse.Item.Answer.ValueX.newBuilder()
.setInteger(Integer.newBuilder().setValue(10).build())
.build()
}
)
}
) {}

assertThat(questionnaireItemViewItem.singleAnswerOrNull).isNull()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.google.fhir.r4.core.Questionnaire

internal const val ITEM_CONTROL_DROP_DOWN = "drop-down"
internal const val ITEM_CONTROL_RADIO_BUTTON = "radio-button"
internal const val ITEM_CONTROL_SLIDER = "slider"

internal const val EXTENSION_ITEM_CONTROL_URL =
"http://hl7.org/fhir/StructureDefinition/questionnaire-itemControl"
Expand All @@ -38,6 +39,7 @@ internal val Questionnaire.Item.itemControl: String?
) {
ITEM_CONTROL_DROP_DOWN -> ITEM_CONTROL_DROP_DOWN
ITEM_CONTROL_RADIO_BUTTON -> ITEM_CONTROL_RADIO_BUTTON
ITEM_CONTROL_SLIDER -> ITEM_CONTROL_SLIDER
else -> null
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import com.google.android.fhir.datacapture.views.QuestionnaireItemEditTextQuanti
import com.google.android.fhir.datacapture.views.QuestionnaireItemEditTextSingleLineViewHolderFactory
import com.google.android.fhir.datacapture.views.QuestionnaireItemGroupViewHolderFactory
import com.google.android.fhir.datacapture.views.QuestionnaireItemRadioGroupViewHolderFactory
import com.google.android.fhir.datacapture.views.QuestionnaireItemSliderViewHolderFactory
import com.google.android.fhir.datacapture.views.QuestionnaireItemViewHolder
import com.google.android.fhir.datacapture.views.QuestionnaireItemViewItem
import com.google.fhir.r4.core.Questionnaire
Expand Down Expand Up @@ -66,6 +67,8 @@ internal class QuestionnaireItemAdapter :
QuestionnaireItemDisplayViewHolderFactory
QuestionnaireItemViewHolderType.QUANTITY ->
QuestionnaireItemEditTextQuantityViewHolderFactory
QuestionnaireItemViewHolderType.SLIDER ->
QuestionnaireItemSliderViewHolderFactory
}
return viewHolderFactory.create(parent)
}
Expand Down Expand Up @@ -94,7 +97,7 @@ internal class QuestionnaireItemAdapter :
QuestionnaireItemTypeCode.Value.TEXT ->
QuestionnaireItemViewHolderType.EDIT_TEXT_MULTI_LINE
QuestionnaireItemTypeCode.Value.INTEGER ->
QuestionnaireItemViewHolderType.EDIT_TEXT_INTEGER
getIntegerViewHolderType(questionnaireItem)
QuestionnaireItemTypeCode.Value.DECIMAL ->
QuestionnaireItemViewHolderType.EDIT_TEXT_DECIMAL
QuestionnaireItemTypeCode.Value.CHOICE ->
Expand Down Expand Up @@ -122,6 +125,14 @@ internal class QuestionnaireItemAdapter :
}
}

private fun getIntegerViewHolderType(questionnaireItem: Questionnaire.Item):
QuestionnaireItemViewHolderType {
if (questionnaireItem.itemControl == ITEM_CONTROL_SLIDER) {
return QuestionnaireItemViewHolderType.SLIDER
}
return QuestionnaireItemViewHolderType.EDIT_TEXT_INTEGER
}

internal companion object {
// Choice questions are rendered as radio group if number of options less than this constant
const val MINIMUM_NUMBER_OF_ANSWER_OPTIONS_FOR_DROP_DOWN = 4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ internal enum class QuestionnaireItemViewHolderType(val value: Int) {
RADIO_GROUP(8),
DROP_DOWN(9),
DISPLAY(10),
QUANTITY(11);
QUANTITY(11),
SLIDER(12);

companion object {
private val VALUES = values()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright 2020 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.TextView
import com.google.android.fhir.datacapture.R
import com.google.android.material.slider.Slider
import com.google.fhir.r4.core.Integer
import com.google.fhir.r4.core.QuestionnaireResponse

internal object QuestionnaireItemSliderViewHolderFactory : QuestionnaireItemViewHolderFactory(
R.layout.questionnaire_item_slider
) {
override fun getQuestionnaireItemViewHolderDelegate(): QuestionnaireItemViewHolderDelegate =
object : QuestionnaireItemViewHolderDelegate {
private lateinit var sliderHeader: TextView
private lateinit var slider: Slider
private lateinit var questionnaireItemViewItem: QuestionnaireItemViewItem

override fun init(itemView: View) {
sliderHeader = itemView.findViewById(R.id.slider_header)
slider = itemView.findViewById(R.id.slider)
}

override fun bind(questionnaireItemViewItem: QuestionnaireItemViewItem) {
this.questionnaireItemViewItem = questionnaireItemViewItem
val questionnaireItem = questionnaireItemViewItem.questionnaireItem
val answer = questionnaireItemViewItem.singleAnswerOrNull
sliderHeader.text = questionnaireItem.text.value
slider.valueFrom = 0.0F
slider.valueTo = 100.0F
slider.stepSize = 10.0F
val sliderValue = answer?.value?.integer?.value?.toString() ?: "0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cant be hardcoded.. imagine if the slider is from 5 to 10

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now, this is hard coded, will create a new issue for this as discussed with @jingtang10 to read values from extensions

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes i think we'll have to do this in a follow up and especially considering the ongoing work in #224

slider.value = sliderValue.toFloat()

slider.addOnChangeListener { _, newValue, _ ->
// Responds to when slider's value is changed
QuestionnaireResponse.Item.Answer.newBuilder()
.apply {
value = QuestionnaireResponse.Item.Answer.ValueX.newBuilder()
.setInteger(Integer.newBuilder().setValue(newValue.toInt())
.build()).build()

questionnaireItemViewItem.singleAnswerOrNull = this
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make this questionnaireItemViewItem.singleAnswerOrNull = for better readability

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, not understanding this. Can you explain this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of questionnaireItemViewItem.singleAnswerOrNull = this inside the apply block can you refactor such that it is something like this

questionnaireItemViewItem.singleAnswerOrNull = QuestionnaireResponse.Item.Answer.newBuilder().apply { value = .... }

You can refer to the other factories to find this pattern

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joiskash but this is already doing the same way as QuestionnaireResponse.Item.Answer.newBuilder().apply in the code
Screenshot from 2021-03-08 17-44-05

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is the same thing. Just suggesting to maintain uniformity in the code. Also why is the callback called inside apply?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not set questionnaireItemViewItem.singleAnswerOrNull inside the apply

also keep questionnaireItemViewItem.questionnaireResponseItemChangedCallback() outside of apply

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

questionnaireItemViewItem.questionnaireResponseItemChangedCallback()
}
}
}
}
}
18 changes: 18 additions & 0 deletions datacapture/src/main/res/layout/questionnaire_item_slider.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginHorizontal="@dimen/item_margin_horizontal"
android:layout_marginVertical="@dimen/item_margin_vertical"
android:orientation="vertical">

<TextView
android:id="@+id/slider_header"
android:layout_width="wrap_content"
android:layout_height="wrap_content" />

<com.google.android.material.slider.Slider
android:id="@+id/slider"
android:layout_width="match_parent"
android:layout_height="wrap_content" />
</LinearLayout>
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import org.robolectric.annotation.Config
class QuestionnaireItemViewHolderTypeTest {
@Test
fun size_shouldReturnNumberOfQuestionnaireViewHolderTypes() {
assertThat(QuestionnaireItemViewHolderType.values().size).isEqualTo(12)
assertThat(QuestionnaireItemViewHolderType.values().size).isEqualTo(13)
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion deps.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ versions.hapi_r4 = '5.3.0'
versions.junit = '4.13'
versions.kotlin = '1.4.30'
versions.lifecycle = '2.2.0'
versions.material = '1.1.0'
versions.material = '1.3.0'
versions.okhttp_logging_interceptor = '4.0.0'
versions.recyclerview = '1.1.0'
versions.retrofit = '2.7.2'
Expand Down