-
Notifications
You must be signed in to change notification settings - Fork 50
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
Refactor rib connector #53
Merged
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
This was referenced May 22, 2019
Merged
yay! |
ShikaSD
reviewed
May 23, 2019
...p-example/src/main/java/com/badoo/ribs/example/rib/dialog_example/DialogExampleInteractor.kt
Show resolved
Hide resolved
...d/app-example/src/main/java/com/badoo/ribs/example/rib/dialog_example/DialogExampleRouter.kt
Show resolved
Hide resolved
...ies/rib-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommand.kt
Show resolved
Hide resolved
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Outdated
Show resolved
Hide resolved
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Outdated
Show resolved
Hide resolved
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Show resolved
Hide resolved
...ies/rib-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationContext.kt
Outdated
Show resolved
Hide resolved
.../main/java/com/badoo/ribs/core/routing/configuration/action/multi/SaveInstanceStateAction.kt
Show resolved
Hide resolved
...base/src/main/java/com/badoo/ribs/core/routing/configuration/feature/ConfigurationFeature.kt
Show resolved
Hide resolved
Nublo
reviewed
May 23, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will continue reviewing tomorrow, looks like there are a lot of changes and I don't see the big picture.
android/libraries/rib-base/src/main/java/com/badoo/ribs/core/Interactor.kt
Show resolved
Hide resolved
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Show resolved
Hide resolved
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Show resolved
Hide resolved
...ies/rib-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationContext.kt
Show resolved
Hide resolved
lukaville
reviewed
May 24, 2019
...id/libraries/rib-base/src/androidTest/java/com/badoo/ribs/android/lifecycle/BaseNodesTest.kt
Outdated
Show resolved
Hide resolved
lukaville
reviewed
May 24, 2019
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Outdated
Show resolved
Hide resolved
lukaville
reviewed
May 24, 2019
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Outdated
Show resolved
Hide resolved
lukaville
reviewed
May 24, 2019
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Outdated
Show resolved
Hide resolved
lukaville
reviewed
May 24, 2019
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Show resolved
Hide resolved
lukaville
reviewed
May 24, 2019
...rib-base/src/main/java/com/badoo/ribs/core/routing/configuration/feature/BackStackFeature.kt
Outdated
Show resolved
Hide resolved
lukaville
reviewed
May 28, 2019
...-base/src/main/java/com/badoo/ribs/core/routing/configuration/ConfigurationCommandCreator.kt
Show resolved
Hide resolved
lukaville
reviewed
May 28, 2019
...rib-base/src/main/java/com/badoo/ribs/core/routing/configuration/feature/BackStackFeature.kt
Outdated
Show resolved
Hide resolved
lukaville
reviewed
May 28, 2019
...rib-base/src/main/java/com/badoo/ribs/core/routing/configuration/feature/BackStackFeature.kt
Outdated
Show resolved
Hide resolved
lukaville
reviewed
May 28, 2019
android/libraries/rib-base/src/test/java/com/badoo/ribs/core/RouterTest.kt
Show resolved
Hide resolved
lukaville
reviewed
May 28, 2019
...ies/rib-base/src/test/java/com/badoo/ribs/core/routing/configuration/BackStackFeatureTest.kt
Outdated
Show resolved
Hide resolved
lukaville
reviewed
May 28, 2019
...rib-base/src/test/java/com/badoo/ribs/core/routing/configuration/ConfigurationFeatureTest.kt
Outdated
Show resolved
Hide resolved
ShikaSD
approved these changes
May 28, 2019
lukaville
approved these changes
May 28, 2019
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.
Lots have changed. The result is a better separation of concerns, easier maintenance, improved understandability (I know there are lots of new concepts, but in total they should be more manageable than the mess what it was before).
BackStackFeature
only cares about back stack as a list of elements. Has no knowledge aboutNode
s,RoutingAction
s, or anything related. All those concerns are extracted.ConfigurationCommand
describes what needs to be done to a Configuration (reminder: Configuration is what aNode
can be in - it is triggered by the actualInteractor
implementation, and resolved toRoutingAction
in actualRouter
implementation. AnyRoutingAction
can result in buildingNodes
or doing other things, such as showing a dialog, or triggering some lambda, like how we update the menu inSwitcherRouter
).ConfigurationCommandCreator
takes two states ofBackStackFeature
, calculates the diff, and translates it to a list ofConfigurationCommand
objects representing how to get from State0 to State1. This thing needed to be done manually until now, now it's fully automatic.ConfigurationFeature
cares about executingConfigurationCommand
s. It manages a pool of configurations + relatedNode
s &RoutingAction
s, without the knowledge of the back stack. All elements are referenced using keys instead. This allows to handle Content-type, Overlay-type, and Permanent parts in a unified way.Router
, outputs of theBackStackFeature
is connected to theConfigurationFeature
via the diffing done inConfigurationCommandCreator
, resulting in all back stack changes being implemented automatically. The flow is:BackStackFeature
, a changed list comes outConfigurationFeature
.Router
sometimes triggers global actions directly inConfigurationFeature
regardless of back stack (e.g. all currently on-screen Configurations need to be detached when view is detached, even if the back stack doesn't change)BackStackFeature
andConfigurationFeature
are automatically persisted and restored fromBundle
Node
builders, but a list of configurations. This is both more powerful (Configuration can be resolved to Node-attachingRoutingAction
but also to anything else), and more useful, since then it can be handled the same way asContent
orOverlay
.Overlay
is no longer a marker interface, but a generic type, making it compile-time safe to use. The downside is thatRouter
andInteractor
has more generic types to know about, but I believe it to be a reasonable trade-off for all the benefits it brings.onLowMemory()
. Will add it back later.