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

Updated STAR demo to support navigating to standard Android #275

Merged
merged 3 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions samples/star/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ android {
defaultConfig {
minSdk = 28
testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
testApplicationId = "com.slack.circuit.star.androidTest"
testApplicationId = "com.slack.circuit.star.apk.androidTest"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this, the test suite can't find the test application at runtime

}

testOptions { unitTests.isIncludeAndroidResources = true }
Expand Down Expand Up @@ -115,7 +115,7 @@ dependencies {
testImplementation(libs.androidx.compose.ui.testing.manifest)
testImplementation(libs.leakcanary.android.instrumentation)

debugImplementation(libs.androidx.compose.ui.testing.manifest)
androidTestImplementation(libs.androidx.compose.ui.testing.manifest)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this, ComposeActivity isn't merged with the manifest meaning it can't be found at runtime. Before introduction of the apk module, we used to support debug builds.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it needs to be present in both?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

By both do you mean unit tests? We were already merging correctly for unit tests

androidTestImplementation(libs.leakcanary.android.instrumentation)
androidTestImplementation(libs.androidx.compose.ui.testing.junit)
androidTestImplementation(libs.junit)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import androidx.compose.ui.test.assertTextEquals
import androidx.compose.ui.test.junit4.createAndroidComposeRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performTouchInput
import androidx.compose.ui.test.swipeUp
import androidx.test.platform.app.InstrumentationRegistry
Expand All @@ -30,10 +31,13 @@ import com.slack.circuit.CircuitCompositionLocals
import com.slack.circuit.CircuitConfig
import com.slack.circuit.star.R
import com.slack.circuit.star.petdetail.PetDetailTestConstants.ANIMAL_CONTAINER_TAG
import com.slack.circuit.star.petdetail.PetDetailTestConstants.FULL_BIO_TAG
import com.slack.circuit.star.petdetail.PetDetailTestConstants.PROGRESS_TAG
import com.slack.circuit.star.petdetail.PetDetailTestConstants.UNKNOWN_ANIMAL_TAG
import com.slack.circuit.star.petdetail.PetPhotoCarouselTestConstants.CAROUSEL_TAG
import com.slack.circuit.star.ui.FakeImageLoader
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.test.runTest
import leakcanary.DetectLeaksAfterTestSuccess.Companion.detectLeaksAfterTestSuccessWrapping
import org.junit.Before
import org.junit.Rule
Expand Down Expand Up @@ -93,6 +97,7 @@ class PetDetailTest {
name = "Baxter",
description = "Grumpy looking Australian Terrier",
tags = listOf("dog", "terrier", "male"),
eventSink = {}
)

var carouselScreen: PetPhotoCarouselScreen? = null
Expand Down Expand Up @@ -127,4 +132,37 @@ class PetDetailTest {
}
}
}

@Test
fun petDetail_emits_event_when_tapping_on_full_bio_button() = runTest {
val channel = Channel<Any>(1)

val success =
PetDetailScreen.State.Success(
url = "url",
photoUrls = listOf("http://some.url"),
photoUrlMemoryCacheKey = null,
name = "Baxter",
description = "Grumpy looking Australian Terrier",
tags = listOf("dog", "terrier", "male"),
eventSink = channel::trySend
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: why the name trySend for this callback? Should we just do send?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't use send here because it's suspend-able, trySend isn't

)

val circuitConfig =
CircuitConfig.Builder()
.setOnUnavailableContent { screen ->
PetPhotoCarousel(PetPhotoCarouselScreen.State(screen as PetPhotoCarouselScreen))
}
.build()

composeTestRule.run {
setContent { CircuitCompositionLocals(circuitConfig) { PetDetail(success) } }

onNodeWithTag(CAROUSEL_TAG).assertIsDisplayed().performTouchInput { swipeUp() }
onNodeWithTag(FULL_BIO_TAG, true).assertIsDisplayed().performClick()

val event = channel.receive()
assertThat(event).isEqualTo(PetDetailScreen.Event.ViewFullBio(success.url))
}
Comment on lines +158 to +166
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity why do we do these run blocks?

Suggested change
composeTestRule.run {
setContent { CircuitCompositionLocals(circuitConfig) { PetDetail(success) } }
onNodeWithTag(CAROUSEL_TAG).assertIsDisplayed().performTouchInput { swipeUp() }
onNodeWithTag(FULL_BIO_TAG, true).assertIsDisplayed().performClick()
val event = channel.receive()
assertThat(event).isEqualTo(PetDetailScreen.Event.ViewFullBio(success.url))
}
composeTestRule.setContent {
setContent { CircuitCompositionLocals(circuitConfig) { PetDetail(success) } }
}
onNodeWithTag(CAROUSEL_TAG).assertIsDisplayed().performTouchInput { swipeUp() }
onNodeWithTag(FULL_BIO_TAG, true).assertIsDisplayed().performClick()
val event = channel.receive()
assertThat(event).isEqualTo(PetDetailScreen.Event.ViewFullBio(success.url))

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All the onNodeWithX methods are extensions on SemanticsNodeInteractionsProvider which is implemented by the AndroidComposeTestRule (ie. composeTestRule). Without the run (or something similar) we'd need to prefix ever onNodeWithX with composeTestRule

Comment on lines +158 to +166
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity why do we do these run blocks?

Suggested change
composeTestRule.run {
setContent { CircuitCompositionLocals(circuitConfig) { PetDetail(success) } }
onNodeWithTag(CAROUSEL_TAG).assertIsDisplayed().performTouchInput { swipeUp() }
onNodeWithTag(FULL_BIO_TAG, true).assertIsDisplayed().performClick()
val event = channel.receive()
assertThat(event).isEqualTo(PetDetailScreen.Event.ViewFullBio(success.url))
}
composeTestRule.setContent {
setContent { CircuitCompositionLocals(circuitConfig) { PetDetail(success) } }
}
onNodeWithTag(CAROUSEL_TAG).assertIsDisplayed().performTouchInput { swipeUp() }
onNodeWithTag(FULL_BIO_TAG, true).assertIsDisplayed().performClick()
val event = channel.receive()
assertThat(event).isEqualTo(PetDetailScreen.Event.ViewFullBio(success.url))

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same comment here

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,17 @@
package com.slack.circuit.star

import android.app.Activity
import android.net.Uri
import android.os.Bundle
import androidx.activity.compose.setContent
import androidx.appcompat.app.AppCompatActivity
import androidx.browser.customtabs.CustomTabColorSchemeParams
import androidx.browser.customtabs.CustomTabsIntent
import androidx.browser.customtabs.CustomTabsIntent.COLOR_SCHEME_DARK
import androidx.browser.customtabs.CustomTabsIntent.COLOR_SCHEME_LIGHT
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Surface
import androidx.compose.runtime.remember
import androidx.lifecycle.ViewModelProvider
import com.slack.circuit.CircuitCompositionLocals
import com.slack.circuit.CircuitConfig
Expand All @@ -33,6 +39,8 @@ import com.slack.circuit.rememberCircuitNavigator
import com.slack.circuit.star.di.ActivityKey
import com.slack.circuit.star.di.AppScope
import com.slack.circuit.star.home.HomeScreen
import com.slack.circuit.star.navigator.AndroidScreen
import com.slack.circuit.star.navigator.LegacyNavigator
import com.slack.circuit.star.petdetail.PetDetailScreen
import com.slack.circuit.star.ui.StarTheme
import com.squareup.anvil.annotations.ContributesMultibinding
Expand Down Expand Up @@ -63,7 +71,9 @@ constructor(
// TODO why isn't the windowBackground enough so we don't need to do this?
Surface(color = MaterialTheme.colorScheme.background) {
val backstack = rememberSaveableBackStack { backStack.forEach { screen -> push(screen) } }
val navigator = rememberCircuitNavigator(backstack)
val circuitNavigator = rememberCircuitNavigator(backstack)
val navigator =
remember(circuitNavigator) { LegacyNavigator(circuitNavigator, this::goTo) }
CircuitCompositionLocals(circuitConfig) {
ContentWithOverlays { NavigableCircuitContent(navigator, backstack) }
}
Expand All @@ -75,4 +85,20 @@ constructor(
override fun getDefaultViewModelProviderFactory(): ViewModelProvider.Factory {
return viewModelProviderFactory
}

private fun goTo(screen: AndroidScreen) =
when (screen) {
is AndroidScreen.CustomTabsIntentScreen -> goTo(screen)
is AndroidScreen.IntentScreen -> TODO()
}

private fun goTo(screen: AndroidScreen.CustomTabsIntentScreen) {
val scheme = CustomTabColorSchemeParams.Builder().setToolbarColor(0x000000).build()
CustomTabsIntent.Builder()
.setColorSchemeParams(COLOR_SCHEME_LIGHT, scheme)
.setColorSchemeParams(COLOR_SCHEME_DARK, scheme)
.setShowTitle(true)
.build()
.launchUrl(this, Uri.parse(screen.url))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (C) 2022 Slack Technologies, 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
*
* https://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.slack.circuit.star.navigator

import android.content.Intent
import com.slack.circuit.Navigator
import com.slack.circuit.Screen
import kotlinx.parcelize.Parcelize

class LegacyNavigator(
private val navigator: Navigator,
private val onAndroidScreen: (AndroidScreen) -> Unit
) : Navigator by navigator {
override fun goTo(screen: Screen) =
when (screen) {
is AndroidScreen -> onAndroidScreen(screen)
else -> navigator.goTo(screen)
}
}

sealed interface AndroidScreen : Screen {
@Parcelize data class CustomTabsIntentScreen(val url: String) : AndroidScreen
@Parcelize data class IntentScreen(val intent: Intent) : AndroidScreen
}
Loading