feat(calls): re-enable Document PiP as an env-gated, always-on pop-out#2391
Merged
Conversation
Re-enable Document Picture-in-Picture for calls (GH-2383 item a), disabled since PR #2297. The historical audio-continuity regression appears already fixed independently: resolveCallAudioPortalTarget() unconditionally pins remote audio to the main document body whenever PiP is open. - Move CALL_DOCUMENT_PIP_ENABLED off a hardcoded const into packages/feature-flags as getEnableCallDocumentPip() (NEXT_PUBLIC_ENABLE_CALL_DOCUMENT_PIP, default true) for an instant rollback without a code change. - Auto-open Document PiP on any active call when supported, not just during screenshare; no longer force-closes when screenshare stops. - Add an explicit pop-out / un-pop toolbar button (required regardless, since browsers mandate a user gesture to open Document PiP). - Unsupported/disabled browsers keep the existing embedded floating dock with no button shown, covering the Safari/Firefox fallback. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughDocument PiP is controlled by a client feature flag that defaults on, supports active-call auto-open and explicit open/close controls, tracks dismissal per call, waits for PiP styles before rendering, and updates PiP controls, menus, capture confirmation, and regression gates. ChangesCall Document Picture-in-Picture
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant GlobalCallDockOverlay
participant useCallDockDocumentPip
participant BrowserDocumentPiP
User->>GlobalCallDockOverlay: enter active call
GlobalCallDockOverlay->>useCallDockDocumentPip: openPip()
useCallDockDocumentPip->>BrowserDocumentPiP: create PiP window and copy styles
BrowserDocumentPiP-->>useCallDockDocumentPip: stylesheet loading completes
useCallDockDocumentPip-->>GlobalCallDockOverlay: stylesReady
GlobalCallDockOverlay-->>User: render PiP dock
User->>GlobalCallDockOverlay: toggle floating window
GlobalCallDockOverlay->>useCallDockDocumentPip: closePip() or openPip()
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/epics/src/common/global-call-dock-overlay.tsx`:
- Around line 1002-1012: Update onToggleDocumentPip to handle rejected promises
from openPip by attaching a catch handler, matching the existing auto-open
effect’s rejection handling while preserving the current toggle behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c5b70a0d-5b7a-49cd-8776-b729850cc164
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (7)
apps/web/.env.templatepackages/epics/package.jsonpackages/epics/src/common/global-call-dock-overlay.tsxpackages/epics/src/common/human-chat-panel/__tests__/call-regression-manual-gates.test.tspackages/epics/src/common/human-chat-panel/call-document-pip-window-geometry.tspackages/feature-flags/src/client.tspackages/feature-flags/src/index.ts
💤 Files with no reviewable changes (1)
- packages/epics/src/common/human-chat-panel/call-document-pip-window-geometry.ts
|
🔗 Custom preview URL: https://pr-2391.preview-app.hypha.earth |
CodeRabbit review on #2391: void openPip() doesn't suppress a rejected requestWindow() call, which can surface as an unhandled promise rejection. Add the same .catch() already used by the auto-open effect. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
5 tasks
…om un-popping - The PiP-mode toolbar footer set overflow-hidden at a fixed h-8 height, clipping the audio-settings, capture, and reaction popovers (all position:absolute; bottom-full, same as the embedded dock, which uses overflow-visible for the same container and works fine). - "Open Space" unconditionally closed Document PiP before navigating. That's two independent actions bundled together — PiP staying open while browsing to the call's own space is exactly the use case PiP is for. Drop the forced closePip(), keep the focus + navigation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Menus anchored to their trigger's right edge with a fixed min-width, which in narrow Document PiP windows pushed content past the left edge where the PiP body's overflow:hidden silently clipped it (worst on the emoji picker). Center on the trigger and cap width to the viewport for pip density so the panel always fits inside the window. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nd stop-recording confirm Root cause of most of these: dockControlsDensity was hardcoded to 'default', so every density === 'pip' branch downstream (popover sizing/clamping, compact buttons, inline stop-recording confirm) never actually ran in the real Document PiP window. Now correctly flips to 'pip' when inDocumentPip. - Capture-consent recording banner now floats above the PiP toolbar instead of pushing it (and its click targets) out of the fixed-height, overflow-hidden PiP window. - react/audio/capture dropdown menus clamp to the PiP window's actual width/height instead of overflowing it; same treatment extended to the embedded (non-PiP) floating dock via a --hypha-call-dock-popover-max-h CSS var driven by the dock's own measured height. - Stop-recording confirmation renders inline in the capture dropdown in PiP instead of a page-level AlertDialog, which portals to the main window's document and is invisible while viewing another tab. - Emoji picker now passes dynamicWidth to emoji-mart's Picker — it defaults to a fixed internal pixel width and ignores container CSS entirely otherwise. - PiP toolbar buttons/icons now match the embedded dock's sizing and grouping (same grid layout, same 32px buttons) instead of a smaller, inconsistent PiP-only tier; removed the [data-call-pip-toolbar] global CSS override this exposed as redundant/harmful (forced every button, including ones inside dropdown menus, to a fixed square). - PiP window shows a lightweight inline-styled loading spinner while its cloned stylesheets are still loading, instead of a flash of unstyled content. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/epics/src/common/global-call-dock-overlay.tsx (1)
622-623: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset PiP dismissal when the active call changes.
This resets only when
showFloatingDockbecomes false. IfactiveRoomIdchanges while the dock remains visible, dismissing PiP in one call suppresses auto-open for the next call.Proposed fix
+ const dismissedCallIdRef = React.useRef<string | null>(null); + React.useEffect(() => { - if (!showFloatingDock) { + if (!showFloatingDock || dismissedCallIdRef.current !== activeRoomId) { pipDismissedRef.current = false; + dismissedCallIdRef.current = activeRoomId; } - }, [showFloatingDock]); + }, [activeRoomId, showFloatingDock]);Also applies to: 881-886
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/epics/src/common/global-call-dock-overlay.tsx` around lines 622 - 623, Reset pipDismissedRef when activeRoomId changes, not only when showFloatingDock becomes false, so PiP dismissal state does not carry between calls. Update the effect or call-transition logic associated with pipDismissedRef and preserve its existing reset behavior when the floating dock closes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/epics/src/common/global-call-dock-overlay.tsx`:
- Around line 96-124: Add an accessible, translated loading label to
CallDocumentPipLoadingView by retrieving the appropriate translation and
exposing it via aria-label or visually hidden text within the role="status"
element. Use the existing translation mechanism and translation key conventions
for all user-facing text.
In
`@packages/epics/src/common/human-chat-panel/human-chat-panel-in-call-controls.tsx`:
- Around line 558-561: Update the PiP menu positioning in the conditional class
blocks around the audio and capture controls so the menu is positioned relative
to the PiP dock/viewport and clamped within its horizontal bounds, rather than
centered on the trigger wrapper via left-1/2 and -translate-x-1/2. Apply the
same correction to both affected menus while preserving the non-PiP positioning.
- Around line 682-710: Update the PiP stop-confirmation block in the human chat
panel controls to use alert-dialog semantics with an accessible label and
description, and move keyboard focus to the cancel or confirm action when the
confirmation appears. Preserve the existing recording/transcript text and
handlers while ensuring focus remains within the confirmation until the user
completes or cancels it.
In `@packages/epics/src/common/use-call-dock-document-pip.ts`:
- Around line 221-224: Update the PiP window initialization flow around
copyStylesIntoWindow so its completion callback only sets stylesReady(true) when
it still belongs to the current PiP window or request generation. Invalidate the
previous request when opening a new window or closing PiP, while preserving the
existing style-loading behavior for the active window.
---
Outside diff comments:
In `@packages/epics/src/common/global-call-dock-overlay.tsx`:
- Around line 622-623: Reset pipDismissedRef when activeRoomId changes, not only
when showFloatingDock becomes false, so PiP dismissal state does not carry
between calls. Update the effect or call-transition logic associated with
pipDismissedRef and preserve its existing reset behavior when the floating dock
closes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ae96097-813b-4480-9264-0272cf725550
📒 Files selected for processing (7)
packages/epics/src/common/global-call-dock-overlay.tsxpackages/epics/src/common/human-chat-panel/__tests__/call-regression-manual-gates.test.tspackages/epics/src/common/human-chat-panel/human-chat-panel-call-react-popover.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-emoji-mart-surface.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-in-call-controls.tsxpackages/epics/src/common/use-call-dock-document-pip.tspackages/ui-utils/src/global.css
💤 Files with no reviewable changes (1)
- packages/ui-utils/src/global.css
- react/audio/capture menus now use fixed positioning anchored to the PiP window's own viewport instead of absolute positioning centered on their trigger — the triggers sit off-center in the toolbar, so trigger-relative centering could still clip a menu against the opposite window edge on narrower windows. - Guard against a stale-window race: if PiP is closed and reopened within the stylesheet-load window, the first window's load can no longer mark the second window ready. - Loading spinner in the PiP window now exposes an accessible/ translated label instead of an empty role="status". - Inline stop-recording confirmation in PiP gets alertdialog semantics (aria-labelledby/describedby) and moves focus to Cancel when it appears. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CALL_DOCUMENT_PIP_ENABLEDmoves from a hardcoded const intopackages/feature-flags(getEnableCallDocumentPip(),NEXT_PUBLIC_ENABLE_CALL_DOCUMENT_PIP, defaulttrue) for an instant rollback without a code change.Static code trace confirms
resolveCallAudioPortalTarget()unconditionally portals remote audio to the main document body whenever PiP is open, which looks like the historical WCUX-PIP audio-continuity fix already landed independently of the disable commit — not yet confirmed live, pending preview testing.Test plan
tsc --noEmitcleaneslintclean (no new warnings)@hypha-platform/epicstest suite: 256/256 pass (1 pre-existing unrelated failure needing a DB env var, confirmed failing onmaintoo)🤖 Generated with Claude Code
Summary by CodeRabbit