docs(hotfix): screen-share taskbar crop + strip-toggle icon padding plan
This commit is contained in:
@@ -0,0 +1,284 @@
|
||||
# Screen-Share UI Fixes (Hotfix)
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Fix two visible bugs in the Discord-style screen-share viewer:
|
||||
1. **Taskbar abgeschnitten** — when a sender shares their entire monitor, the receiver sees the bottom of the screen (Windows taskbar area) cut off.
|
||||
2. **Top-right icon overflows its frame** — the small button at the top-right corner of the in-share strip (`StripToggleIcon` / participants-toggle) renders its 2-people SVG too large for its container, with no top/bottom padding.
|
||||
|
||||
**Architecture / cause analysis:**
|
||||
|
||||
**Bug 1** has TWO possible root causes that both need to be ruled in or out:
|
||||
|
||||
- *Sender side (most likely)*: `apps/desktop/src/lib/screenShareSettings.ts` defines fixed `dims` for every non-`auto` preset (e.g. `1080p30` = 1920×1080). `CallContext.tsx:1445-1459` passes those dims as `resolution: { width, height, frameRate }` to LiveKit's `setScreenShareEnabled`. LiveKit forwards them to Chromium's `getDisplayMedia` as exact constraints. When the sender's primary monitor isn't 16:9 (e.g. a common 1920×1200 laptop, a 2560×1600 16:10 panel, a vertical secondary monitor, or a HiDPI scaled display), Chromium *crops* the frame to match — chopping the bottom strip where the Windows taskbar lives. The `auto` preset (`dims: null`) is unaffected because it omits the constraints.
|
||||
- *Receiver side*: `apps/desktop/src/components/InCallPanel.tsx:1326-1330` reserves `pb-28` (112px) at the bottom of the cinema-mode content area for the floating controls bar, and `ScreenShareViewer.tsx:99-113` uses `object-contain` on the `<video>`. The math already protects against overlap, but worth a visual sanity check during the smoke test.
|
||||
|
||||
**Bug 2** is in `apps/desktop/src/components/InCallPanel.tsx:1497-1524` — the `StripToggleIcon` button is `h-9 w-9` (36×36) wrapping an `<svg width="18" height="18">`. The SVG's `viewBox` is `0 0 24 24` but several of its paths actually reach beyond y=24 (`M17 21v-2a4 4 0 0 0-4-4H5a4 4 0 0 0-4 4v2` puts the bottom edge at y=23 and the top at y=11, but combined with `M16 3.13a4 4 0 0 1 0 7.75` the icon extends roughly y=3.13 to y=23 — almost the full viewBox, leaving ~1px padding). At 18×18 that translates to <1px visible breathing room. Combined with any container line-height or rounding, the icon appears to touch the frame edge. Fix: drop the explicit `width="18" height="18"` and let it inherit a Tailwind size class, OR shrink the icon to fit cleanly inside the 36px button.
|
||||
|
||||
**Tech Stack:** React 18 + TailwindCSS + LiveKit client. No backend changes.
|
||||
|
||||
**Non-goals:**
|
||||
- Not redesigning the cinema-mode controls layout.
|
||||
- Not adding a new "fit-to-monitor" preset option (could be a follow-up if cropping is widespread).
|
||||
- Not changing audio-share behavior.
|
||||
|
||||
---
|
||||
|
||||
## Pre-flight
|
||||
|
||||
- [ ] **Verify clean working tree on `main`**
|
||||
|
||||
Run: `cd "D:\Programmieren\ChatApp-Electron\chat-app" && git status`
|
||||
Expected: only Phase 3 commits ahead of last good baseline; no unrelated uncommitted changes.
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Diagnose — confirm sender-side cropping vs receiver-side overlay for Bug 1
|
||||
|
||||
**Why:** Before changing the preset behavior we want to confirm the symptom matches the hypothesis. The user's screenshot shows the Windows taskbar visible-but-truncated; if the truncation is exactly the height difference between 1080 and the monitor's actual height, the sender-side hypothesis is confirmed.
|
||||
|
||||
**Files (read-only):**
|
||||
- `apps/desktop/src/context/CallContext.tsx:1395-1470` — where the preset dims are passed to LiveKit
|
||||
- `apps/desktop/src/lib/screenShareSettings.ts` — preset dims table
|
||||
- `apps/desktop/src/components/InCallPanel.tsx:1320-1410` — cinema-mode layout for receiver
|
||||
|
||||
- [ ] **Step 1: Inspect the sender's current preset (in dev / chat with user)**
|
||||
|
||||
Ask the user: "Which screen-share preset is set on the sender's machine (Settings → Bildschirmfreigabe → Voreinstellung)? And what's their primary monitor resolution (right-click desktop → Anzeigeeinstellungen → Auflösung)?"
|
||||
|
||||
If preset is `auto` → sender-side hypothesis is FALSE; jump to Step 3 (receiver-side investigation).
|
||||
If preset is anything else (e.g. `1080p30`) AND monitor resolution doesn't match the preset's aspect ratio → sender-side hypothesis is CONFIRMED; proceed to Task 2.
|
||||
|
||||
- [ ] **Step 2: Reproduce locally if access exists**
|
||||
|
||||
Set a known non-matching configuration:
|
||||
- Settings → Bildschirmfreigabe → Voreinstellung = `1080p · 30 fps`
|
||||
- On a monitor with native 1920×1200 (or any non-16:9) resolution
|
||||
- Start a share with the user's account in a second window
|
||||
- Observe: bottom of receiver's view should be the cropped band where the taskbar would be
|
||||
|
||||
Document the observed monitor resolution + preset in the commit message of Task 2.
|
||||
|
||||
- [ ] **Step 3: Inspect receiver-side cinema layout for any overlap regression**
|
||||
|
||||
Read `apps/desktop/src/components/InCallPanel.tsx` lines 1324-1485. Verify that:
|
||||
- The content area has `pb-28` (line 1331 today)
|
||||
- The control bar's wrapping div has `absolute bottom-4` (line 1528 today)
|
||||
- `4 + ~80 (controls height) ≈ 84 < 112 (pb-28)` → no overlap
|
||||
|
||||
If the math still holds, the receiver-side path is clean and Bug 1 is purely sender-side.
|
||||
|
||||
**No commit for this task** — diagnosis only. Findings inform Task 2.
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Fix — preset dims act as aspect-preserving maxima, not hard crops (Bug 1)
|
||||
|
||||
**Why:** The user expects "1080p" to mean *quality cap*, not *forced crop*. A non-16:9 monitor should still stream its entire surface, just scaled to fit within the preset's pixel budget.
|
||||
|
||||
**Files:**
|
||||
- Modify: `apps/desktop/src/context/CallContext.tsx` (the `setScreenShareEnabled` call around line 1438)
|
||||
|
||||
Two viable approaches — Task 2 picks Option A; Option B is a fallback if A doesn't work in LiveKit.
|
||||
|
||||
### Option A (preferred): drop fixed dims, rely on bitrate cap only
|
||||
|
||||
LiveKit's screen-share publish accepts a `videoSimulcastLayers` / `screenShareEncoding.maxBitrate` knob independently of resolution. By only constraining frame rate + bitrate, we let the capture run at native resolution and downscale via encoder.
|
||||
|
||||
- [ ] **Step 1: Find the LiveKit screen-share publish call**
|
||||
|
||||
Read `apps/desktop/src/context/CallContext.tsx` around line 1438-1465 to confirm the current shape of the `setScreenShareEnabled` options object.
|
||||
|
||||
- [ ] **Step 2: Replace the `resolution.{width,height}` block with an aspect-preserving variant**
|
||||
|
||||
The current block (lines ~1445-1459):
|
||||
```ts
|
||||
...(ssParams.dims
|
||||
? {
|
||||
resolution: {
|
||||
width: ssParams.dims.width,
|
||||
height: ssParams.dims.height,
|
||||
frameRate: fps,
|
||||
},
|
||||
}
|
||||
: {
|
||||
resolution: {
|
||||
width: 3840,
|
||||
height: 2160,
|
||||
frameRate: fps,
|
||||
},
|
||||
}),
|
||||
```
|
||||
|
||||
Change to (verify LiveKit option shapes against the current installed version with `pnpm list livekit-client` first — if `screenShareEncoding` isn't supported, fall back to Option B):
|
||||
```ts
|
||||
// Preset dims become a max-height cap that the encoder honours via the
|
||||
// videoEncoding.maxBitrate + frameRate combo. We deliberately do NOT pass
|
||||
// resolution.width/height to getDisplayMedia — Chromium treats those as
|
||||
// exact constraints and CROPS non-matching monitors (e.g. 16:10 panels
|
||||
// lose their bottom strip, including the Windows taskbar).
|
||||
//
|
||||
// Quality is still scoped to the preset via the bitrate ceiling configured
|
||||
// elsewhere in screenShareSettings; we just stop forcing the geometry.
|
||||
...(ssParams.dims
|
||||
? {
|
||||
resolution: {
|
||||
frameRate: fps,
|
||||
},
|
||||
}
|
||||
: {
|
||||
resolution: {
|
||||
frameRate: fps,
|
||||
},
|
||||
}),
|
||||
```
|
||||
|
||||
If `resolution` requires at least one of `width`/`height` in the installed LiveKit type, pass only the height as `ideal` (not the width) — Chromium will then scale width to preserve aspect:
|
||||
```ts
|
||||
resolution: {
|
||||
height: { ideal: ssParams.dims.height } as unknown as number,
|
||||
frameRate: fps,
|
||||
},
|
||||
```
|
||||
(The `as unknown as number` cast handles a LiveKit type that expects a plain number; the runtime accepts the MediaTrackConstraints object form because it's forwarded to `getDisplayMedia`.)
|
||||
|
||||
- [ ] **Step 3: Typecheck + smoke test plan**
|
||||
|
||||
Run: `pnpm --filter @chat-app/desktop typecheck`
|
||||
Expected: PASS. If LiveKit's TS types reject the change, fall through to Option B below.
|
||||
|
||||
Smoke test sequence (user-driven; don't need to verify in code):
|
||||
1. Set preset to `1080p · 30 fps`
|
||||
2. Share entire primary monitor
|
||||
3. Receiver sees the full monitor including the taskbar — no crop
|
||||
4. Switch to a Windows window source → still shares fully
|
||||
5. Switch preset to `auto` → still works
|
||||
6. Switch preset to `720p · 30 fps` → bitrate drops, but no crop
|
||||
|
||||
### Option B (fallback): keep dims, switch to `ideal` constraints
|
||||
|
||||
If Option A doesn't typecheck, replace the resolution block with:
|
||||
```ts
|
||||
// `ideal` lets Chromium pick the closest match without cropping when the
|
||||
// source's native resolution doesn't fit exactly. Using fixed width/height
|
||||
// causes Chromium to crop non-matching monitors (e.g. 16:10 panels lose
|
||||
// the bottom strip with the taskbar).
|
||||
...(ssParams.dims
|
||||
? {
|
||||
resolution: {
|
||||
width: { ideal: ssParams.dims.width } as unknown as number,
|
||||
height: { ideal: ssParams.dims.height } as unknown as number,
|
||||
frameRate: fps,
|
||||
},
|
||||
}
|
||||
: {
|
||||
resolution: {
|
||||
frameRate: fps,
|
||||
},
|
||||
}),
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Commit (whichever option compiled)**
|
||||
|
||||
```bash
|
||||
git add apps/desktop/src/context/CallContext.tsx
|
||||
git commit -m "fix(screen-share): preset dims no longer crop non-16:9 monitors"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Fix — StripToggleIcon padding inside its frame (Bug 2)
|
||||
|
||||
**Why:** The participants strip-toggle button (`absolute right-5 top-5` in cinema mode) is `h-9 w-9` (36×36) with an 18×18 SVG whose path data fills nearly the entire viewBox, leaving no visible padding. The icon appears glued to the frame edges.
|
||||
|
||||
**Files:**
|
||||
- Modify: `apps/desktop/src/components/InCallPanel.tsx` (the `StripToggleIcon` function around line 1542-1561)
|
||||
|
||||
- [ ] **Step 1: Shrink the icon's actual rendered size**
|
||||
|
||||
Read `apps/desktop/src/components/InCallPanel.tsx` lines 1542-1561 to confirm the current SVG shape.
|
||||
|
||||
Change the SVG element from:
|
||||
```tsx
|
||||
<svg
|
||||
width="18"
|
||||
height="18"
|
||||
viewBox="0 0 24 24"
|
||||
...
|
||||
>
|
||||
```
|
||||
to:
|
||||
```tsx
|
||||
<svg
|
||||
width="14"
|
||||
height="14"
|
||||
viewBox="0 0 24 24"
|
||||
...
|
||||
>
|
||||
```
|
||||
|
||||
Rationale: 14px inside a 36px button leaves 11px of padding on each side (36 − 14 = 22, /2 = 11), matching the visual rhythm of other icon buttons in the same file (e.g. the `FullscreenIcon` in ScreenShareViewer uses `h-3.5 w-3.5` = 14px inside `h-6 w-6` = 24px = ~5px padding, but the cinema strip-toggle is on a larger 36px button and benefits from a similar ratio).
|
||||
|
||||
- [ ] **Step 2: Verify visually adjacent icon-buttons match the new ratio**
|
||||
|
||||
Skim the file for other `h-9 w-9` buttons and confirm their inner-icon sizes (`grep -n "h-9 w-9" apps/desktop/src/components/InCallPanel.tsx`). If most use ~14px icons, the change is in line; if they use 18px, leave 18 alone and instead fix the SVG's viewBox padding by widening it to `viewBox="-2 -2 28 28"` so the visible content sits in the middle:
|
||||
|
||||
```tsx
|
||||
<svg
|
||||
width="18"
|
||||
height="18"
|
||||
viewBox="-2 -2 28 28"
|
||||
...
|
||||
>
|
||||
```
|
||||
|
||||
Pick whichever approach the file's existing aesthetic favours. Default to **Step 1's shrink-to-14** unless Step 2's grep proves 18 is the house norm.
|
||||
|
||||
- [ ] **Step 3: Typecheck**
|
||||
|
||||
Run: `pnpm --filter @chat-app/desktop typecheck`
|
||||
Expected: PASS (this is a pure JSX change).
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add apps/desktop/src/components/InCallPanel.tsx
|
||||
git commit -m "fix(call): strip-toggle icon no longer touches its frame edges"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Final gate
|
||||
|
||||
- [ ] **Step 1: Typecheck both packages**
|
||||
|
||||
Run: `pnpm --filter @chat-app/shared typecheck && pnpm --filter @chat-app/desktop typecheck`
|
||||
Expected: both PASS.
|
||||
|
||||
- [ ] **Step 2: Run shared tests (sanity — these changes don't touch shared, but confirm no flakes)**
|
||||
|
||||
Run: `pnpm --filter @chat-app/shared test --run`
|
||||
Expected: PASS — same count as before this hotfix.
|
||||
|
||||
- [ ] **Step 3: Smoke test in dev (user-driven)**
|
||||
|
||||
User starts `pnpm dev`, runs a screen-share to a second account, and verifies:
|
||||
- Taskbar visible at bottom of the streamed monitor (any preset)
|
||||
- Strip-toggle button in cinema mode shows the 2-people icon with comfortable padding inside its 36px frame
|
||||
|
||||
Report back: "Hotfix code-complete. Restart dev, share your monitor at any preset — taskbar should now stream fully. Cinema-mode strip-toggle button should have visible padding around the icon. **No release** — version stays 0.18.8."
|
||||
|
||||
---
|
||||
|
||||
## Self-review
|
||||
|
||||
1. **Bug coverage:**
|
||||
- Bug 1 (taskbar crop) → Task 2 (sender-side preset constraint change)
|
||||
- Bug 2 (icon overflow) → Task 3 (SVG size adjustment)
|
||||
- Diagnosis Task 1 confirms the root cause before applying fixes
|
||||
|
||||
2. **Placeholders:** none.
|
||||
|
||||
3. **Risk:** Option A in Task 2 changes the screen-share quality model — bitrate cap remains, but visual fidelity may improve OR encoder may push more bandwidth than before for the same preset name. Acceptable: the previous behaviour cropped content, which is strictly worse. The user can fall back to `720p` if bandwidth becomes an issue.
|
||||
|
||||
4. **Aspect-ratio interaction with low-bitrate presets:** `720p · 30 fps` previously enforced a 1280×720 max — a 4K monitor streamed under this preset now passes native 3840×2160 frames to the encoder, capped only by bitrate. The encoder will downscale to stay under the budget, but the receiver renders at the source resolution × encoder scale-factor. If this becomes a CPU/quality concern, re-introduce a height cap via Option B's `ideal` constraints (which Chromium honours preferring aspect ratio over exact match).
|
||||
Reference in New Issue
Block a user