-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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
feat(inIframe/isEmbedded) add ability to detect generic embedding #15724
Conversation
3a0e93f
to
2696d64
Compare
LGTM, but let's wait and for @hristoterezov :) |
react/features/base/config/isEmbeddedInterfaceConfigWhitelist.ts
Outdated
Show resolved
Hide resolved
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 still like the alternative approach better where we allow to override the options for all RN use cases and then have a whitelist in the JitsiMeet App for the things we care about from the apps perspective. In practice this is the same thing and we just have exception for the JitsiMeet App but for any other RN app that uses the SDK those will be allowed and it will be apps responsibility to not pass unverified values for these props. So I don't see much value in the exception for our app...
But this is personal preference and I'd say even this approach is acceptable although I like more the other one.
Added some questions and comments. I think we can definitely reduce the list of the allowed config props.
@hristoterezov PTAL. It now includes the timeout implementation for mobile. |
Jenkins please test this please. |
* Additional interface config whitelist extending the original whitelist applied when Jitsi Meet is embedded | ||
* in another app be that with an iframe or a mobile SDK. | ||
*/ | ||
export default [ |
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 think this array needs to be empty!
On web we detect if we run on an iframe, and on mobile we detect if the app is one of ours.
@damencho Tests passed but the CI choked on something regarding my branch:
|
On web we detect if we run on an iframe, and on mobile we detect if the app is one of ours.