From d8b735f80e7ba766d62b49a62f6c59bc415400e8 Mon Sep 17 00:00:00 2001 From: Ben Weiss Date: Wed, 11 Dec 2024 16:09:37 +0100 Subject: [PATCH] Revert "Improve accessibility to ForYou topics (#1309)" (#1732) This reverts commit 48544194f4a8f525d2523486e4bd29c7a6ecf545. --- .../apps/nowinandroid/ui/NavigationTest.kt | 27 ++----- .../feature/foryou/ForYouScreenTest.kt | 4 +- .../feature/foryou/ForYouScreen.kt | 76 +++++-------------- .../foryou/src/main/res/values/strings.xml | 2 - 4 files changed, 27 insertions(+), 82 deletions(-) diff --git a/app/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt b/app/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt index 2b0bc3bf0..54053a1bb 100644 --- a/app/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt +++ b/app/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt @@ -18,8 +18,7 @@ package com.google.samples.apps.nowinandroid.ui import androidx.compose.ui.semantics.SemanticsActions.ScrollBy import androidx.compose.ui.test.assertCountEquals -import androidx.compose.ui.test.assertIsDisplayed -import androidx.compose.ui.test.assertIsNotDisplayed +import androidx.compose.ui.test.assertIsOn import androidx.compose.ui.test.assertIsSelected import androidx.compose.ui.test.hasTestTag import androidx.compose.ui.test.hasText @@ -88,8 +87,6 @@ class NavigationTest { private val forYou by composeTestRule.stringResource(FeatureForyouR.string.feature_foryou_title) private val interests by composeTestRule.stringResource(FeatureSearchR.string.feature_search_interests) private val sampleTopic = "Headlines" - private val sampleTopicCheckIconDescription = "Headlines checked" - private val sampleTopicAddIconDescription = "Headlines add" private val appName by composeTestRule.stringResource(R.string.app_name) private val saved by composeTestRule.stringResource(BookmarksR.string.feature_bookmarks_title) private val settings by composeTestRule.stringResource(SettingsR.string.feature_settings_top_app_bar_action_icon_description) @@ -118,20 +115,13 @@ class NavigationTest { fun navigationBar_navigateToPreviouslySelectedTab_restoresContent() { composeTestRule.apply { // GIVEN the user follows a topic - onNodeWithContentDescription(sampleTopic).performClick() + onNodeWithText(sampleTopic).performClick() // WHEN the user navigates to the Interests destination onNodeWithText(interests).performClick() // AND the user navigates to the For You destination onNodeWithText(forYou).performClick() // THEN the state of the For You destination is restored - onNodeWithContentDescription( - sampleTopicCheckIconDescription, - useUnmergedTree = true, - ).assertIsDisplayed() - onNodeWithContentDescription( - sampleTopicAddIconDescription, - useUnmergedTree = true, - ).assertIsNotDisplayed() + onNodeWithContentDescription(sampleTopic).assertIsOn() } } @@ -142,18 +132,11 @@ class NavigationTest { fun navigationBar_reselectTab_keepsState() { composeTestRule.apply { // GIVEN the user follows a topic - onNodeWithContentDescription(sampleTopic).performClick() + onNodeWithText(sampleTopic).performClick() // WHEN the user taps the For You navigation bar item onNodeWithText(forYou).performClick() // THEN the state of the For You destination is restored - onNodeWithContentDescription( - sampleTopicCheckIconDescription, - useUnmergedTree = true, - ).assertIsDisplayed() - onNodeWithContentDescription( - sampleTopicAddIconDescription, - useUnmergedTree = true, - ).assertIsNotDisplayed() + onNodeWithContentDescription(sampleTopic).assertIsOn() } } diff --git a/feature/foryou/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt b/feature/foryou/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt index 6bb2197c5..c3ec5c560 100644 --- a/feature/foryou/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt +++ b/feature/foryou/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt @@ -128,7 +128,7 @@ class ForYouScreenTest { testData.forEach { testTopic -> composeTestRule - .onNodeWithContentDescription(testTopic.topic.name) + .onNodeWithText(testTopic.topic.name) .assertExists() .assertHasClickAction() } @@ -175,7 +175,7 @@ class ForYouScreenTest { followableTopicTestData.forEach { testTopic -> composeTestRule - .onNodeWithContentDescription(testTopic.topic.name) + .onNodeWithText(testTopic.topic.name) .assertExists() .assertHasClickAction() } diff --git a/feature/foryou/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt b/feature/foryou/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt index 7190f5cbc..1a3325996 100644 --- a/feature/foryou/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt +++ b/feature/foryou/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt @@ -75,13 +75,6 @@ import androidx.compose.ui.platform.LocalInspectionMode import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource -import androidx.compose.ui.semantics.clearAndSetSemantics -import androidx.compose.ui.semantics.contentDescription -import androidx.compose.ui.semantics.semantics -import androidx.compose.ui.semantics.stateDescription -import androidx.compose.ui.semantics.toggleableState -import androidx.compose.ui.state.ToggleableState.Off -import androidx.compose.ui.state.ToggleableState.On import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp @@ -275,30 +268,23 @@ private fun LazyStaggeredGridScope.onboarding( is OnboardingUiState.Shown -> { item(span = StaggeredGridItemSpan.FullLine, contentType = "onboarding") { - Column( - modifier = interestsItemModifier, - ) { - Column( + Column(modifier = interestsItemModifier) { + Text( + text = stringResource(R.string.feature_foryou_onboarding_guidance_title), + textAlign = TextAlign.Center, modifier = Modifier - .semantics(mergeDescendants = true) { }, - ) { - Text( - text = stringResource(R.string.feature_foryou_onboarding_guidance_title), - textAlign = TextAlign.Center, - modifier = Modifier - .fillMaxWidth() - .padding(top = 24.dp), - style = MaterialTheme.typography.titleMedium, - ) - Text( - text = stringResource(R.string.feature_foryou_onboarding_guidance_subtitle), - modifier = Modifier - .fillMaxWidth() - .padding(top = 8.dp, start = 24.dp, end = 24.dp), - textAlign = TextAlign.Center, - style = MaterialTheme.typography.bodyMedium, - ) - } + .fillMaxWidth() + .padding(top = 24.dp), + style = MaterialTheme.typography.titleMedium, + ) + Text( + text = stringResource(R.string.feature_foryou_onboarding_guidance_subtitle), + modifier = Modifier + .fillMaxWidth() + .padding(top = 8.dp, start = 24.dp, end = 24.dp), + textAlign = TextAlign.Center, + style = MaterialTheme.typography.bodyMedium, + ) TopicSelection( onboardingUiState, onTopicCheckedChanged, @@ -398,21 +384,7 @@ private fun SingleTopicButton( Surface( modifier = Modifier .width(312.dp) - .heightIn(min = 56.dp) - .semantics(mergeDescendants = true) { - toggleableState = if (isSelected) { - On - } else { - Off - } - stateDescription = if (isSelected) { - "Following" - } else { - "Not Following" - } - - contentDescription = name - }, + .heightIn(min = 56.dp), shape = RoundedCornerShape(corner = CornerSize(8.dp)), color = MaterialTheme.colorScheme.surface, selected = isSelected, @@ -422,9 +394,7 @@ private fun SingleTopicButton( ) { Row( verticalAlignment = Alignment.CenterVertically, - modifier = Modifier - .padding(start = 12.dp, end = 8.dp) - .clearAndSetSemantics { }, + modifier = Modifier.padding(start = 12.dp, end = 8.dp), ) { TopicIcon( imageUrl = imageUrl, @@ -443,19 +413,13 @@ private fun SingleTopicButton( icon = { Icon( imageVector = NiaIcons.Add, - contentDescription = stringResource( - id = R.string.feature_foryou_topic_icon_add, - name, - ), + contentDescription = name, ) }, checkedIcon = { Icon( imageVector = NiaIcons.Check, - contentDescription = stringResource( - id = R.string.feature_foryou_topic_icon_checked, - name, - ), + contentDescription = name, ) }, ) diff --git a/feature/foryou/src/main/res/values/strings.xml b/feature/foryou/src/main/res/values/strings.xml index 3efa10b0b..166749664 100644 --- a/feature/foryou/src/main/res/values/strings.xml +++ b/feature/foryou/src/main/res/values/strings.xml @@ -21,7 +21,5 @@ Navigate up What are you interested in? Updates from topics you follow will appear here. Follow some things to get started. - %s checked - %s add