-
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
W.I.P. feat(ios): add PiP functionality #15168
Changes from 1 commit
3561849
2543e9e
15db4a2
8fa68b1
93d884e
a3c3b34
a6105a1
84b0359
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { View } from 'react-native'; | ||
|
||
const FallbackView = () => { | ||
|
||
return( | ||
<View style={{ height: 800, width: 400, backgroundColor: 'red' }} /> | ||
) | ||
} | ||
|
||
export default FallbackView; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,27 @@ | ||
import React, { Component } from 'react'; | ||
import React, {Component, RefObject} from 'react'; | ||
import { GestureResponderEvent } from 'react-native'; | ||
import { MediaStream, RTCView } from 'react-native-webrtc'; | ||
import { connect } from 'react-redux'; | ||
|
||
import { MediaStream, RTCPIPView, startIOSPIP, stopIOSPIP } from 'react-native-webrtc'; | ||
|
||
import { IReduxState} from '../../../../app/types'; | ||
import { translate } from '../../../i18n/functions'; | ||
import Pressable from '../../../react/components/native/Pressable'; | ||
|
||
import logger from '../../logger'; | ||
|
||
import VideoTransform from './VideoTransform'; | ||
import styles from './styles'; | ||
import FallbackView from "./FallbackView"; | ||
|
||
|
||
/** | ||
* The type of the React {@code Component} props of {@link Video}. | ||
*/ | ||
interface IProps { | ||
|
||
_enableIosPIP?: boolean; | ||
|
||
mirror: boolean; | ||
|
||
onPlaying: Function; | ||
|
@@ -58,7 +69,15 @@ interface IProps { | |
* {@code HTMLVideoElement} and wraps around react-native-webrtc's | ||
* {@link RTCView}. | ||
*/ | ||
export default class Video extends Component<IProps> { | ||
class Video extends Component<IProps> { | ||
|
||
_ref: RefObject<typeof Video>; | ||
|
||
constructor(props) { | ||
super(props); | ||
this._ref = React.createRef(); | ||
} | ||
|
||
/** | ||
* React Component method that executes once component is mounted. | ||
* | ||
|
@@ -79,7 +98,28 @@ export default class Video extends Component<IProps> { | |
* @returns {ReactElement|null} | ||
*/ | ||
render() { | ||
const { onPress, stream, zoomEnabled } = this.props; | ||
const { _enableIosPIP, onPress, stream, zoomEnabled } = this.props; | ||
|
||
const iosPIPOptions = { | ||
startAutomatically: true, | ||
fallbackView: (<FallbackView />), | ||
preferredSize: { | ||
width: 400, | ||
height: 800, | ||
} | ||
} | ||
|
||
if (_enableIosPIP) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ISSUE: no-console (Severity: Medium) Remediation: 🤖 powered by PullRequest Automation 👋 verified by Xiaoyong W |
||
console.log('TESTING _enableIosPIP is', _enableIosPIP); | ||
console.log(this._ref?.current, 'Picture in picture mode on'); | ||
// logger.warn(this._ref?.current, `Picture in picture mode on`); | ||
// startIOSPIP(this._ref?.current); | ||
} else { | ||
console.log('TESTING _enableIosPIP is', _enableIosPIP); | ||
console.log('TESTING Picture in picture mode off'); | ||
// logger.warn(`Picture in picture mode off`); | ||
// stopIOSPIP(this._ref?.current); | ||
} | ||
|
||
if (stream) { | ||
// RTCView | ||
|
@@ -90,9 +130,11 @@ export default class Video extends Component<IProps> { | |
: 'cover'; | ||
const rtcView | ||
= ( | ||
<RTCView | ||
<RTCPIPView | ||
iosPIP = { iosPIPOptions } | ||
mirror = { this.props.mirror } | ||
objectFit = { objectFit } | ||
ref = { this._ref } | ||
streamURL = { stream.toURL() } | ||
style = { style } | ||
zOrder = { this.props.zOrder } /> | ||
|
@@ -132,3 +174,20 @@ export default class Video extends Component<IProps> { | |
return null; | ||
} | ||
} | ||
|
||
/** | ||
* Maps part of the Redux state to the props of this component. | ||
* | ||
* @param {Object} state - The Redux state. | ||
* @returns {Object} | ||
*/ | ||
function _mapStateToProps(state: IReduxState) { | ||
const iosPIP = state['features/mobile/picture-in-picture']?.enableIosPIP | ||
|
||
return { | ||
_enableIosPIP: iosPIP | ||
}; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ISSUE: @typescript-eslint/ban-ts-comment (Severity: Medium) Remediation: 🤖 powered by PullRequest Automation 👋 verified by Xiaoyong W |
||
// @ts-ignore | ||
export default translate(connect(_mapStateToProps)(Video)); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import MiddlewareRegistry from '../../base/redux/MiddlewareRegistry'; | ||
import { APP_STATE_CHANGED } from '../background/actionTypes'; | ||
import { ENABLE_IOS_PIP } from './actionTypes'; | ||
|
||
MiddlewareRegistry.register(store => next => action => { | ||
const result = next(action); | ||
|
||
switch (action.type) { | ||
case APP_STATE_CHANGED: { | ||
const state = store.getState(); | ||
const { dispatch } = store; | ||
const { appState } = state['features/background']; | ||
|
||
if (appState === 'inactive') { | ||
dispatch({ | ||
type: ENABLE_IOS_PIP, | ||
enableIosPIP: false }) | ||
} | ||
break; | ||
} | ||
} | ||
|
||
return result; | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import ReducerRegistry from '../../base/redux/ReducerRegistry'; | ||
import { ENABLE_IOS_PIP } from './actionTypes'; | ||
|
||
const DEFAULT_STATE = { | ||
enableIosPIP: false | ||
}; | ||
|
||
export interface IMobilePictureInPictureState { | ||
enableIosPIP: boolean; | ||
} | ||
|
||
const STORE_NAME = 'features/mobile/picture-in-picture'; | ||
|
||
ReducerRegistry.register<IMobilePictureInPictureState>(STORE_NAME, (state = DEFAULT_STATE, action): IMobilePictureInPictureState => { | ||
switch (action.type) { | ||
|
||
case ENABLE_IOS_PIP: { | ||
const { enableIosPIP } = action; | ||
|
||
return { | ||
...state, | ||
enableIosPIP | ||
} | ||
} | ||
|
||
default: | ||
return state; | ||
} | ||
}) |
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. TheFunction
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