Skip to content

Commit

Permalink
Fix #3907: [A11Y] Output Congratulations for screenreader (#3980)
Browse files Browse the repository at this point in the history
* Add announceForAccessibility

* Use resources

* Use resourceHandler

* Only announce "Correct" if there is no feedback.

* Wrap announceForAccessibility to accomodate testing.

* Add kdoc.

* Revert test exploration changes.

* Expand comments.

* Add checks

* Add checks

* Remove feedback_1.

* Add comma.

* feedback_1

* Revert feedback nulls

* Remove ? from CharSequence.

* Inject accessibilityChecker in the Builder's Factory.

* Update kdoc.

* Add question player test.

* lint

* Update utility/src/main/java/org/oppia/android/util/accessibility/FakeAccessibilityChecker.kt

Co-authored-by: Ben Henning <[email protected]>

* Update app/src/main/java/org/oppia/android/app/player/state/StatePlayerRecyclerViewAssembler.kt

Co-authored-by: Ben Henning <[email protected]>

* Expand file content checks, add tests.

* lint

* Fix RegexPatternValidationCheckTest

* Fix RegexPatternValidationCheckTest

* Fix RegexPatternValidationCheckTest

* Fix RegexPatternValidationCheckTest

* Fix RegexPatternValidationCheckTest

* Move accessibilityChecker to the constructor.

* Add more tests.

* lint

* Add KDoc

* Add FakeAccessibilityCheckerTest

* Add newline

* Add exempted file

* Rename AccessibilityChecker to AccessibilityService

* Update BUILD file

* Update test_file_exemptions.textproto

* Update file_content_validation_checks.textproto

* Update providers.

* Inject accessibilityService in test.

* lint

* lint

* Add TestModule

* Undo add TestModule

* Add Config

* Add TestApplication

* nit

Co-authored-by: viktoriia <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
  • Loading branch information
3 people committed Jan 19, 2022
1 parent 034b55f commit da47af6
Show file tree
Hide file tree
Showing 24 changed files with 401 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import org.oppia.android.app.recyclerview.OnItemDragListener
import org.oppia.android.app.shim.ViewBindingShim
import org.oppia.android.app.view.ViewComponentFactory
import org.oppia.android.app.view.ViewComponentImpl
import org.oppia.android.util.accessibility.AccessibilityChecker
import org.oppia.android.util.accessibility.AccessibilityService
import org.oppia.android.util.gcsresource.DefaultResourceBucketName
import org.oppia.android.util.parser.html.ExplorationHtmlParserEntityType
import org.oppia.android.util.parser.html.HtmlParser
Expand All @@ -40,7 +40,7 @@ class DragDropSortInteractionView @JvmOverloads constructor(
lateinit var htmlParserFactory: HtmlParser.Factory

@Inject
lateinit var accessibilityChecker: AccessibilityChecker
lateinit var accessibilityService: AccessibilityService

@Inject
@field:ExplorationHtmlParserEntityType
Expand All @@ -64,7 +64,7 @@ class DragDropSortInteractionView @JvmOverloads constructor(
val viewComponent = viewComponentFactory.createViewComponent(this) as ViewComponentImpl
viewComponent.inject(this)

isAccessibilityEnabled = accessibilityChecker.isScreenReaderEnabled()
isAccessibilityEnabled = accessibilityService.isScreenReaderEnabled()
}

fun allowMultipleItemsInSamePosition(isAllowed: Boolean) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import org.oppia.android.app.utility.ClickableAreasImage
import org.oppia.android.app.utility.OnClickableAreaClickedListener
import org.oppia.android.app.view.ViewComponentFactory
import org.oppia.android.app.view.ViewComponentImpl
import org.oppia.android.util.accessibility.AccessibilityChecker
import org.oppia.android.util.accessibility.AccessibilityService
import org.oppia.android.util.gcsresource.DefaultResourceBucketName
import org.oppia.android.util.locale.OppiaLocale
import org.oppia.android.util.parser.html.ExplorationHtmlParserEntityType
Expand Down Expand Up @@ -43,7 +43,7 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor(
private lateinit var listener: OnClickableAreaClickedListener

@Inject
lateinit var accessibilityChecker: AccessibilityChecker
lateinit var accessibilityService: AccessibilityService

@Inject
lateinit var imageLoader: ImageLoader
Expand Down Expand Up @@ -141,7 +141,7 @@ class ImageRegionSelectionInteractionView @JvmOverloads constructor(
val viewComponent = viewComponentFactory.createViewComponent(this) as ViewComponentImpl
viewComponent.inject(this)

isAccessibilityEnabled = accessibilityChecker.isScreenReaderEnabled()
isAccessibilityEnabled = accessibilityService.isScreenReaderEnabled()
}

fun setOnRegionClicked(onRegionClicked: OnClickableAreaClickedListener) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ class StateFragmentPresenter @Inject constructor(
moveToNextState()
} else {
if (result.labelledAsCorrectAnswer) {
recyclerViewAssembler.showCelebrationOnCorrectAnswer()
recyclerViewAssembler.showCelebrationOnCorrectAnswer(result.feedback)
} else {
viewModel.setCanSubmitAnswer(canSubmitAnswer = false)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import androidx.fragment.app.Fragment
import androidx.lifecycle.Observer
import kotlinx.coroutines.CoroutineDispatcher
import nl.dionsegijn.konfetti.KonfettiView
import org.oppia.android.R
import org.oppia.android.app.model.AnswerAndResponse
import org.oppia.android.app.model.EphemeralState
import org.oppia.android.app.model.EphemeralState.StateTypeCase
Expand Down Expand Up @@ -87,6 +88,7 @@ import org.oppia.android.databinding.SubmittedAnswerListItemBinding
import org.oppia.android.databinding.SubmittedHtmlAnswerItemBinding
import org.oppia.android.databinding.TextInputInteractionItemBinding
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.util.accessibility.AccessibilityService
import org.oppia.android.util.parser.html.HtmlParser
import org.oppia.android.util.threading.BackgroundDispatcher
import javax.inject.Inject
Expand Down Expand Up @@ -119,6 +121,7 @@ private const val CONGRATULATIONS_TEXT_VIEW_VISIBLE_MILLIS: Long = 800
* - [ReturnToTopicNavigationButtonListener] if the return to topic button is enabled
*/
class StatePlayerRecyclerViewAssembler private constructor(
private val accessibilityService: AccessibilityService,
val adapter: BindableAdapter<StateItemViewModel>,
val rhsAdapter: BindableAdapter<StateItemViewModel>,
private val playerFeatureSet: PlayerFeatureSet,
Expand Down Expand Up @@ -457,8 +460,11 @@ class StatePlayerRecyclerViewAssembler private constructor(
/**
* Shows a celebratory animation with a congratulations message and confetti when the learner submits
* a correct answer.
*
* @param feedback Oppia's feedback to the learner's most recent answer. If the feedback is empty,
* talkback confirms correct answers by a separate announcement.
*/
fun showCelebrationOnCorrectAnswer() {
fun showCelebrationOnCorrectAnswer(feedback: SubtitledHtml) {
check(playerFeatureSet.showCelebrationOnCorrectAnswer) {
"Cannot show congratulations message for assembler that doesn't support it"
}
Expand All @@ -471,9 +477,15 @@ class StatePlayerRecyclerViewAssembler private constructor(
val confettiConfig = checkNotNull(congratulationsTextConfettiConfig) {
"Expected non-null reference to confetti animation configuration"
}

createBannerConfetti(confettiView, confettiConfig)
animateCongratulationsTextView(textView)

if (feedback.html.isBlank()) {
accessibilityService.announceForAccessibilityForView(
textView,
resourceHandler.getStringInLocale(R.string.correct)
)
}
}

/** Shows confetti when the learner reaches the end of an exploration session. */
Expand Down Expand Up @@ -864,6 +876,7 @@ class StatePlayerRecyclerViewAssembler private constructor(
* using its injectable [Factory].
*/
class Builder private constructor(
private var accessibilityService: AccessibilityService,
private val htmlParserFactory: HtmlParser.Factory,
private val resourceBucketName: String,
private val entityType: String,
Expand Down Expand Up @@ -1317,6 +1330,7 @@ class StatePlayerRecyclerViewAssembler private constructor(
fun build(): StatePlayerRecyclerViewAssembler {
val playerFeatureSet = featureSets.reduce(PlayerFeatureSet::union)
val assembler = StatePlayerRecyclerViewAssembler(
accessibilityService,
/* adapter= */ adapterBuilder.build(),
/* rhsAdapter= */ adapterBuilder.build(),
playerFeatureSet,
Expand Down Expand Up @@ -1347,6 +1361,7 @@ class StatePlayerRecyclerViewAssembler private constructor(

/** Fragment injectable factory to create new [Builder]s. */
class Factory @Inject constructor(
private val accessibilityService: AccessibilityService,
private val htmlParserFactory: HtmlParser.Factory,
private val fragment: Fragment,
private val context: Context,
Expand All @@ -1362,6 +1377,7 @@ class StatePlayerRecyclerViewAssembler private constructor(
*/
fun create(resourceBucketName: String, entityType: String, profileId: ProfileId): Builder {
return Builder(
accessibilityService,
htmlParserFactory,
resourceBucketName,
entityType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ class QuestionPlayerFragmentPresenter @Inject constructor(
recyclerViewAssembler.isCorrectAnswer.set(result.isCorrectAnswer)
if (result.isCorrectAnswer) {
questionViewModel.setHintBulbVisibility(false)
recyclerViewAssembler.showCelebrationOnCorrectAnswer()
recyclerViewAssembler.showCelebrationOnCorrectAnswer(result.feedback)
} else {
questionViewModel.setCanSubmitAnswer(canSubmitAnswer = false)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package org.oppia.android.app.accessibility

import android.app.Application
import android.view.View
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.google.common.truth.Truth.assertThat
import dagger.BindsInstance
import dagger.Component
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.mock
import org.oppia.android.util.accessibility.AccessibilityTestModule
import org.oppia.android.util.accessibility.FakeAccessibilityService
import org.robolectric.annotation.Config
import org.robolectric.annotation.LooperMode
import javax.inject.Inject
import javax.inject.Singleton

/** Tests for [FakeAccessibilityService]. */
@RunWith(AndroidJUnit4::class)
@LooperMode(LooperMode.Mode.PAUSED)
@Config(application = FakeAccessibilityServiceTest.TestApplication::class)
class FakeAccessibilityServiceTest {
@Inject
lateinit var accessibilityService: FakeAccessibilityService

@Before
fun setUp() {
setUpTestApplicationComponent()
}

@Test
fun testFakeAccessibilityService_initialState_isScreenReaderEnabled_isFalse() {
assertThat(accessibilityService.isScreenReaderEnabled()).isFalse()
}

@Test
fun testFakeAccessibilityService_initialState_getLatestAnnouncement_isNull() {
assertThat(accessibilityService.getLatestAnnouncement()).isNull()
}

@Test
fun testFakeAccessibilityService_setScreenReaderEnabledTrue_isTrue() {
accessibilityService.setScreenReaderEnabled(true)
assertThat(accessibilityService.isScreenReaderEnabled()).isTrue()
}

@Test
fun testFakeAccessibilityService_setScreenReaderEnabledFalse_isFalse() {
accessibilityService.setScreenReaderEnabled(false)
assertThat(accessibilityService.isScreenReaderEnabled()).isFalse()
}

@Test
fun testFakeAccessibilityService_announceForAccessibilityForView_latestAnnouncementIsSet() {
accessibilityService.announceForAccessibilityForView(mock(View::class.java), "test")
assertThat(accessibilityService.getLatestAnnouncement()).isEqualTo("test")
}

@Test
fun testFakeAccessibilityService_resetLatestAnnouncement_latestAnnouncementIsNull() {
accessibilityService.resetLatestAnnouncement()
assertThat(accessibilityService.getLatestAnnouncement()).isNull()
}

private fun setUpTestApplicationComponent() {
ApplicationProvider.getApplicationContext<TestApplication>().inject(this)
}

@Singleton
@Component(modules = [AccessibilityTestModule::class])
interface TestApplicationComponent {
@Component.Builder
interface Builder {
@BindsInstance
fun setApplication(application: Application): Builder
fun build(): TestApplicationComponent
}

fun inject(fakeAccessibilityServiceTest: FakeAccessibilityServiceTest)
}

class TestApplication : Application() {
private val component: TestApplicationComponent by lazy {
DaggerFakeAccessibilityServiceTest_TestApplicationComponent.builder()
.setApplication(ApplicationProvider.getApplicationContext())
.build()
}

fun inject(fakeAccessibilityServiceTest: FakeAccessibilityServiceTest) {
component.inject(fakeAccessibilityServiceTest)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ import org.oppia.android.testing.threading.TestCoroutineDispatchers
import org.oppia.android.testing.threading.TestDispatcherModule
import org.oppia.android.testing.time.FakeOppiaClockModule
import org.oppia.android.util.accessibility.AccessibilityTestModule
import org.oppia.android.util.accessibility.FakeAccessibilityService
import org.oppia.android.util.caching.AssetModule
import org.oppia.android.util.caching.CacheAssetsLocally
import org.oppia.android.util.caching.LoadImagesFromAssets
Expand Down Expand Up @@ -191,6 +192,9 @@ class StateFragmentLocalTest {
@Inject
lateinit var editTextInputAction: EditTextInputAction

@Inject
lateinit var accessibilityManager: FakeAccessibilityService

@Inject
lateinit var translationController: TranslationController

Expand All @@ -216,7 +220,6 @@ class StateFragmentLocalTest {
.setAnimationExecutor(executorService)
.setSourceExecutor(executorService)
)

profileTestHelper.initializeProfiles()
ShadowMediaPlayer.addException(audioDataSource1, IOException("Test does not have networking"))
}
Expand Down Expand Up @@ -352,6 +355,58 @@ class StateFragmentLocalTest {
}
}

@Test
@Config(qualifiers = "+port")
fun testStateFragment_portrait_submitCorrectAnswerWithFeedback_correctIsNotAnnounced() {
launchForExploration(FRACTIONS_EXPLORATION_ID_1).use {
startPlayingExploration()
playThroughFractionsState1()
playThroughFractionsState2()
accessibilityManager.resetLatestAnnouncement()
playThroughFractionsState3()

assertThat(accessibilityManager.getLatestAnnouncement()).isNull()
}
}

@Test
@Config(qualifiers = "+land")
fun testStateFragment_landscape_submitCorrectAnswerWithFeedback_correctIsNotAnnounced() {
launchForExploration(FRACTIONS_EXPLORATION_ID_1).use {
startPlayingExploration()
playThroughFractionsState1()
playThroughFractionsState2()
accessibilityManager.resetLatestAnnouncement()
playThroughFractionsState3()

assertThat(accessibilityManager.getLatestAnnouncement()).isNull()
}
}

@Test
@Config(qualifiers = "+port")
fun testStateFragment_portrait_submitCorrectAnswer_correctIsAnnounced() {
launchForExploration(FRACTIONS_EXPLORATION_ID_1).use {
startPlayingExploration()
playThroughFractionsState1()
playThroughFractionsState2()

assertThat(accessibilityManager.getLatestAnnouncement()).isEqualTo("Correct!")
}
}

@Test
@Config(qualifiers = "+land")
fun testStateFragment_landscape_submitCorrectAnswer_correctIsAnnounced() {
launchForExploration(FRACTIONS_EXPLORATION_ID_1).use {
startPlayingExploration()
playThroughFractionsState1()
playThroughFractionsState2()

assertThat(accessibilityManager.getLatestAnnouncement()).isEqualTo("Correct!")
}
}

@Test
@Config(qualifiers = "+port")
fun testStateFragment_portrait_submitCorrectAnswer_confettiIsActive() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ import org.oppia.android.testing.threading.TestCoroutineDispatchers
import org.oppia.android.testing.threading.TestDispatcherModule
import org.oppia.android.testing.time.FakeOppiaClockModule
import org.oppia.android.util.accessibility.AccessibilityTestModule
import org.oppia.android.util.accessibility.FakeAccessibilityChecker
import org.oppia.android.util.accessibility.FakeAccessibilityService
import org.oppia.android.util.caching.AssetModule
import org.oppia.android.util.caching.testing.CachingTestModule
import org.oppia.android.util.gcsresource.GcsResourceModule
Expand Down Expand Up @@ -105,7 +105,7 @@ class StateFragmentAccessibilityTest {
lateinit var context: Context

@Inject
lateinit var fakeAccessibilityManager: FakeAccessibilityChecker
lateinit var fakeAccessibilityManager: FakeAccessibilityService

private val internalProfileId: Int = 1

Expand Down
Loading

0 comments on commit da47af6

Please sign in to comment.