-
Notifications
You must be signed in to change notification settings - Fork 109
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
Permalink navigation - Navigation to rooms and users #2713
Conversation
f4b702a
to
84d0442
Compare
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
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.
Good work, thanks!
Some first remarks
appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt
Outdated
Show resolved
Hide resolved
appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt
Outdated
Show resolved
Hide resolved
appnav/src/main/kotlin/io/element/android/appnav/room/joined/JoinedRoomLoadedFlowNode.kt
Outdated
Show resolved
Hide resolved
appnav/src/main/kotlin/io/element/android/appnav/room/resolver/ResolveRoomModule.kt
Outdated
Show resolved
Hide resolved
appnav/src/main/kotlin/io/element/android/appnav/room/resolver/RoomAliasResolverView.kt
Outdated
Show resolved
Hide resolved
appnav/src/main/kotlin/io/element/android/appnav/room/resolver/RoomAliasResolverView.kt
Outdated
Show resolved
Hide resolved
import java.util.Optional | ||
|
||
interface JoinRoomEntryPoint : FeatureEntryPoint { | ||
fun createNode(parentNode: Node, buildContext: BuildContext, inputs: Inputs): Node | ||
|
||
data class Inputs( | ||
val roomId: RoomId, | ||
val roomIdOrAlias: RoomIdOrAlias, |
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'd use an Optional<RoomAlias>
instead
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.
That would be more correct, hence it brings some complexity.
At some point the room resolver should probably be moved in the joinroom
module (I am more and more convinced about this approach), and so, the first param roomId
should just be removed.
...joinroom/impl/src/main/kotlin/io/element/android/features/joinroom/impl/JoinRoomPresenter.kt
Outdated
Show resolved
Hide resolved
...joinroom/impl/src/main/kotlin/io/element/android/features/joinroom/impl/JoinRoomPresenter.kt
Outdated
Show resolved
Hide resolved
...ures/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesNode.kt
Outdated
Show resolved
Hide resolved
- prepare navigating to an Event - add NodeBuilder to MessagesEntryPoint
984b197
to
35de787
Compare
…crease end padding to ensure that rendering is correct for big numbers.
…rate screenshots.
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.
Thanks for the changes, LGTM!
Quality Gate passedIssues Measures |
RoomAlias
value class and move many String toRoomAlias
RoomIdOrAlias
PermalinkData
classes