-
Notifications
You must be signed in to change notification settings - Fork 7.9k
feat(documentPiP): auto open on tab switch and trigger button #17450
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
base: gsoc2026-document-pip
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,11 +7,21 @@ import { muteLocal } from '../video-menu/actions.any'; | |
| import { SET_PIP_ACTIVE } from './actionTypes'; | ||
| import { | ||
| cleanupMediaSessionHandlers, | ||
| enterPiP, | ||
| clearPiPWindow, | ||
| enterVideoPiP, | ||
| getStoredPiPWindow, | ||
| initPiPWindow, | ||
| setupMediaSessionHandlers, | ||
| shouldShowPiP | ||
| shouldShowPiP, | ||
| } from './functions'; | ||
| import logger from './logger'; | ||
| import { getDocumentPiPWindow, isDocumentPiPOpen, isDocumentPiPSupported } from "./utils"; | ||
|
|
||
| /** | ||
| * Flag to track if a Document PiP request is currently pending. | ||
| * Prevents duplicate requestWindow() calls before the first one resolves. | ||
| */ | ||
| let docPiPPending = false; | ||
|
|
||
| /** | ||
| * Action to set Picture-in-Picture active state. | ||
|
|
@@ -63,21 +73,29 @@ export function toggleVideoFromPiP() { | |
|
|
||
| /** | ||
| * Action to exit Picture-in-Picture mode. | ||
| * Handles both Document PiP and Video PiP. | ||
| * | ||
| * @returns {Function} | ||
| */ | ||
| export function exitPiP() { | ||
| return (dispatch: IStore['dispatch']) => { | ||
| logger.debug('exitPiP called'); | ||
|
|
||
| const pipWindow = getStoredPiPWindow(); | ||
|
|
||
| if (pipWindow && !pipWindow.closed) { | ||
| pipWindow.close(); | ||
| clearPiPWindow(); | ||
| } | ||
|
|
||
| if (document.pictureInPictureElement) { | ||
| document.exitPictureInPicture() | ||
| .then(() => { | ||
| .then(() => { | ||
| logger.debug('Exited Picture-in-Picture mode'); | ||
| }) | ||
| .catch((err: Error) => { | ||
| logger.error(`Error while exiting PiP: ${err.message}`); | ||
| }); | ||
| }) | ||
| .catch((err: Error) => { | ||
| logger.error(`Error while exiting PiP: ${err.message}`); | ||
| }); | ||
| } | ||
|
|
||
| dispatch(setPiPActive(false)); | ||
|
|
@@ -100,7 +118,7 @@ export function handleWindowBlur(videoElement: HTMLVideoElement) { | |
| logger.debug(`Window blur detected, isPiPActive=${isPiPActive}`); | ||
|
|
||
| if (!isPiPActive) { | ||
| enterPiP(videoElement); | ||
| enterVideoPiP(videoElement); | ||
| } | ||
| }; | ||
| } | ||
|
|
@@ -163,7 +181,7 @@ export function handlePipEnterEvent() { | |
| * @returns {Function} | ||
| */ | ||
| export function showPiP() { | ||
| return (_dispatch: IStore['dispatch'], getState: IStore['getState']) => { | ||
| return (dispatch: IStore['dispatch'], getState: IStore['getState']) => { | ||
| const state = getState(); | ||
| const isPiPActive = state['features/pip']?.isPiPActive; | ||
| const _shouldShowPip = shouldShowPiP(state); | ||
|
|
@@ -175,15 +193,19 @@ export function showPiP() { | |
| } | ||
|
|
||
| if (!isPiPActive) { | ||
| const videoElement = document.getElementById('pipVideo') as HTMLVideoElement; | ||
| if (isDocumentPiPSupported()) { | ||
| dispatch(openDocumentPiP()); | ||
| } else { | ||
| const videoElement = document.getElementById('pipVideo') as HTMLVideoElement; | ||
|
|
||
| if (!videoElement) { | ||
| logger.warn('showPiP: pipVideo element not found'); | ||
| if (!videoElement) { | ||
| logger.warn('showPiP: pipVideo element not found'); | ||
|
|
||
| return; | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| enterPiP(videoElement); | ||
| enterVideoPiP(videoElement); | ||
| } | ||
| } | ||
| }; | ||
| } | ||
|
|
@@ -206,3 +228,93 @@ export function hidePiP() { | |
| } | ||
| }; | ||
| } | ||
|
|
||
| export function enterPiP() { | ||
|
Contributor
Author
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. I wanted to reuse existing showPiP function only. But the guard shouldShowPiP() is preventing for tab switch cases as we want to always open it. This can be merged together I think |
||
| return (dispatch: IStore["dispatch"]) => { | ||
| if (isDocumentPiPSupported()) { | ||
| const pipWindow = getDocumentPiPWindow(); | ||
|
|
||
| if (pipWindow) { | ||
| dispatch(exitPiP()); | ||
| } else { | ||
| dispatch(openDocumentPiP()); | ||
| } | ||
| } else { | ||
| const videoElement = document.getElementById("pipVideo") as HTMLVideoElement; | ||
|
|
||
| if (videoElement) { | ||
| enterVideoPiP(videoElement); | ||
| } | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| export function openDocumentPiP() { | ||
| return (dispatch: IStore["dispatch"], getState: IStore["getState"]) => { | ||
| if (!isDocumentPiPSupported()) { | ||
| logger.warn("Document Picture-in-Picture not supported, use Video PiP button"); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const state = getState(); | ||
| const pipConfig = state["features/base/config"]?.pip; | ||
| const docPiPConfig = pipConfig?.documentPiP.windowOptions; | ||
|
|
||
| if (isDocumentPiPOpen() || getStoredPiPWindow()) { | ||
| return; | ||
| } | ||
|
|
||
| if (docPiPPending) { | ||
| logger.debug('Document PiP request already pending, skipping duplicate request'); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const docPiP = (window as any).documentPictureInPicture; | ||
|
|
||
| if (!docPiP) { | ||
| logger.warn("Document Picture-in-Picture not available"); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| docPiPPending = true; | ||
|
|
||
| try { | ||
| const promise = docPiP.requestWindow({ | ||
| width: docPiPConfig?.width ?? 600, | ||
| height: docPiPConfig?.height ?? 450, | ||
| disallowReturnToOpener: docPiPConfig?.disallowReturnToOpener ?? false, | ||
| preferInitialWindowPlacement: docPiPConfig?.preferInitialWindowPlacement ?? false, | ||
| }); | ||
|
|
||
| return promise | ||
| .then((pipWindow: Window) => { | ||
| initPiPWindow(pipWindow); | ||
|
|
||
| dispatch(setPiPActive(true)); | ||
|
|
||
| pipWindow.addEventListener("pagehide", () => { | ||
| clearPiPWindow(); | ||
| dispatch(setPiPActive(false)); | ||
| }); | ||
| }) | ||
| .catch((error: Error) => { | ||
| logger.error("Failed to open Document PiP:", error); | ||
| dispatch(setPiPActive(false)); | ||
|
|
||
| throw error; | ||
| }) | ||
| .finally(() => { | ||
| docPiPPending = false; | ||
| }); | ||
| } catch (error) { | ||
| docPiPPending = false; | ||
| logger.error("Failed to open Document PiP:", error); | ||
| dispatch(setPiPActive(false)); | ||
|
|
||
| throw error; | ||
| } | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,54 @@ | ||
| import React from 'react'; | ||
| import React, { useRef } from 'react'; | ||
| import { useSelector } from 'react-redux'; | ||
|
|
||
| import { IReduxState } from '../../app/types'; | ||
| import { MEDIA_TYPE } from '../../base/media/constants'; | ||
| import { isLocalTrackMuted } from '../../base/tracks/functions.any'; | ||
| import { shouldShowPiP } from '../functions'; | ||
| import { useDocumentPiPMediaSession } from '../hooks'; | ||
|
|
||
| import PiPVideoElement from './PiPVideoElement'; | ||
|
|
||
| /** | ||
| * Wrapper component that conditionally renders PiPVideoElement. | ||
| * Prevents mounting when PiP is disabled or on prejoin without showOnPrejoin flag. | ||
| * Inner component for the Document PiP. | ||
| * | ||
| * @returns {React.ReactElement} | ||
| */ | ||
| function DocumentPiPContent() { | ||
|
Contributor
Author
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. this can be more polished I think |
||
| const playerRef = useRef<HTMLDivElement>(null); | ||
| const containerRef = useRef<HTMLDivElement>(null); | ||
| const audioMuted = useSelector( | ||
| (state: IReduxState) => isLocalTrackMuted(state['features/base/tracks'], MEDIA_TYPE.AUDIO)); | ||
| const videoMuted = useSelector( | ||
| (state: IReduxState) => isLocalTrackMuted(state['features/base/tracks'], MEDIA_TYPE.VIDEO)); | ||
|
|
||
| useDocumentPiPMediaSession(playerRef, containerRef, !audioMuted, !videoMuted); | ||
|
|
||
| return ( | ||
| <div id = 'document-pip-container' ref = {containerRef}> | ||
| <div id = 'document-pip-player' ref = {playerRef}> | ||
| {/* TODO: document pip contents */} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper component that selects the appropriate PiP implementation. | ||
| * Uses Document PiP API when available, falls back to Video PiP. | ||
| * | ||
| * @returns {React.ReactElement | null} | ||
| */ | ||
| function PiP() { | ||
| const showPiP = useSelector(shouldShowPiP); | ||
|
|
||
| // Document PiP must mount regardless of shouldShowPiP | ||
| // because useDocumentPiPMediaSession registers the enterpictureinpicture | ||
| // MediaSession handler needed for tab-switch auto-open. | ||
| if ('documentPictureInPicture' in window) { | ||
| return <DocumentPiPContent />; | ||
| } | ||
|
|
||
| if (!showPiP) { | ||
| return null; | ||
| } | ||
|
|
||
|
Contributor
Author
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. maybe I should rename it to PiPActionButton ?
Member
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. I don't have preference. both sounds OK to me. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { connect } from 'react-redux'; | ||
|
|
||
| import { createToolbarEvent } from '../../../analytics/AnalyticsEvents'; | ||
| import { sendAnalytics } from '../../../analytics/functions'; | ||
| import { IReduxState } from '../../../app/types'; | ||
| import { translate } from '../../../base/i18n/functions.web'; | ||
| import { IconPip } from '../../../base/icons/svg'; | ||
| import AbstractButton, { IProps as AbstractButtonProps } from '../../../base/toolbox/components/AbstractButton'; | ||
| import { enterPiP } from '../../actions'; | ||
|
|
||
| interface IProps extends AbstractButtonProps { | ||
| _isDocPiPActive?: boolean; | ||
| } | ||
|
|
||
| /** | ||
| * PiP toggle button | ||
| * Either opens or closes the existing picture in picture window | ||
| * Opens Document PiP or Video PiP based on browser availability and hides when both are not supported (eg: firefox). | ||
| */ | ||
| class PipTriggerButton extends AbstractButton<IProps> { | ||
| override accessibilityLabel = 'toolbar.accessibilityLabel.pip'; | ||
| override label = 'toolbar.pipOpen'; | ||
| override toggledLabel = 'toolbar.pipClose'; | ||
| override tooltip = 'toolbar.pipOpen'; | ||
| override toggledTooltip = 'toolbar.pipClose'; | ||
| override icon = IconPip; | ||
|
|
||
| override _isToggled(): boolean { | ||
| return Boolean(this.props._isDocPiPActive); | ||
| } | ||
|
|
||
| override _handleClick() { | ||
| const { dispatch } = this.props; | ||
| sendAnalytics(createToolbarEvent('picture-in-picture')); | ||
| dispatch(enterPiP()); | ||
|
Member
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. Does this handle only the enterPip use case. I see you've added some strings for close but is this implemented?
Contributor
Author
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. yeah I have added that too |
||
| } | ||
| } | ||
|
|
||
| function mapStateToProps(state: IReduxState): Partial<IProps> { | ||
| return { | ||
| _isDocPiPActive: Boolean(state['features/pip']?.isPiPActive) | ||
| }; | ||
| } | ||
|
|
||
| export default connect(mapStateToProps)(translate(PipTriggerButton)); | ||
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 common properties like disabled or should we show on prejoin or not, is kept as it is. added documentPiP for properties related to that.
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.
Sounds good to me in general