Skip to content

Conversation

@samhvw8
Copy link
Contributor

@samhvw8 samhvw8 commented May 12, 2025

replace sound-play with use-sound for audio handling and add new sound files

Related GitHub Issue

Closes: #2187

Description

Move backend (node) play audio to webview play audio

Test Procedure

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Documentation Updates

Additional Notes


Important

Replaces sound-play with use-sound for webview audio handling, moving playback from backend to frontend.

  • Behavior:
    • Replaces sound-play with use-sound for audio handling in ChatView.tsx.
    • Moves audio playback from backend to webview, removing playSound handler from webviewMessageHandler.ts.
  • Dependencies:
    • Removes sound-play from package.json.
    • Adds use-sound to webview-ui/package.json.
  • Code Removal:
    • Deletes sound.ts which contained sound utility functions.
  • Tests:
    • Updates tests in ClineProvider.test.ts and ChatView.test.tsx to mock use-sound and verify sound behavior.
  • Misc:
    • Updates vite.config.ts to include .wav files in assetsInclude.

This description was created by Ellipsis for 48dc6cb7e2470cdf0f6664c59ecfe139b3120df0. You can customize this summary. It will automatically update as commits are pushed.

@changeset-bot
Copy link

changeset-bot bot commented May 12, 2025

⚠️ No Changeset found

Latest commit: 1fc13a9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels May 12, 2025
@samhvw8
Copy link
Contributor Author

samhvw8 commented May 12, 2025

@ScyDev @kbradsha can you test with this pr ?

@samhvw8 samhvw8 force-pushed the feat/move-audio-to-webview branch from 48dc6cb to e443218 Compare May 12, 2025 05:09
@samhvw8
Copy link
Contributor Author

samhvw8 commented May 12, 2025

@cte what do you think if we migrate to Vitest?

@samhvw8 samhvw8 force-pushed the feat/move-audio-to-webview branch 3 times, most recently from cdfd3d3 to 6201c98 Compare May 13, 2025 00:00
@hannesrudolph hannesrudolph linked an issue May 14, 2025 that may be closed by this pull request
@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap May 14, 2025
@KJ7LNW
Copy link
Contributor

KJ7LNW commented May 14, 2025

sound from this PR works for me on my Linux system (Oracle Linux 9, a RHEL 9 clone)

@samhvw8 samhvw8 force-pushed the feat/move-audio-to-webview branch from 60ec784 to 63c8062 Compare May 14, 2025 14:36
bgilbert6 pushed a commit to bgilbert6/Roo-Code that referenced this pull request May 14, 2025
* downloadMcp protobus migration

* added setIsDownloading(false) to error handling
samhvw8 added 2 commits May 15, 2025 21:15
replace sound-play with use-sound for audio handling and add new sound files
@samhvw8 samhvw8 force-pushed the feat/move-audio-to-webview branch from 63c8062 to 1fc13a9 Compare May 15, 2025 14:16
@samhvw8
Copy link
Contributor Author

samhvw8 commented May 15, 2025

sr for force push, can you test again @SmartManoj @KJ7LNW ?

Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested but the code seems reasonable to me. Have you confirmed that it works both in debug as well as in a built vsix?

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 15, 2025
@samhvw8
Copy link
Contributor Author

samhvw8 commented May 15, 2025

I haven't tested but the code seems reasonable to me. Have you confirmed that it works both in debug as well as in a built vsix?

@mrubens i remember @SmartManoj has test and build to vsix to test, i have commit from him

@KJ7LNW
Copy link
Contributor

KJ7LNW commented May 15, 2025

audio still works for me as of the current PR state

@KJ7LNW
Copy link
Contributor

KJ7LNW commented May 15, 2025

I think this PR introduced the following error:

index.html?id=26a653…se=webviewView:1060 The source list for the Content Security Policy directive 'connect-src' contains an invalid source: 'https://file+.vscode-resource.vscode-cdn.net'. It will be ignored.

here:

1fc13a9a0 src/core/webview/ClineProvider.ts     (sam hoang               2025-05-15 21:15:31 +0700  725)                        <meta http-equiv="Content-Security-Policy" content="default-src 'none'; font-src ${webview.cspSource}; style-src ${webview.cspSource} 'unsafe-inline'; img-src ${webview.cspSource} data:; media-src ${webview.cspSource}; script-src ${webview.cspSource} 'wasm-unsafe-eval' 'nonce-${nonce}' https://us-assets.i.posthog.com 'strict-dynamic'; connect-src https://openrouter.ai https://api.requesty.ai https://us.i.posthog.com https://us-assets.i.posthog.com https://file+.vscode-resource.vscode-cdn.net;">

@KJ7LNW
Copy link
Contributor

KJ7LNW commented May 15, 2025

...test and build to vsix to test

here is one that you can use:
https://www.linuxglobal.com/out/roo/roo-cline-3.17.0-feat-move-audio-to-webview.vsix

@samhvw8
Copy link
Contributor Author

samhvw8 commented May 15, 2025 via email

@KJ7LNW
Copy link
Contributor

KJ7LNW commented May 15, 2025

I tested as a one hour ago and it worked, I do not see any new commit messages. Is there something else for me to test?

]$ gh pr checkout 3487 --repo RooVetGit/Roo-Code
From https://github.com/RooVetGit/Roo-Code
 * branch              refs/pull/3487/head -> FETCH_HEAD
Already up to date.

@samhvw8
Copy link
Contributor Author

samhvw8 commented May 15, 2025

The source list for the Content Security Policy directive 'connect-src' contains an invalid source

that's weird, don't know why it still have error

@SmartManoj
Copy link
Contributor

SmartManoj commented May 16, 2025

@samhvw8, Would you please remove the mention in the commit message? Side effects

@SmartManoj
Copy link
Contributor

I think this PR introduced the following error:

index.html?id=26a653…se=webviewView:1060 The source list for the Content Security Policy directive 'connect-src' contains an invalid source: 'file+.vscode-resource.vscode-cdn.net'. It will be ignored.

here:

1fc13a9a0 src/core/webview/ClineProvider.ts     (sam hoang               2025-05-15 21:15:31 +0700  725)                        <meta http-equiv="Content-Security-Policy" content="default-src 'none'; font-src ${webview.cspSource}; style-src ${webview.cspSource} 'unsafe-inline'; img-src ${webview.cspSource} data:; media-src ${webview.cspSource}; script-src ${webview.cspSource} 'wasm-unsafe-eval' 'nonce-${nonce}' https://us-assets.i.posthog.com 'strict-dynamic'; connect-src https://openrouter.ai https://api.requesty.ai https://us.i.posthog.com https://us-assets.i.posthog.com https://file+.vscode-resource.vscode-cdn.net;">

Why did CI not captured that?

@samhvw8
Copy link
Contributor Author

samhvw8 commented May 16, 2025

I think this PR introduced the following error:
index.html?id=26a653…se=webviewView:1060 The source list for the Content Security Policy directive 'connect-src' contains an invalid source: 'file+.vscode-resource.vscode-cdn.net'. It will be ignored.
here:

1fc13a9a0 src/core/webview/ClineProvider.ts     (sam hoang               2025-05-15 21:15:31 +0700  725)                        <meta http-equiv="Content-Security-Policy" content="default-src 'none'; font-src ${webview.cspSource}; style-src ${webview.cspSource} 'unsafe-inline'; img-src ${webview.cspSource} data:; media-src ${webview.cspSource}; script-src ${webview.cspSource} 'wasm-unsafe-eval' 'nonce-${nonce}' https://us-assets.i.posthog.com 'strict-dynamic'; connect-src https://openrouter.ai https://api.requesty.ai https://us.i.posthog.com https://us-assets.i.posthog.com https://file+.vscode-resource.vscode-cdn.net;">

Why did CI not captured that?

yeah that's strange

@samhvw8
Copy link
Contributor Author

samhvw8 commented May 16, 2025

@SmartManoj thanks for extend my work,

close because we have this pr #3659
@KJ7LNW can you test again with this pr

@samhvw8 samhvw8 closed this May 16, 2025
@github-project-automation github-project-automation bot moved this from PR [Pre Approval Review] to Done in Roo Code Roadmap May 16, 2025
@SmartManoj
Copy link
Contributor

1fc13a9 It's not Windows related, but it addresses the following: "The extension worked in dev mode but not in build mode."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Failed to play sound effect inside Docker Container Sound Notifications not working on Ubuntu 20.04

5 participants