From 504db33b89ad7d410a92b5afe558ee2020f88e84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Odin=20Asbj=C3=B8rnsen?= <54936943+oas004@users.noreply.github.com> Date: Tue, 12 Jul 2022 08:30:34 +0200 Subject: [PATCH] Fix textoverflow on top appbar for components with long names (#240) * Fix textoverflow on top appbar for components with long names * Add testcase for textoverflow in top app bar * Move semantics to semantic utils --- .../ShowkaseBrowserTest.kt | 38 ++++++++++++++----- .../ShowkaseBrowserTestFlows.kt | 11 +++++- .../TestComposables.kt | 9 +++++ .../android/showkase/ui/SemanticsUtils.kt | 9 +++++ .../android/showkase/ui/ShowkaseBrowserApp.kt | 27 ++++++++++--- 5 files changed, 78 insertions(+), 16 deletions(-) create mode 100644 showkase/src/main/java/com/airbnb/android/showkase/ui/SemanticsUtils.kt diff --git a/showkase-browser-testing/src/androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTest.kt b/showkase-browser-testing/src/androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTest.kt index 14447d61..5334b20f 100644 --- a/showkase-browser-testing/src/androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTest.kt +++ b/showkase-browser-testing/src/androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTest.kt @@ -1,15 +1,13 @@ package com.airbnb.android.showkase_browser_testing import androidx.compose.ui.test.junit4.AndroidComposeTestRule -import androidx.test.ext.junit.rules.ActivityScenarioRule -import androidx.test.platform.app.InstrumentationRegistry import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performGesture import androidx.compose.ui.test.swipeDown +import androidx.test.ext.junit.rules.ActivityScenarioRule +import androidx.test.platform.app.InstrumentationRegistry import com.airbnb.android.showkase.models.Showkase import com.airbnb.android.showkase.ui.ShowkaseBrowserActivity -import com.airbnb.android.showkase_browser_testing.getBrowserIntent -import kotlinx.coroutines.delay import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -52,7 +50,7 @@ class ShowcaseBrowserTest { verifyLandingScreen() // Tap on the "Components" row - clickRowWithText("Components (6)") + clickRowWithText("Components (7)") // Verify that all the groups are displayed on the screen verifyRowsWithTextAreDisplayed("Group1 (2)", "Group2 (1)", "Group3 (2)", "Submodule (1)") @@ -94,7 +92,7 @@ class ShowcaseBrowserTest { verifyLandingScreen() // Tap on the "Components" row - clickRowWithText("Components (6)") + clickRowWithText("Components (7)") // Select "Group1" clickRowWithText("Group1 (2)") @@ -200,7 +198,7 @@ class ShowcaseBrowserTest { verifyLandingScreen() // Select Components - clickRowWithText("Components (6)") + clickRowWithText("Components (7)") // Tap on the search icon clickRowWithTag("SearchIcon") @@ -264,7 +262,7 @@ class ShowcaseBrowserTest { verifyLandingScreen() // Select components - clickRowWithText("Components (6)") + clickRowWithText("Components (7)") // Select Group 3 clickRowWithText("Group3 (2)") @@ -355,7 +353,7 @@ class ShowcaseBrowserTest { verifyLandingScreen() // Select components to go to the component groups screen - clickRowWithText("Components (6)") + clickRowWithText("Components (7)") // Click on "Group 1" to go to the components in a group screen clickRowWithText("Group1 (2)") @@ -456,4 +454,26 @@ class ShowcaseBrowserTest { verifyLandingScreen() } } + + @Test + fun components_with_long_names_have_a_correct_top_app_bar() { + composeTestRule.apply { + // Assert that all the categories are displayed on the screen and that they are clickable. + verifyLandingScreen() + + // Tap on the "Components" row + clickRowWithText("Components (7)") + + // Select "Group4" + clickRowWithText("Group4 (1)") + + // Select Component in question + clickRowWithText("Test Composable6") + + waitForIdle() + + //Check that the top app bar wraps 3 lines + verifyLineCountIsValue(3) + } + } } diff --git a/showkase-browser-testing/src/androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTestFlows.kt b/showkase-browser-testing/src/androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTestFlows.kt index eeac56f1..767a08fd 100644 --- a/showkase-browser-testing/src/androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTestFlows.kt +++ b/showkase-browser-testing/src/androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTestFlows.kt @@ -1,6 +1,7 @@ package com.airbnb.android.showkase_browser_testing import androidx.activity.ComponentActivity +import androidx.compose.ui.test.SemanticsMatcher import androidx.compose.ui.test.SemanticsNodeInteraction import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.assertIsDisplayed @@ -14,6 +15,7 @@ import androidx.compose.ui.test.performGesture import androidx.compose.ui.test.performTextInput import androidx.compose.ui.test.swipeUp import androidx.test.ext.junit.rules.ActivityScenarioRule +import com.airbnb.android.showkase.ui.SemanticsUtils.LineCountKey import com.airbnb.android.showkase.ui.ShowkaseBrowserActivity internal fun AndroidComposeTestRule, ShowkaseBrowserActivity>.clickRowWithText( @@ -77,7 +79,7 @@ internal fun AndroidComposeTestRule, ShowkaseBrowserActivity>.verifyLandingScreen() { - verifyRowsWithTextAreDisplayed("Components (6)", "Typography (13)", "Colors (4)") + verifyRowsWithTextAreDisplayed("Components (7)", "Typography (13)", "Colors (4)") } internal fun AndroidComposeTestRule, ShowkaseBrowserActivity>.verifyTypographyDetailScreen() { @@ -99,3 +101,10 @@ internal fun AndroidComposeTestRule, ShowkaseBrowserActivity>.verifyLineCountIsValue( + value: Int +) { + onNode(SemanticsMatcher.expectValue(LineCountKey, value)).assertExists() +} + diff --git a/showkase-browser-testing/src/main/java/com/airbnb/android/showkase_browser_testing/TestComposables.kt b/showkase-browser-testing/src/main/java/com/airbnb/android/showkase_browser_testing/TestComposables.kt index 48c1e34e..5951bb0d 100644 --- a/showkase-browser-testing/src/main/java/com/airbnb/android/showkase_browser_testing/TestComposables.kt +++ b/showkase-browser-testing/src/main/java/com/airbnb/android/showkase_browser_testing/TestComposables.kt @@ -34,6 +34,15 @@ fun TestComposable4() { BasicText(text = "Test Composable4") } +@ShowkaseComposable( + "Composable6 Button Component wih a lot of extra stuff like spacing and such", + "Group4" +) +@Composable +fun TestComposable6() { + BasicText(text = "Test Composable6") +} + class WrapperComposableClass { @ShowkaseComposable("Composable5", "Group3") @Composable diff --git a/showkase/src/main/java/com/airbnb/android/showkase/ui/SemanticsUtils.kt b/showkase/src/main/java/com/airbnb/android/showkase/ui/SemanticsUtils.kt new file mode 100644 index 00000000..7147a420 --- /dev/null +++ b/showkase/src/main/java/com/airbnb/android/showkase/ui/SemanticsUtils.kt @@ -0,0 +1,9 @@ +package com.airbnb.android.showkase.ui + +import androidx.compose.ui.semantics.SemanticsPropertyKey +import androidx.compose.ui.semantics.SemanticsPropertyReceiver + +object SemanticsUtils { + val LineCountKey = SemanticsPropertyKey("lineCount") + var SemanticsPropertyReceiver.lineCountVal by LineCountKey +} diff --git a/showkase/src/main/java/com/airbnb/android/showkase/ui/ShowkaseBrowserApp.kt b/showkase/src/main/java/com/airbnb/android/showkase/ui/ShowkaseBrowserApp.kt index 86f5c9d0..32f1aac0 100644 --- a/showkase/src/main/java/com/airbnb/android/showkase/ui/ShowkaseBrowserApp.kt +++ b/showkase/src/main/java/com/airbnb/android/showkase/ui/ShowkaseBrowserApp.kt @@ -7,7 +7,6 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.material.Icon import androidx.compose.material.IconButton @@ -21,6 +20,8 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.MutableState import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -28,10 +29,11 @@ import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.platform.LocalConfiguration import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.testTag +import androidx.compose.ui.semantics.semantics import androidx.compose.ui.text.TextStyle import androidx.compose.ui.text.font.FontFamily import androidx.compose.ui.text.font.FontWeight -import androidx.compose.ui.unit.dp +import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.sp import androidx.navigation.NavHostController import androidx.navigation.compose.NavHost @@ -46,6 +48,7 @@ import com.airbnb.android.showkase.models.ShowkaseBrowserTypography import com.airbnb.android.showkase.models.ShowkaseCategory import com.airbnb.android.showkase.models.ShowkaseCurrentScreen import com.airbnb.android.showkase.models.insideGroup +import com.airbnb.android.showkase.ui.SemanticsUtils.lineCountVal @Composable internal fun ShowkaseBrowserApp( @@ -91,8 +94,7 @@ internal fun ShowkaseAppBar( Row( Modifier.fillMaxWidth() .graphicsLayer(shadowElevation = 4f) - .padding(padding2x) - .height(64.dp), + .padding(padding2x), horizontalArrangement = Arrangement.SpaceBetween, verticalAlignment = Alignment.CenterVertically ) { @@ -186,19 +188,32 @@ private fun ShowkaseAppBarTitle( } } + @Composable fun ToolbarTitle( string: String, modifier: Modifier ) { + val lineCount = remember { + mutableStateOf(0) + } + Text( text = string, - modifier = modifier, + modifier = modifier then Modifier + .semantics { + lineCountVal = lineCount.value + }, style = TextStyle( fontSize = 20.sp, fontFamily = FontFamily.Monospace, fontWeight = FontWeight.Bold - ) + ), + maxLines = 3, + overflow = TextOverflow.Ellipsis, + onTextLayout = { + lineCount.value = it.lineCount + } ) }