-
Notifications
You must be signed in to change notification settings - Fork 7k
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
W.I.P. feat(ios): add PiP functionality #15168
Conversation
|
||
/** | ||
* The type of the React {@code Component} props of {@link Video}. | ||
*/ | ||
interface IProps { | ||
|
||
_enableIosPIP?: boolean; | ||
|
||
mirror: boolean; | ||
|
||
onPlaying: Function; |
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.
ISSUE: @typescript-eslint/ban-types (Severity: Medium)
Don't use Function
as a type. The Function
type accepts any function-like value.
It provides no type safety when calling the function, which can be a common source of bugs.
It also accepts things like class declarations, which will throw at runtime as they will not be called with new
.
If you are expecting the function to accept certain arguments, you should explicitly define the function shape.
Remediation:
I understand this is not part of the change but this is a good best practice recommendation from the automation. Please consider improving in the future
🤖 powered by PullRequest Automation 👋 verified by Xiaoyong W
} | ||
} | ||
|
||
if (_enableIosPIP) { |
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.
ISSUE: no-console (Severity: Medium)
Unexpected 'console' statement.
Remediation:
This is an automation generated comment regarding using console.log
. It's a good recommendation to avoid console
statement in production code. This includes several other console
usages in this code
🤖 powered by PullRequest Automation 👋 verified by Xiaoyong W
_enableIosPIP: iosPIP | ||
}; | ||
} | ||
|
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.
ISSUE: @typescript-eslint/ban-ts-comment (Severity: Medium)
Do not use "@ts-ignore" because it alters compilation errors.
Remediation:
This is a valid recommendation from automation to avoid using @ts-ignore
🤖 powered by PullRequest Automation 👋 verified by Xiaoyong W
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Is there any update on this PR? Will there be a release for mobile regarding this? |
We will pick it back up at a later time. |
What is blocking this PR from being merged? Does any development need to be done on the iOS side? |
It's not complete. |
Alright then, looking forward for it! |
Related to react-native-webrtc/react-native-webrtc#1629