-
Notifications
You must be signed in to change notification settings - Fork 731
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
Feature/fga/auto accept invite #3531
Conversation
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.
LGTM, just some minor remarks
@OnLifecycleEvent(Lifecycle.Event.ON_PAUSE) | ||
fun entersBackground() { | ||
compositeDisposable.clear() |
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.
Never disposed then? this change is a bit weird here.
Maybe move the code from init {}
to entersForeground()
?
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.
Nope I want the invites to be accepted when the app is in background so I can receive calls.
Its just to keep a reference to the disposable so it doesn't get GC
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.
So you will restore this line?
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!
// also auto-join the virtual room if we have a matching native room | ||
// (possibly we should only join if we've also joined the native room, then we'd also have | ||
// to make sure we joined virtual rooms on joining a native one) | ||
session.joinRoom(invitedRoomId) |
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.
test the compil flag?
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 don't think so, auto join virtual room is not mandatory
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.
If you say so :)
).size | ||
var dmInvites = 0 | ||
var roomsInvite = 0 | ||
if (!autoAcceptInvites.hideInvites) { |
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.
Could be nicer if we also had a method autoAcceptInvites.showInvites()
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.
Yes
|
||
interface AutoAcceptInvites { | ||
val isEnabled: Boolean | ||
val hideInvites: Boolean |
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.
Default could be = isEnabled
in the itf, no?
Also would be nice to add some Kdoc
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.
Yes will add some.
e209f6e
to
48fa9e1
Compare
This PR introduces AutoAcceptInvite. It can be enabled at compile time for now.