Various clips fixes - #3456
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
Code Review Summary
PR #3456 adds recording-owner attribution to meeting history rows, narrows trash recordings by organization, improves popover sizing/re-anchoring, gates upload banners on authentication, supports display selection for region capture, and adds the desktop tray version. The desktop changes are generally well-contained, and the new owner-formatting tests cover email normalization and missing owners. Risk is standard because the change crosses access-scoped data queries, UI data contracts, and native recording behavior.
Key Findings
- 🟡 MEDIUM — Meeting owner is never populated by the meeting actions: the UI type and renderer now expect
ownerEmail, butlist-meetingsandsearch-meetingsdo not resolve it from the linked recording, so the new attribution remains absent for persisted history and search results. - 🟡 MEDIUM — Search snippets suppress the recording-owner hint: a non-empty search snippet takes the fallback branch and hides the owner attribution, contrary to the intended always-visible ownership context.
🧪 Browser testing: Will run after this review (PR touches UI code)
| source?: "calendar" | "adhoc" | "manual"; | ||
| participants?: AttendeeStackParticipant[]; | ||
| ownerEmail?: string | null; |
There was a problem hiding this comment.
🟡 Populate the meeting owner field from the data sources
The route/row type now declares ownerEmail, but neither list-meetings nor search-meetings supplies it from the linked recording. As a result, persisted history and search results pass undefined to formatOwnerHint, so the new “Recorded by …” attribution and solo avatar never render. Resolve the linked recording owner in both access-scoped actions.
| const ownerHint = formatOwnerHint(meeting.ownerEmail, session?.email, t); | ||
| const subtitle = | ||
| snippet?.trim() || formatParticipantNames(participants, session?.email); | ||
| snippet?.trim() || |
There was a problem hiding this comment.
🟡 Keep the recording owner visible with search snippets
The snippet?.trim() || ... branch renders only the search snippet whenever it is non-empty, so search results omit ownerHint. Compose the snippet/participant text and owner hint independently before joining them, so shared meeting search results retain the new recording attribution.
|
Here's a visual recap of what changed: Open the full interactive recap |

Changes