Unify AppyxComponent composables #643
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KovalevAndrey
commented
Dec 15, 2023
| @@ -56,189 +19,27 @@ internal val defaultExtraTouch = 48.dp | |||
| fun <InteractionTarget : Any, ModelState : Any> ParentNode<InteractionTarget>.AppyxComponent( | |||
| appyxComponent: BaseAppyxComponent<InteractionTarget, ModelState>, | |||
| modifier: Modifier = Modifier, | |||
| draggables: ImmutableList<Draggable> = immutableListOf(appyxComponent), | |||
There was a problem hiding this comment.
I think we should remove it or find a better implementation. If we accidentally pass an empty list it will stop working
KovalevAndrey
commented
Dec 15, 2023
| @@ -73,7 +74,8 @@ class BackStackNode( | |||
| override fun resolve(interactionTarget: InteractionTarget, buildContext: BuildContext): Node = | |||
| when (interactionTarget) { | |||
| is InteractionTarget.Child -> node(buildContext) { | |||
| val backgroundColor = remember { colors.shuffled().random() } | |||
| val backgroundColor = | |||
There was a problem hiding this comment.
Just checking here that we save UI state for a Node correctly
manuel-martos
approved these changes
Dec 15, 2023
| @@ -5,6 +5,7 @@ | |||
| ### API breaking changes | |||
|
|
|||
| - [#630](https://github.com/bumble-tech/appyx/pull/630) – Pass initial state into Spotlights visualisations | |||
| - [#643](https://github.com/bumble-tech/appyx/pull/643) – Unify AppyxComponent composable between appyx-navigation and appyx-interactions modules | |||
There was a problem hiding this comment.
Any change needed on docs?
|
|
||
| return alignmentOffset | ||
| com.bumble.appyx.interactions.core.AppyxComponent( |
There was a problem hiding this comment.
Wouldn't it be nicer to import it with an alias and don't use fully qualified name here?
There was a problem hiding this comment.
Maybe, but not necessarily. Since it's just one method call I think it's fine like this as well
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
AppyxComponent in appyx-interactions and appyx-navigation are identical in their core and should reused as much code as possible to simplify support
Check list
CHANGELOG.mdif required.