diff --git a/app/src/main/java/net/skyscanner/backpack/demo/compose/AppSearchModalStory.kt b/app/src/main/java/net/skyscanner/backpack/demo/compose/AppSearchModalStory.kt index 37a3ec8dc..049d74572 100644 --- a/app/src/main/java/net/skyscanner/backpack/demo/compose/AppSearchModalStory.kt +++ b/app/src/main/java/net/skyscanner/backpack/demo/compose/AppSearchModalStory.kt @@ -18,7 +18,6 @@ package net.skyscanner.backpack.demo.compose -import android.util.Log import androidx.compose.foundation.Image import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -49,7 +48,6 @@ import net.skyscanner.backpack.compose.tokens.Airports import net.skyscanner.backpack.compose.tokens.City import net.skyscanner.backpack.compose.tokens.Landmark import net.skyscanner.backpack.compose.tokens.UseLocation -import net.skyscanner.backpack.compose.utils.BehaviouralCallback import net.skyscanner.backpack.demo.R import net.skyscanner.backpack.demo.components.AppSearchModalComponent import net.skyscanner.backpack.demo.meta.ComposeStory @@ -119,15 +117,6 @@ internal fun DefaultAppSearchModalSample( onClose = { showModal.value = false }, onInputChanged = { destination.value = it }, clearAction = BpkClearAction(stringResource(id = R.string.text_field_clear_action_description)) { destination.value = "" }, - behaviouralCallback = object : BehaviouralCallback { - override fun onDrawn(element: Any) { - Log.i("AppSearchModalStory", "onDrawn $element") - } - - override fun onClick(element: Any) { - Log.i("AppSearchModalStory", "onClick $element") - } - }, ) } } diff --git a/backpack-compose/src/androidTest/kotlin/net/skyscanner/backpack/compose/appsearchmodal/BpkAppSearchModalTest.kt b/backpack-compose/src/androidTest/kotlin/net/skyscanner/backpack/compose/appsearchmodal/BpkAppSearchModalTest.kt deleted file mode 100644 index e6a570a1e..000000000 --- a/backpack-compose/src/androidTest/kotlin/net/skyscanner/backpack/compose/appsearchmodal/BpkAppSearchModalTest.kt +++ /dev/null @@ -1,186 +0,0 @@ -/** - * Backpack for Android - Skyscanner's Design System - * - * Copyright 2018 Skyscanner Ltd - * - * 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 - * - * http://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 net.skyscanner.backpack.compose.appsearchmodal - -import androidx.compose.runtime.Composable -import androidx.compose.ui.test.junit4.createComposeRule -import androidx.compose.ui.test.onNodeWithText -import androidx.compose.ui.test.performClick -import androidx.compose.ui.text.buildAnnotatedString -import junit.framework.Assert.assertFalse -import kotlinx.coroutines.test.runTest -import net.skyscanner.backpack.compose.icon.BpkIcon -import net.skyscanner.backpack.compose.textfield.BpkClearAction -import net.skyscanner.backpack.compose.theme.BpkTheme -import net.skyscanner.backpack.compose.tokens.City -import net.skyscanner.backpack.compose.utils.BehaviouralCallback -import org.junit.Assert.assertTrue -import org.junit.Rule -import org.junit.Test - -class BpkAppSearchModalTest { - - @get:Rule - val composeTestRule = createComposeRule() - - @Test - fun withBehaviouralCallbackAndClickActionAdded() = runTest { - var didReceiveBehaviouralOnDrawnCallback = false - var didReceiveBehaviouralOnClickCallback = false - var didReceiveRegularOnClickCallback = false - - val onClickAction: () -> Unit = { - didReceiveRegularOnClickCallback = true - } - - val testLabel = "TestLabel" - - composeTestRule.setContent { - BpkTheme { - BpkAppSearchModal( - title = "Title", - inputText = "Input", - inputHint = "Hint", - results = contentResult(label = testLabel, onClickAction = onClickAction), - closeAccessibilityLabel = "Close", - onClose = { }, - onInputChanged = { }, - clearAction = BpkClearAction("Clear") { }, - behaviouralCallback = object : BehaviouralCallback { - override fun onDrawn(element: Any) { - didReceiveBehaviouralOnDrawnCallback = true - } - - override fun onClick(element: Any) { - didReceiveBehaviouralOnClickCallback = true - } - }, - ) - } - } - - // assert the correct state pre-click - assertTrue(didReceiveBehaviouralOnDrawnCallback) - assertFalse(didReceiveBehaviouralOnClickCallback) - assertFalse(didReceiveRegularOnClickCallback) - - // perform a click action - composeTestRule.onNodeWithText(testLabel).performClick() - - // assert the correct state post-click - assertTrue(didReceiveBehaviouralOnClickCallback) - assertTrue(didReceiveRegularOnClickCallback) - } - - @Test - fun withOnlyBehaviouralCallbackAdded() = runTest { - var didReceiveBehaviouralOnDrawnCallback = false - var didReceiveBehaviouralOnClickCallback = false - - val testLabel = "TestLabel" - - composeTestRule.setContent { - BpkTheme { - BpkAppSearchModal( - title = "Title", - inputText = "Input", - inputHint = "Hint", - results = contentResult(label = testLabel, onClickAction = { }), - closeAccessibilityLabel = "Close", - onClose = { }, - onInputChanged = { }, - clearAction = BpkClearAction("Clear") { }, - behaviouralCallback = object : BehaviouralCallback { - override fun onDrawn(element: Any) { - didReceiveBehaviouralOnDrawnCallback = true - } - - override fun onClick(element: Any) { - didReceiveBehaviouralOnClickCallback = true - } - }, - ) - } - } - - // assert the correct state pre-click - assertTrue(didReceiveBehaviouralOnDrawnCallback) - assertFalse(didReceiveBehaviouralOnClickCallback) - - // perform a click action - composeTestRule.onNodeWithText(testLabel).performClick() - - // assert the correct state post-click - assertTrue(didReceiveBehaviouralOnClickCallback) - } - - @Test - fun withOnlyClickActionAdded() = runTest { - var didReceiveRegularOnClickCallback = false - - val onClickAction: () -> Unit = { - didReceiveRegularOnClickCallback = true - } - - val testLabel = "TestLabel" - - composeTestRule.setContent { - BpkTheme { - BpkAppSearchModal( - title = "Title", - inputText = "Input", - inputHint = "Hint", - results = contentResult(label = testLabel, onClickAction = onClickAction), - closeAccessibilityLabel = "Close", - onClose = { }, - onInputChanged = { }, - clearAction = BpkClearAction("Clear") { }, - /* no behavioural callback added- the default value is null */ - ) - } - } - - // assert the correct state pre-click - assertFalse(didReceiveRegularOnClickCallback) - - // perform a click action - composeTestRule.onNodeWithText(testLabel).performClick() - - // assert the correct state post-click - assertTrue(didReceiveRegularOnClickCallback) - } - - @Composable - internal fun contentResult(label: String, onClickAction: () -> Unit) = BpkAppSearchModalResult.Content( - sections = listOf( - BpkSection( - headings = BpkSectionHeading(title = "Heading"), - items = listOf( - BpkItem( - title = buildAnnotatedString { "Title" }, - subtitle = buildAnnotatedString { "Subtitle" }, - icon = BpkIcon.City, - onItemSelected = { onClickAction.invoke() }, - tertiaryLabel = label, - ), - ), - ), - ), - ) -} diff --git a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/BpkAppSearchModal.kt b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/BpkAppSearchModal.kt index aa1efa3f5..c8e2a794c 100644 --- a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/BpkAppSearchModal.kt +++ b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/BpkAppSearchModal.kt @@ -30,7 +30,7 @@ import net.skyscanner.backpack.compose.modal.BpkModalState import net.skyscanner.backpack.compose.modal.rememberBpkModalState import net.skyscanner.backpack.compose.navigationbar.NavIcon import net.skyscanner.backpack.compose.textfield.BpkClearAction -import net.skyscanner.backpack.compose.utils.BehaviouralCallback +import net.skyscanner.backpack.compose.utils.BpkBehaviouralEventWrapper sealed class BpkAppSearchModalResult { data class Content( @@ -79,7 +79,7 @@ fun BpkAppSearchModal( clearAction: BpkClearAction, modifier: Modifier = Modifier, state: BpkModalState = rememberBpkModalState(), - behaviouralCallback: BehaviouralCallback? = null, + behaviouralEventWrapper: BpkBehaviouralEventWrapper? = null, ) { val coroutineScope = rememberCoroutineScope() BpkModal( @@ -101,7 +101,7 @@ fun BpkAppSearchModal( results = results, onInputChanged = onInputChanged, clearAction = clearAction, - behaviouralCallback = behaviouralCallback, + behaviouralEventWrapper = behaviouralEventWrapper, ) } } diff --git a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkAppSearchModalImpl.kt b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkAppSearchModalImpl.kt index a6b54b40d..36a68afa6 100644 --- a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkAppSearchModalImpl.kt +++ b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkAppSearchModalImpl.kt @@ -31,7 +31,7 @@ import net.skyscanner.backpack.compose.textfield.BpkClearAction import net.skyscanner.backpack.compose.textfield.BpkTextField import net.skyscanner.backpack.compose.tokens.BpkSpacing import net.skyscanner.backpack.compose.tokens.Search -import net.skyscanner.backpack.compose.utils.BehaviouralCallback +import net.skyscanner.backpack.compose.utils.BpkBehaviouralEventWrapper @Composable internal fun BpkAppSearchModalImpl( @@ -40,8 +40,8 @@ internal fun BpkAppSearchModalImpl( results: BpkAppSearchModalResult, onInputChanged: (String) -> Unit, clearAction: BpkClearAction, - behaviouralCallback: BehaviouralCallback?, modifier: Modifier = Modifier, + behaviouralEventWrapper: BpkBehaviouralEventWrapper? = null, ) { when (results) { is BpkAppSearchModalResult.Error -> { @@ -63,7 +63,7 @@ internal fun BpkAppSearchModalImpl( clearAction = clearAction, ) if (results is BpkAppSearchModalResult.Content) { - BpkSearchModalContent(results = results, behaviouralCallback = behaviouralCallback) + BpkSearchModalContent(results = results, behaviouralEventWrapper = behaviouralEventWrapper) } else if (results is BpkAppSearchModalResult.Loading) { BpkSearchModalLoading(results) } diff --git a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkSearchModalContent.kt b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkSearchModalContent.kt index 504a2f79f..e2d7408cb 100644 --- a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkSearchModalContent.kt +++ b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkSearchModalContent.kt @@ -24,13 +24,13 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import net.skyscanner.backpack.compose.appsearchmodal.BpkAppSearchModalResult import net.skyscanner.backpack.compose.tokens.BpkSpacing -import net.skyscanner.backpack.compose.utils.BehaviouralCallback +import net.skyscanner.backpack.compose.utils.BpkBehaviouralEventWrapper @Composable internal fun BpkSearchModalContent( results: BpkAppSearchModalResult.Content, - behaviouralCallback: BehaviouralCallback?, modifier: Modifier = Modifier, + behaviouralEventWrapper: BpkBehaviouralEventWrapper? = null, ) { LazyColumn(modifier = modifier) { results.shortcuts?.let { @@ -50,11 +50,20 @@ internal fun BpkSearchModalContent( } } items(section.items) { - BpkSectionItem( - item = it, - modifier = Modifier.padding(BpkSpacing.Base), - behaviouralCallback = behaviouralCallback, - ) + if (behaviouralEventWrapper != null) { + behaviouralEventWrapper(it, Modifier) { + BpkSectionItem( + item = it, + modifier = Modifier.padding(BpkSpacing.Base), + clickHandleScope = this, + ) + } + } else { + BpkSectionItem( + item = it, + modifier = Modifier.padding(BpkSpacing.Base), + ) + } } } } diff --git a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkSection.kt b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkSection.kt index 4a7c6bf94..d8bc1aa9c 100644 --- a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkSection.kt +++ b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/appsearchmodal/internal/BpkSection.kt @@ -36,8 +36,8 @@ import net.skyscanner.backpack.compose.icon.BpkIconSize import net.skyscanner.backpack.compose.text.BpkText import net.skyscanner.backpack.compose.theme.BpkTheme import net.skyscanner.backpack.compose.tokens.BpkSpacing -import net.skyscanner.backpack.compose.utils.BehaviouralCallback -import net.skyscanner.backpack.compose.utils.registerForBehaviouralEvents +import net.skyscanner.backpack.compose.utils.BpkClickHandleScope +import net.skyscanner.backpack.compose.utils.clickable @Composable internal fun BpkSectionHeading( @@ -65,11 +65,14 @@ internal fun BpkSectionHeading( } @Composable -internal fun BpkSectionItem(item: BpkItem, modifier: Modifier = Modifier, behaviouralCallback: BehaviouralCallback? = null) { +internal fun BpkSectionItem(item: BpkItem, modifier: Modifier = Modifier, clickHandleScope: BpkClickHandleScope? = null) { Row( modifier = modifier .fillMaxWidth() - .registerForBehaviouralEvents(item = item, behaviouralCallback = behaviouralCallback, onClick = item.onItemSelected), + .clickable { + item.onItemSelected() + clickHandleScope?.notifyClick() + }, verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(BpkSpacing.Base), ) { diff --git a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/utils/BehaviouralModifiers.kt b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/utils/BehaviouralModifiers.kt deleted file mode 100644 index cfceac950..000000000 --- a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/utils/BehaviouralModifiers.kt +++ /dev/null @@ -1,62 +0,0 @@ -/** - * Backpack for Android - Skyscanner's Design System - * - * Copyright 2018 Skyscanner Ltd - * - * 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 - * - * http://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 net.skyscanner.backpack.compose.utils - -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.runtime.setValue -import androidx.compose.ui.Modifier -import androidx.compose.ui.composed -import androidx.compose.ui.draw.drawBehind - -/** - * This registers the component for Behavioural Events. - * Right now the two supported types are "onDrawn" and "onClick" events. - * - * @param item the component that was clicked on. Any handling of types must be by the consumer. - * @param behaviouralCallback the callback to use for events. - * @param onClick the additional callback to use for click events. This is to allow wrapping of existing onClick behaviour - * without duplicating code in the consumer. - */ -internal fun Modifier.registerForBehaviouralEvents( - item: Any, - behaviouralCallback: BehaviouralCallback?, - onClick: () -> Unit, -): Modifier = composed { - - if (behaviouralCallback == null) { - this - } else { - var wasDrawn by rememberSaveable { mutableStateOf(false) } - drawBehind { - if (!wasDrawn) { - wasDrawn = true - behaviouralCallback.onDrawn(item) - } - } - } -}.clickable { - behaviouralCallback?.onClick(item) - onClick.invoke() -} - -interface BehaviouralCallback { - fun onDrawn(element: Any) - fun onClick(element: Any) -} diff --git a/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/utils/BpkBehaviouralEventWrapper.kt b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/utils/BpkBehaviouralEventWrapper.kt new file mode 100644 index 000000000..b7d4d0061 --- /dev/null +++ b/backpack-compose/src/main/kotlin/net/skyscanner/backpack/compose/utils/BpkBehaviouralEventWrapper.kt @@ -0,0 +1,33 @@ +/** + * Backpack for Android - Skyscanner's Design System + * + * Copyright 2018 Skyscanner Ltd + * + * 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 + * + * http://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 net.skyscanner.backpack.compose.utils + +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier + +/** + * This is used for registering components for behavioural events. Right now this supports `onClick` events, while any other + * events must be handled by the consumer. + */ + +typealias BpkBehaviouralEventWrapper = @Composable (item: Any, modifier: Modifier, content: @Composable BpkClickHandleScope.() -> Unit) -> Unit + +interface BpkClickHandleScope { + fun notifyClick() +}