feat(audio): persistent audio player across room navigation#41120
feat(audio): persistent audio player across room navigation#41120ggazzo wants to merge 21 commits into
Conversation
Audio message playback no longer stops when you switch or close the conversation. A single shared audio element is hoisted above the room layout into a new MediaPlayerProvider, so it survives message-list unmounting and keeps playing while the UI re-attaches to it. A "Now playing" card pinned to the top of the sidebar reflects the shared player: play/pause, seek, playback speed (1x/1.5x/2x), and a shortcut back to the originating conversation. In-message audio controls drive the same shared element.
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: bc0c16a The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
WalkthroughThis PR adds a persistent shared audio player, threads audio attachment source metadata through message rendering, and surfaces the active track in a new sidebar “Now Playing” card with playback controls and navigation back to the source conversation. ChangesPersistent Audio Player Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AudioAttachment
participant MediaPlayerProvider
participant NowPlayingSection
participant AudioElement
User->>AudioAttachment: click play on message attachment
AudioAttachment->>MediaPlayerProvider: play(track)
MediaPlayerProvider->>AudioElement: set src, load, play
MediaPlayerProvider-->>NowPlayingSection: context updates (track, playing)
User->>NowPlayingSection: navigate to another room
NowPlayingSection->>MediaPlayerProvider: playback persists
User->>NowPlayingSection: click header
NowPlayingSection->>User: navigate to originating room
User->>NowPlayingSection: click close
NowPlayingSection->>MediaPlayerProvider: close()
MediaPlayerProvider->>AudioElement: pause, clear src, reset state
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #41120 +/- ##
===========================================
+ Coverage 69.14% 69.94% +0.80%
===========================================
Files 3433 3374 -59
Lines 132323 130860 -1463
Branches 23091 22705 -386
===========================================
+ Hits 91489 91528 +39
+ Misses 37472 36015 -1457
+ Partials 3362 3317 -45
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
The in-message audio player is restored to the native fuselage AudioPlayer (unchanged UI/behavior). Persistence now works as a hand-off: a usePersistentAudio hook observes the message's audio element and, when the message unmounts while playing, hands the track to the shared detached element in MediaPlayerProvider so playback continues in the sidebar card. Re-entering the room hands the track back and resumes in the message element.
Replace the two-element hand-off with one shared <audio> element owned by MediaPlayerProvider and never recreated. Both the in-message controls and the sidebar card drive that same element, so switching or closing the room only swaps which UI renders — playback continues with no reload, seek, or gap. Removes usePersistentAudio (hand-off no longer needed).
… closing a thread)
|
/jira ARCH-2167 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/meteor/client/sidebar/sections/NowPlayingSection.tsx (1)
13-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove implementation comment per coding guidelines.
As per coding guidelines,
**/*.{ts,tsx,js}files should "Avoid code comments in the implementation."🤖 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 `@apps/meteor/client/sidebar/sections/NowPlayingSection.tsx` around lines 13 - 18, Remove the implementation comment from NowPlayingSection so the TSX file follows the no-comments guideline; delete the block above the component in NowPlayingSection and keep the code unchanged.Source: Coding guidelines
apps/meteor/client/sidebar/sections/SidebarCard.tsx (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove implementation comment per coding guidelines.
As per coding guidelines,
**/*.{ts,tsx,js}files should "Avoid code comments in the implementation." This JSDoc block should be removed or the description embedded in the PR/documentation instead.🤖 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 `@apps/meteor/client/sidebar/sections/SidebarCard.tsx` around lines 6 - 9, Remove the implementation JSDoc comment from SidebarCard since the coding guidelines discourage code comments in tsx implementation files. Update the SidebarCard component by deleting the descriptive block above the reusable elevated surface logic, and keep any needed explanation in external documentation or the PR description instead.Source: Coding guidelines
apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this upstream-tracking TODO out of the component file.
The repo guideline for TS/TSX files asks us to avoid implementation comments. Please track the planned Fuselage swap in the PR/issues instead of keeping the TODO block in shipped code. As per coding guidelines, "
**/*.{ts,tsx,js}: ... Avoid code comments in the implementation`."🤖 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 `@apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx` around lines 6 - 9, The upstream-tracking TODO in AudioPlayerControls is an implementation comment that should not live in shipped TSX code. Remove the TODO block from AudioPlayerControls and track the Fuselage swap in the PR or issue instead, keeping the component file focused on implementation only.Source: Coding guidelines
apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx (1)
13-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the docblock from this implementation file.
The repo guideline explicitly avoids code comments in implementation, and
AudioAttachmentSourceis already self-descriptive. As per coding guidelines, "Avoid code comments in the implementation."Suggested cleanup
-/** Extra context about the message that owns this audio, used by the shared player. */ export type AudioAttachmentSource = { rid?: string; mid?: string; username?: string; name?: string; };🤖 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 `@apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx` around lines 13 - 19, Remove the docblock above AudioAttachmentSource in AudioAttachment.tsx, since this is an implementation file and the type is already self-descriptive. Keep the AudioAttachmentSource export unchanged and only delete the comment so the component file follows the repo rule against code comments in implementation.Source: Coding guidelines
🤖 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 `@apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx`:
- Around line 64-117: The recovery flow in handleMediaURLRecovery should verify
the active track still matches after awaiting getRedirectURLInfo() and avoid
leaving isRecoveringRef.current stuck true on early returns. Move the in-flight
guard so it only stays set for the actual recovery work, re-check
trackRef.current/current identity after the await before assigning node.src, and
always clear isRecoveringRef.current in a finally path when the attempt is
abandoned or fails.
In `@apps/meteor/client/sidebar/sections/NowPlayingSection.tsx`:
- Around line 43-51: The clickable Box in NowPlayingSection only supports mouse
interaction, so keyboard users can’t trigger openConversation. Update the Box
that currently uses onClick/openConversation to behave like an accessible button
by adding an appropriate role, tabIndex, and key handling (for Enter/Space), and
keep the pointer cursor conditional on track.rid. Use the NowPlayingSection and
openConversation identifiers to locate the interactive card area.
---
Nitpick comments:
In `@apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx`:
- Around line 6-9: The upstream-tracking TODO in AudioPlayerControls is an
implementation comment that should not live in shipped TSX code. Remove the TODO
block from AudioPlayerControls and track the Fuselage swap in the PR or issue
instead, keeping the component file focused on implementation only.
In
`@apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx`:
- Around line 13-19: Remove the docblock above AudioAttachmentSource in
AudioAttachment.tsx, since this is an implementation file and the type is
already self-descriptive. Keep the AudioAttachmentSource export unchanged and
only delete the comment so the component file follows the repo rule against code
comments in implementation.
In `@apps/meteor/client/sidebar/sections/NowPlayingSection.tsx`:
- Around line 13-18: Remove the implementation comment from NowPlayingSection so
the TSX file follows the no-comments guideline; delete the block above the
component in NowPlayingSection and keep the code unchanged.
In `@apps/meteor/client/sidebar/sections/SidebarCard.tsx`:
- Around line 6-9: Remove the implementation JSDoc comment from SidebarCard
since the coding guidelines discourage code comments in tsx implementation
files. Update the SidebarCard component by deleting the descriptive block above
the reusable elevated surface logic, and keep any needed explanation in external
documentation or the PR description instead.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41407861-799c-4a26-8489-3af75a90f723
📒 Files selected for processing (18)
.changeset/persistent-audio-player.mdapps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsxapps/meteor/client/components/AudioPlayer/formatPlaybackTime.tsapps/meteor/client/components/message/content/Attachments.tsxapps/meteor/client/components/message/content/attachments/AttachmentsItem.tsxapps/meteor/client/components/message/content/attachments/FileAttachment.tsxapps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.tsapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsxapps/meteor/client/providers/MediaPlayerProvider/index.tsapps/meteor/client/providers/MeteorProvider.tsxapps/meteor/client/sidebar/Sidebar.tsxapps/meteor/client/sidebar/sections/NowPlayingSection.tsxapps/meteor/client/sidebar/sections/SidebarCard.tsxapps/meteor/client/views/navigation/sidebar/Sidebar.tsxpackages/i18n/src/locales/en.i18n.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
⚠️ CI failures not shown inline (5)
GitHub Actions: CI / 0_✅ Tests Done.txt: feat(audio): persistent audio player across room navigation
Conclusion: failure
##[group]Run if [[ 'success' != 'success' ]]; then
�[36;1mif [[ 'success' != 'success' ]]; then�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [[ 'success' != 'success' ]]; then�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [[ 'success' != 'success' ]]; then�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [[ 'success' != 'success' ]]; then�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [[ 'success' != 'success' ]]; then�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [[ 'failure' != 'success' ]]; then�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [[ 'success' != 'success' ]]; then�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [[ 'success' != 'success' ]]; then�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mif [[ 'success' != 'success' ]]; then�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mecho finished�[0m
shell: /usr/bin/bash -e {0}
env:
TOOL_NODE_FLAGS: --max_old_space_size=4096
##[endgroup]
##[error]Process completed with exit code 1.
GitHub Actions: CI / 6_📦 Track Image Sizes.txt: feat(audio): persistent audio player across room navigation
Conclusion: failure
##[group]Run current_total=$(jq -r '.total' current-sizes.json)
�[36;1mcurrent_total=$(jq -r '.total' current-sizes.json)�[0m
�[36;1m�[0m
�[36;1mif [[ ! -f baseline-sizes.json ]]; then�[0m
�[36;1m echo "No baseline available"�[0m
�[36;1m echo "size-diff=0" >> $GITHUB_OUTPUT�[0m
�[36;1m echo "size-diff-percent=0" >> $GITHUB_OUTPUT�[0m
�[36;1m echo "comment-triggered=false" >> $GITHUB_OUTPUT�[0m
�[36;1m�[0m
�[36;1m cat > report.md << 'EOF'�[0m
�[36;1m# 📦 Docker Image Size Report�[0m
�[36;1m�[0m
�[36;1m**Status:** First measurement - no baseline for comparison�[0m
�[36;1m�[0m
�[36;1m**Total Size:** $(numfmt --to=iec-i --suffix=B $current_total)�[0m
�[36;1mEOF�[0m
�[36;1m exit 0�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mbaseline_total=$(jq -r '.total' baseline-sizes.json)�[0m
�[36;1mdiff=$((current_total - baseline_total))�[0m
�[36;1m�[0m
�[36;1mif [[ $baseline_total -gt 0 ]]; then�[0m
�[36;1m percent=$(awk "BEGIN {printf \"%.2f\", ($diff / $baseline_total) * 100}")�[0m
�[36;1melse�[0m
�[36;1m percent=0�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mecho "size-diff=$diff" >> $GITHUB_OUTPUT�[0m
�[36;1mecho "size-diff-percent=$percent" >> $GITHUB_OUTPUT�[0m
�[36;1m�[0m
�[36;1m# Only comment when size is bigger than baseline; optionally require per-image thresholds�[0m
�[36;1mTHRESHOLDS="$SIZE_THRESHOLDS"�[0m
�[36;1mFAIL_THRESHOLDS="$FAIL_THRESHOLDS"�[0m
�[36;1mcomment_triggered=false�[0m
�[36;1mfail_triggered=false�[0m
�[36;1mif [[ $diff -gt 0 ]]; then�[0m
�[36;1m if [[ -z "$THRESHOLDS" ]] || [[ "$THRESHOLDS" == "{}" ]]; then�[0m
�[36;1m comment_triggered=true�[0m
�[36;1m fi�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mcolor="gray"�[0m
�[36;1mif (( $(awk "BEGIN {print ($percent > 0.01)}") )); then�[0m
�[36;1m color="red"�[0m
�[36;1melif (( $(awk "BEGIN {print ($percent < -0.01)}") )); then�[0m
�[36;1m color="green"�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1m# Generate report�[0m
�[36;1mif [[ $diff -gt 0 ]]; then�[0m
�[36;1m emoji=...
GitHub Actions: CI / 13_🔨 Test UI (EE) _ MongoDB 8.0 coverage (5_5).txt: feat(audio): persistent audio player across room navigation
Conclusion: failure
##[group]Run set -o xtrace
�[36;1mset -o xtrace�[0m
�[36;1m�[0m
�[36;1myarn prepare�[0m
�[36;1myarn test:e2e --shard="$E2E_SHARD/$E2E_TOTAL_SHARD"�[0m
shell: /usr/bin/bash -e {0}
env:
MONGO_URL: mongodb://localhost:27017/rocketchat?replicaSet=rs0&directConnection=true
TOOL_NODE_FLAGS: --max_old_space_size=4096
LOWERCASE_REPOSITORY: rocketchat
DOCKER_TAG: pr-41120-amd64
DOCKER_TAG_SUFFIX_ROCKETCHAT:
MONGODB_VERSION: 8.0
COVERAGE_DIR: /tmp/coverage/ui
COVERAGE_FILE_NAME: ui-5.json
COVERAGE_REPORTER: json
NODE_VERSION: 22.22.3
DENO_VERSION: 2.3.1
MONGOMS_DOWNLOAD_DIR: /home/runner/work/_temp/mongodb-memory-server
MONGOMS_PREFER_GLOBAL_PATH: false
TURBOGHA_PORT: 41230
TURBO_API: http://localhost:41230
TURBO_***REDACTED***
TURBO_TEAM: turbogha
E2E_COVERAGE: true
IS_EE: true
REPORTER_ROCKETCHAT_***REDACTED***
REPORTER_ROCKETCHAT_URL: ***
REPORTER_JIRA_ROCKETCHAT_***REDACTED***
REPORTER_ROCKETCHAT_REPORT: true
REPORTER_ROCKETCHAT_RUN: 62995
REPORTER_ROCKETCHAT_BRANCH: refs/pull/41120/merge
REPORTER_ROCKETCHAT_DRAFT: true
REPORTER_ROCKETCHAT_HEAD_SHA: 6f1328704549e45109ba8b45b1f60a5acc9cdbb0
REPORTER_ROCKETCHAT_AUTHOR: ggazzo
REPORTER_ROCKETCHAT_RUN_URL: https://github.com/RocketChat/Rocket.Chat/actions/runs/28522795519
REPORTER_ROCKETCHAT_PR: 41120
QASE_API_***REDACTED***
QASE_REPORT:
CI: true
PLAYWRIGHT_RETRIES: 0
E2E_SHARD: 5
E2E_TOTAL_SHARD: 5
##[endgroup]
+ yarn prepare
+ yarn test:e2e --shard=5/5
GitCommitInfo: timeout of 3000ms exceeded while running "git fetch origin b6e0d3b30d86a01764df2dd652f989c20a51a11f"
Running 141 tests using 1 worker, shard 5 of 5
✓ 1 tests/e2e/rooms-join.spec.ts:46:7 › Join rooms › public channels without preview-c-room › should let a non-member join a public channel (7.7s)
[INFO] qase: Test should let a non-member join a public channel passed
✓ 2 tests/e2e/rooms-join.spec.ts:82:7 › Join rooms › public channel wi...
GitHub Actions: CI / 31_🔨 Test Unit _ Unit Tests.txt: feat(audio): persistent audio player across room navigation
Conclusion: failure
_old_space_size=4096
ENTERPRISE_LICENSE: MK+bpK5NveUuNlWGaQXGoy+8b74Luet82M3ZGcBB8b5P9Y+m67NEtpW64dc1d5lEWi6d0nFjCjtCMneVD7bKxodz/Cml8URKEo5P7cQb/9wmeT0MzAhYNaRFZlIGkZ3ITF59pDV2u4HZuosEDJikVRwnaJ5ZoU/pOsHSPUPhTyGNIqLeKynODtUpfwDdIKEmHxpf2yVkKjgRiIJmbWjM6A4k+MNNYXWVXHzye7GggqWVg/ZcT7nKU1CCadpLhTJiIrgrrPzil1G5DQ4xnLs3Q2tu2dILSDiW5OYw/ywu2yCMicTjMq4MLL5SXDQJj6WoJzZ54HosbvsDzOXvsdC9gI1CjhPL2uRuvC8XLrzn3vL2UgXnifzD1VrLTtdZ+aSADveqtlzYlRWtqoUFBbNw8o+YVHdhbZGR0beMoAyRbHi5EMpxpad3L+NyztUIT/Uh/IjQ/C2SQZ6jB0GKPBOPxFLN56FNhTGrffLFR++TVoBu0Iquc7kajWkNit3bVbZvbx+oFcVW2PcjQ/+i2jpJjbgtUFUKrTKxGMAXTWoDzIQQ35zNzGAy268IM4Ymp5JmsVEnBOEUkbF9yx6fzkO6xZhpsHf0muklnW0kA+Tlore/TUrBWh1/RwWlQeZlxM5NyWoRM5onQmr/k/4BmObtL1Hpmbk8oMG29z89xtE9y/4=
NODE_VERSION: 22.22.3
DENO_VERSION: 2.3.1
MONGOMS_DOWNLOAD_DIR: /home/runner/work/_temp/mongodb-memory-server
MONGOMS_PREFER_GLOBAL_PATH: false
TURBOGHA_PORT: 41230
TURBO_API: http://localhost:41230
TURBO_***REDACTED***
TURBO_TEAM: turbogha
##[endgroup]
##[group]Run missing_deps=""
�[36;1mmissing_deps=""�[0m
�[36;1m�[0m
�[36;1m# Check for always-required commands�[0m
�[36;1mfor cmd in bash git curl; do�[0m
�[36;1m if ! command -v "$cmd" >/dev/null 2>&1; then�[0m
�[36;1m missing_deps="$missing_deps $cmd"�[0m
�[36;1m fi�[0m
�[36;1mdone�[0m
�[36;1m�[0m
�[36;1m# Check for gpg only if validation is not being skipped�[0m
�[36;1mif [ "$INPUT_SKIP_VALIDATION" != "true" ]; then�[0m
�[36;1m if ! command -v gpg >/dev/null 2>&1; then�[0m
�[36;1m missing_deps="$missing_deps gpg"�[0m
�[36;1m fi�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1m# Report missing required dependencies�[0m
�[36;1mif [ -n "$missing_deps" ]; then�[0m
�[36;1m echo "Error: The following required dependencies are missing:$missing_deps"�[0m
�[36;1m echo "Please install these dependencies before using this action."�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mecho "All required system dependencies are available."�[0m
shell: /usr...
GitHub Actions: CI / 16_🔨 Test UI (EE) _ MongoDB 8.0 coverage (4_5).txt: feat(audio): persistent audio player across room navigation
Conclusion: failure
##[group]Run set -o xtrace
�[36;1mset -o xtrace�[0m
�[36;1m�[0m
�[36;1myarn prepare�[0m
�[36;1myarn test:e2e --shard="$E2E_SHARD/$E2E_TOTAL_SHARD"�[0m
shell: /usr/bin/bash -e {0}
env:
MONGO_URL: mongodb://localhost:27017/rocketchat?replicaSet=rs0&directConnection=true
TOOL_NODE_FLAGS: --max_old_space_size=4096
LOWERCASE_REPOSITORY: rocketchat
DOCKER_TAG: pr-41120-amd64
DOCKER_TAG_SUFFIX_ROCKETCHAT:
MONGODB_VERSION: 8.0
COVERAGE_DIR: /tmp/coverage/ui
COVERAGE_FILE_NAME: ui-4.json
COVERAGE_REPORTER: json
NODE_VERSION: 22.22.3
DENO_VERSION: 2.3.1
MONGOMS_DOWNLOAD_DIR: /home/runner/work/_temp/mongodb-memory-server
MONGOMS_PREFER_GLOBAL_PATH: false
TURBOGHA_PORT: 41230
TURBO_API: http://localhost:41230
TURBO_***REDACTED***
TURBO_TEAM: turbogha
E2E_COVERAGE: true
IS_EE: true
REPORTER_ROCKETCHAT_***REDACTED***
REPORTER_ROCKETCHAT_URL: ***
REPORTER_JIRA_ROCKETCHAT_***REDACTED***
REPORTER_ROCKETCHAT_REPORT: true
REPORTER_ROCKETCHAT_RUN: 62995
REPORTER_ROCKETCHAT_BRANCH: refs/pull/41120/merge
REPORTER_ROCKETCHAT_DRAFT: true
REPORTER_ROCKETCHAT_HEAD_SHA: 6f1328704549e45109ba8b45b1f60a5acc9cdbb0
REPORTER_ROCKETCHAT_AUTHOR: ggazzo
REPORTER_ROCKETCHAT_RUN_URL: https://github.com/RocketChat/Rocket.Chat/actions/runs/28522795519
REPORTER_ROCKETCHAT_PR: 41120
QASE_API_***REDACTED***
QASE_REPORT:
CI: true
PLAYWRIGHT_RETRIES: 0
E2E_SHARD: 4
E2E_TOTAL_SHARD: 5
##[endgroup]
+ yarn prepare
+ yarn test:e2e --shard=4/5
GitCommitInfo: timeout of 3000ms exceeded while running "git fetch origin b6e0d3b30d86a01764df2dd652f989c20a51a11f"
Running 149 tests using 1 worker, shard 4 of 5
✓ 1 tests/e2e/omnichannel/omnichannel-livechat-hide-expand-chat.spec.ts:46:6 › OC - Livechat - Hide "Expand chat" › OC - Livechat - Hide "Expand chat" (7.0s)
[INFO] qase: Test OC - Livechat - Hide "Expand chat" passed
✓ 2 tests/e2e/omnichannel/omnichannel-livechat-logo.spec.ts:38:6 › OC ...
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/sidebar/Sidebar.tsxapps/meteor/client/components/AudioPlayer/formatPlaybackTime.tsapps/meteor/client/views/navigation/sidebar/Sidebar.tsxapps/meteor/client/providers/MeteorProvider.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/providers/MediaPlayerProvider/index.tsapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.tsapps/meteor/client/components/message/content/attachments/AttachmentsItem.tsxapps/meteor/client/sidebar/sections/SidebarCard.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/content/Attachments.tsxapps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsxapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsxapps/meteor/client/components/message/content/attachments/FileAttachment.tsxapps/meteor/client/sidebar/sections/NowPlayingSection.tsxapps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx
🧠 Learnings (7)
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/sidebar/Sidebar.tsxapps/meteor/client/views/navigation/sidebar/Sidebar.tsxapps/meteor/client/providers/MeteorProvider.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/components/message/content/attachments/AttachmentsItem.tsxapps/meteor/client/sidebar/sections/SidebarCard.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/content/Attachments.tsxapps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsxapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsxapps/meteor/client/components/message/content/attachments/FileAttachment.tsxapps/meteor/client/sidebar/sections/NowPlayingSection.tsxapps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/sidebar/Sidebar.tsxapps/meteor/client/components/AudioPlayer/formatPlaybackTime.tsapps/meteor/client/views/navigation/sidebar/Sidebar.tsxapps/meteor/client/providers/MeteorProvider.tsxapps/meteor/client/components/message/variants/room/RoomMessageContent.tsxapps/meteor/client/providers/MediaPlayerProvider/index.tsapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.tsapps/meteor/client/components/message/content/attachments/AttachmentsItem.tsxapps/meteor/client/sidebar/sections/SidebarCard.tsxapps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsxapps/meteor/client/components/message/content/Attachments.tsxapps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsxapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsxapps/meteor/client/components/message/content/attachments/FileAttachment.tsxapps/meteor/client/sidebar/sections/NowPlayingSection.tsxapps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/components/AudioPlayer/formatPlaybackTime.tsapps/meteor/client/providers/MediaPlayerProvider/index.tsapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/components/AudioPlayer/formatPlaybackTime.tsapps/meteor/client/providers/MediaPlayerProvider/index.tsapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/client/components/AudioPlayer/formatPlaybackTime.tsapps/meteor/client/providers/MediaPlayerProvider/index.tsapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/client/components/AudioPlayer/formatPlaybackTime.tsapps/meteor/client/providers/MediaPlayerProvider/index.tsapps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.
Applied to files:
.changeset/persistent-audio-player.md
🔇 Additional comments (12)
.changeset/persistent-audio-player.md (1)
6-6: LGTM!packages/i18n/src/locales/en.i18n.json (1)
3951-3951: LGTM!Also applies to: 4220-4220, 4862-4862
apps/meteor/client/sidebar/sections/SidebarCard.tsx (1)
1-5: LGTM!Also applies to: 10-16
apps/meteor/client/sidebar/sections/NowPlayingSection.tsx (1)
19-42: LGTM!Also applies to: 52-76
apps/meteor/client/sidebar/Sidebar.tsx (1)
9-9: LGTM!Also applies to: 25-25
apps/meteor/client/views/navigation/sidebar/Sidebar.tsx (1)
8-8: LGTM!Also applies to: 17-17
apps/meteor/client/components/message/content/Attachments.tsx (1)
4-15: LGTM!apps/meteor/client/components/message/content/attachments/AttachmentsItem.tsx (1)
8-18: LGTM!apps/meteor/client/components/message/content/attachments/FileAttachment.tsx (1)
4-15: LGTM!apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx (1)
31-81: LGTM!apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx (1)
76-82: LGTM!apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx (1)
68-74: LGTM!
There was a problem hiding this comment.
5 issues found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx">
<violation number="1" location="apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx:74">
P2: If the recovery URL (fetched during `handleMediaURLRecovery`) also fails to load — for example the redirect URL is also expired or there is a transient network issue — the `loadedmetadata` event never fires, so `isRecoveringRef` stays `true` permanently. All subsequent `error`/`stalled` events for that track are silently dropped, and no further recovery is ever attempted. The `catch` block only catches synchronous errors and promise rejections from `getRedirectURLInfo`, not the asynchronous loading failure after `node.load()`. Consider resetting `isRecoveringRef` when loading the new URL also errors out, so a later retry can still recover.</violation>
</file>
<file name="apps/meteor/client/sidebar/sections/SidebarCard.tsx">
<violation number="1" location="apps/meteor/client/sidebar/sections/SidebarCard.tsx:10">
P2: Missing `memo` wrapper inconsistent with sidebar convention. Every other exported component in the sidebar directory wraps with `memo`, which helps prevent unnecessary re-renders of this pure presentational card when the parent re-renders.</violation>
</file>
<file name="apps/meteor/client/sidebar/sections/NowPlayingSection.tsx">
<violation number="1" location="apps/meteor/client/sidebar/sections/NowPlayingSection.tsx:34">
P2: If opening the source room fails, the Now Playing jump action still adds the audio message id to the current URL because `useGoToRoom` handles errors internally and returns normally. That can leave the user on the wrong conversation with a stale `msg` query parameter; consider only setting the jump parameter after a confirmed navigation target, or changing `goToRoom` to expose failure to this caller.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (isRecoveringRef.current) { | ||
| return; | ||
| } | ||
| isRecoveringRef.current = true; |
There was a problem hiding this comment.
P2: If the recovery URL (fetched during handleMediaURLRecovery) also fails to load — for example the redirect URL is also expired or there is a transient network issue — the loadedmetadata event never fires, so isRecoveringRef stays true permanently. All subsequent error/stalled events for that track are silently dropped, and no further recovery is ever attempted. The catch block only catches synchronous errors and promise rejections from getRedirectURLInfo, not the asynchronous loading failure after node.load(). Consider resetting isRecoveringRef when loading the new URL also errors out, so a later retry can still recover.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx, line 74:
<comment>If the recovery URL (fetched during `handleMediaURLRecovery`) also fails to load — for example the redirect URL is also expired or there is a transient network issue — the `loadedmetadata` event never fires, so `isRecoveringRef` stays `true` permanently. All subsequent `error`/`stalled` events for that track are silently dropped, and no further recovery is ever attempted. The `catch` block only catches synchronous errors and promise rejections from `getRedirectURLInfo`, not the asynchronous loading failure after `node.load()`. Consider resetting `isRecoveringRef` when loading the new URL also errors out, so a later retry can still recover.</comment>
<file context>
@@ -0,0 +1,219 @@
+ if (isRecoveringRef.current) {
+ return;
+ }
+ isRecoveringRef.current = true;
+
+ try {
</file context>
| * Reusable elevated surface for sidebar sections: a padded, rounded, subtly | ||
| * bordered container in the sidebar tint. | ||
| */ | ||
| const SidebarCard = ({ children }: SidebarCardProps) => ( |
There was a problem hiding this comment.
P2: Missing memo wrapper inconsistent with sidebar convention. Every other exported component in the sidebar directory wraps with memo, which helps prevent unnecessary re-renders of this pure presentational card when the parent re-renders.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/sidebar/sections/SidebarCard.tsx, line 10:
<comment>Missing `memo` wrapper inconsistent with sidebar convention. Every other exported component in the sidebar directory wraps with `memo`, which helps prevent unnecessary re-renders of this pure presentational card when the parent re-renders.</comment>
<file context>
@@ -0,0 +1,16 @@
+ * Reusable elevated surface for sidebar sections: a padded, rounded, subtly
+ * bordered container in the sidebar tint.
+ */
+const SidebarCard = ({ children }: SidebarCardProps) => (
+ <Box m={8} p={10} borderRadius='x8' borderWidth='default' borderStyle='solid' borderColor='stroke-light' backgroundColor='surface-tint'>
+ {children}
</file context>
| if (!track.rid) { | ||
| return; | ||
| } | ||
| await goToRoom(track.rid); |
There was a problem hiding this comment.
P2: If opening the source room fails, the Now Playing jump action still adds the audio message id to the current URL because useGoToRoom handles errors internally and returns normally. That can leave the user on the wrong conversation with a stale msg query parameter; consider only setting the jump parameter after a confirmed navigation target, or changing goToRoom to expose failure to this caller.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/sidebar/sections/NowPlayingSection.tsx, line 34:
<comment>If opening the source room fails, the Now Playing jump action still adds the audio message id to the current URL because `useGoToRoom` handles errors internally and returns normally. That can leave the user on the wrong conversation with a stale `msg` query parameter; consider only setting the jump parameter after a confirmed navigation target, or changing `goToRoom` to expose failure to this caller.</comment>
<file context>
@@ -0,0 +1,78 @@
+ if (!track.rid) {
+ return;
+ }
+ await goToRoom(track.rid);
+ if (track.mid) {
+ void setMessageJumpQueryStringParameter(track.mid);
</file context>
… fetch; keyboard-accessible card
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/sidebar/sections/NowPlayingSection.tsx">
<violation number="1" location="apps/meteor/client/sidebar/sections/NowPlayingSection.tsx:48">
P3: Tracks that do not have `rid` now expose the Now Playing header as a focusable button even though activating it cannot navigate anywhere. Since `openConversation()` returns early for those tracks, it would be better to apply `role`, `tabIndex`, and keyboard/click handlers only when `track.rid` is present (or mark the control disabled) so keyboard and screen-reader users don't land on a dead button.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| role='button' | ||
| tabIndex={0} | ||
| onClick={() => void openConversation()} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| void openConversation(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
P3: Tracks that do not have rid now expose the Now Playing header as a focusable button even though activating it cannot navigate anywhere. Since openConversation() returns early for those tracks, it would be better to apply role, tabIndex, and keyboard/click handlers only when track.rid is present (or mark the control disabled) so keyboard and screen-reader users don't land on a dead button.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/sidebar/sections/NowPlayingSection.tsx, line 48:
<comment>Tracks that do not have `rid` now expose the Now Playing header as a focusable button even though activating it cannot navigate anywhere. Since `openConversation()` returns early for those tracks, it would be better to apply `role`, `tabIndex`, and keyboard/click handlers only when `track.rid` is present (or mark the control disabled) so keyboard and screen-reader users don't land on a dead button.</comment>
<file context>
@@ -45,7 +45,15 @@ const NowPlayingSection = () => {
alignItems='center'
minWidth={0}
flexGrow={1}
+ role='button'
+ tabIndex={0}
onClick={() => void openConversation()}
</file context>
| role='button' | |
| tabIndex={0} | |
| onClick={() => void openConversation()} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| void openConversation(); | |
| } | |
| }} | |
| role={track.rid ? 'button' : undefined} | |
| tabIndex={track.rid ? 0 : undefined} | |
| onClick={track.rid ? () => void openConversation() : undefined} | |
| onKeyDown={ | |
| track.rid | |
| ? (e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| void openConversation(); | |
| } | |
| } | |
| : undefined | |
| } |
Proposed changes
Playing an audio message attachment used to stop the moment you switched or closed the conversation, because the
<audio>element lived inside the message component and was destroyed when the message list unmounted.This PR makes audio playback persistent:
<audio>element is hoisted above the room layout into a newMediaPlayerProvider. Because it lives above the room, it survives navigation and message-list unmounting, so playback continues uninterrupted.useReloadOnError) is ported into the provider so the persistent element keeps recovering expired media URLs mid-playback.Architecture
providers/MediaPlayerProvider<audio>element + playback state (track,playing,currentTime,duration,playbackRate) and actions (play,toggle,seek,cyclePlaybackRate,close,isActive).components/AudioPlayer/AudioPlayerControlscompactmode for the sidebar).sidebar/sections/NowPlayingSectionAudioAttachmentMessage metadata (
rid,mid, sender) is threaded through the attachment chain so the card can show the sender and jump back to the conversation.How to test
Further comments
UI follows the sidebar card placement. The provider is placement-agnostic, so a floating / footer / topbar dock could reuse the same context later.
Follow-up (TODO)
The in-message/card controls (
apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx) are a temporary local copy of a controlled component being upstreamed to Fuselage: RocketChat/fuselage#2052. Once that ships in@rocket.chat/fuselage, delete the local copy (andformatPlaybackTime) and importAudioPlayerControlsfrom@rocket.chat/fuselageinstead.Task: ARCH-2202
Summary by CodeRabbit