Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

Jitsi Widget Permissions #3393

Merged
merged 6 commits into from
Nov 22, 2019
Merged

Jitsi Widget Permissions #3393

merged 6 commits into from
Nov 22, 2019

Conversation

BillCarsonFr
Copy link
Member

@BillCarsonFr BillCarsonFr commented Nov 20, 2019

Fixes #3391

Depends on matrix-org/matrix-android-sdk#514

Pull Request Checklist

image

  • Changes has been tested on an Android device or Android emulator with API 16
  • UI change has been tested on both light and dark themes
  • Pull request is based on the develop branch
  • Pull request updates CHANGES.rst
  • Pull request includes screenshots or videos if containing UI changes

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few remarks

}
return Unit.INSTANCE;
});
bs.show(getSupportFragmentManager(), "JitsyPerm");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: 'JitsiPerm' with a "i" :)

@@ -65,7 +65,11 @@ class WidgetActivity : VectorAppCompatActivity() {
//already there
} else {
RoomWidgetPermissionBottomSheet
.newInstance(viewModel.session!!.myUserId, viewModel.widget)
.newInstance(viewModel.session!!.myUserId, viewModel.widget).apply {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this code is robust to screen rotation, will the bottomSheet is displayed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worst case you have to tap again on the video conf banner


private val sharedActivityViewModel: RoomWidgetViewModel by activityViewModel()
var onFinish: ((Boolean) -> Unit)? = null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sharedViewModel was better, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes the bottom sheet more generic, can be used in other activity easier

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, thanks for the update

@@ -1772,7 +1772,7 @@ private void launchJitsiActivity(Widget widget, boolean aIsVideoCall) {

String domain = UrlUtilKt.extractDomain(JitsiCallActivity.JITSI_SERVER_URL);
if (domain == null) return; //display a toast?
boolean isAllowed = mSession.getIntegrationManager().isVideoConfDomainAllowed(domain);
boolean isAllowed = mSession.getIntegrationManager().isNativeWidgetAllowed("jitsi", domain);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@BillCarsonFr BillCarsonFr merged commit 788f15f into develop Nov 22, 2019
@BillCarsonFr BillCarsonFr deleted the feature/jitsi_perm branch November 22, 2019 12:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Widget Permission for jitsi widgets
2 participants