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.

Sign up for GitHub

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

Fix textoverflow on top appbar for components with long names #240

Merged
merged 3 commits into from
Jul 12, 2022
Merged

Fix textoverflow on top appbar for components with long names #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
605db41
Fix textoverflow on top appbar for components with long names
oas004 Jul 7, 2022
baea74a
Add testcase for textoverflow in top app bar
oas004 Jul 10, 2022
833e234
Move semantics to semantic utils
oas004 Jul 11, 2022
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
38 changes: 29 additions & 9 deletions ...g/src/androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)")
Expand Down Expand Up @@ -94,7 +92,7 @@ class ShowcaseBrowserTest {
verifyLandingScreen()

// Tap on the "Components" row
clickRowWithText("Components (6)")
clickRowWithText("Components (7)")

// Select "Group1"
clickRowWithText("Group1 (2)")
Expand Down Expand Up @@ -200,7 +198,7 @@ class ShowcaseBrowserTest {
verifyLandingScreen()

// Select Components
clickRowWithText("Components (6)")
clickRowWithText("Components (7)")

// Tap on the search icon
clickRowWithTag("SearchIcon")
Expand Down Expand Up @@ -264,7 +262,7 @@ class ShowcaseBrowserTest {
verifyLandingScreen()

// Select components
clickRowWithText("Components (6)")
clickRowWithText("Components (7)")

// Select Group 3
clickRowWithText("Group3 (2)")
Expand Down Expand Up @@ -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)")
Expand Down Expand Up @@ -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)
}
}
}
11 changes: 10 additions & 1 deletion .../androidTest/java/com/airbnb/android/showkase_browser_testing/ShowkaseBrowserTestFlows.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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<ActivityScenarioRule<ShowkaseBrowserActivity>, ShowkaseBrowserActivity>.clickRowWithText(
Expand Down Expand Up @@ -77,7 +79,7 @@ internal fun <T : ComponentActivity> AndroidComposeTestRule<ActivityScenarioRule
}

internal fun AndroidComposeTestRule<ActivityScenarioRule<ShowkaseBrowserActivity>, ShowkaseBrowserActivity>.verifyLandingScreen() {
verifyRowsWithTextAreDisplayed("Components (6)", "Typography (13)", "Colors (4)")
verifyRowsWithTextAreDisplayed("Components (7)", "Typography (13)", "Colors (4)")
}

internal fun AndroidComposeTestRule<ActivityScenarioRule<ShowkaseBrowserActivity>, ShowkaseBrowserActivity>.verifyTypographyDetailScreen() {
Expand All @@ -99,3 +101,10 @@ internal fun AndroidComposeTestRule<ActivityScenarioRule<ShowkaseBrowserActivity
"Primary", "Primary Variant", "Secondary", "Secondary Variant"
)
}

internal fun AndroidComposeTestRule<ActivityScenarioRule<ShowkaseBrowserActivity>, ShowkaseBrowserActivity>.verifyLineCountIsValue(
value: Int
) {
onNode(SemanticsMatcher.expectValue(LineCountKey, value)).assertExists()
}

9 changes: 9 additions & 0 deletions ...wser-testing/src/main/java/com/airbnb/android/showkase_browser_testing/TestComposables.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions showkase/src/main/java/com/airbnb/android/showkase/ui/SemanticsUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<Int>("lineCount")
var SemanticsPropertyReceiver.lineCountVal by LineCountKey
}
27 changes: 21 additions & 6 deletions showkase/src/main/java/com/airbnb/android/showkase/ui/ShowkaseBrowserApp.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -21,17 +20,20 @@ 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
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
Expand All @@ -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(
Expand Down Expand Up @@ -91,8 +94,7 @@ internal fun ShowkaseAppBar(
Row(
Modifier.fillMaxWidth()
.graphicsLayer(shadowElevation = 4f)
.padding(padding2x)
.height(64.dp),
.padding(padding2x),
Copy link
Collaborator

Choose a reason for hiding this comment

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

we might want to add a maxSize for the height but I'm curious to see the screenshots before that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm yeah maybe you're right 🤔 Is it also maybe a bit weird that it spans 3 lines instead of 2? What do you think? :)

horizontalArrangement = Arrangement.SpaceBetween,
verticalAlignment = Alignment.CenterVertically
) {
Expand Down Expand Up @@ -186,19 +188,32 @@ private fun ShowkaseAppBarTitle(
}
}


@Composable
fun ToolbarTitle(
string: String,
modifier: Modifier
) {
val lineCount = remember {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not ideal to have test logic live in the component directly but I guess that happens pretty often in Compose due to how semantics are setup with some hooks into testing. Don't have a better idea on testing this so might be fine for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree with you on that! I think this can work for now :)

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
}
)
}

Expand Down