refactor(frontend): サウンド再生まわりのリファクタリング#16990
refactor(frontend): サウンド再生まわりのリファクタリング#16990kakkokari-gtyih wants to merge 4 commits intomisskey-dev:developfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #16990 +/- ##
===========================================
+ Coverage 13.97% 14.01% +0.04%
===========================================
Files 237 238 +1
Lines 11273 11289 +16
Branches 3728 3734 +6
===========================================
+ Hits 1575 1582 +7
- Misses 7578 7586 +8
- Partials 2120 2121 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the frontend sound playback system by converting function-based utilities to a class-based SoundManager approach. The refactoring improves code organization and maintainability while preserving all existing functionality.
Key changes:
- Converted standalone sound utility functions to a
SoundManagerclass with instance methods - Created a singleton
soundManagerinstance exported from@/sound.js - Updated API property names (
soundSource→sourceNode) and method names (playMisskeySfx→playSfx,playMisskeySfxFile→playSfxFile,getSoundDuration→getDuration)
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/frontend/src/utility/sound.ts | Complete refactoring: converted standalone functions to SoundManager class, improved type organization, added defensive _DEV_ check |
| packages/frontend/src/sound.ts | New file exporting singleton soundManager instance |
| packages/frontend/src/widgets/WidgetJobQueue.vue | Updated to use soundManager instance and new sourceNode property name |
| packages/frontend/src/ui/deck/tl-note-notification.ts | Updated to use soundManager.getDuration() and soundManager.playSfxFile() |
| packages/frontend/src/ui/common/common.vue | Updated to use soundManager.playSfx() |
| packages/frontend/src/pages/settings/sounds.sound.vue | Updated to use soundManager.getDuration() and soundManager.playSfxFile() |
| packages/frontend/src/pages/reversi/index.vue | Updated to use soundManager.playUrl() |
| packages/frontend/src/pages/reversi/game.board.vue | Updated to use soundManager.playUrl() |
| packages/frontend/src/pages/drop-and-fusion.game.vue | Updated to use soundManager methods and new sourceNode property name |
| packages/frontend/src/pages/chat/room.vue | Updated to use soundManager.playSfx() |
| packages/frontend/src/pages/chat/XMessage.vue | Updated to use soundManager.playSfx() |
| packages/frontend/src/components/MkStreamingNotesTimeline.vue | Updated to use soundManager.playSfxFile() and soundManager.playSfx() |
| packages/frontend/src/components/MkReactionsViewer.reaction.vue | Updated to use soundManager.playSfx() |
| packages/frontend/src/components/MkNoteDetailed.vue | Updated to use soundManager.playSfx() |
| packages/frontend/src/components/MkNote.vue | Updated to use soundManager.playSfx() |
| packages/frontend/src/boot/main-boot.ts | Updated to use soundManager.playSfx() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * 直接URLを指定して再生 | ||
| */ | ||
| public async playUrl(url: string, opts: AudioOptions) { | ||
| if (opts.volume === 0) return; |
There was a problem hiding this comment.
The volume check should also handle undefined values. Currently, if opts.volume is undefined, the check opts.volume === 0 will be false, allowing execution to continue. While all current call sites provide a volume value, this could lead to unexpected behavior if the method is called without a volume parameter in the future. Consider changing to if (opts.volume === 0 || opts.volume === undefined) or if (!opts.volume) to be more defensive.
| if (opts.volume === 0) return; | |
| if (opts.volume === 0 || opts.volume === undefined) return; |
There was a problem hiding this comment.
new SoundManagerしてる箇所が1箇所しかないならsrc/sound.tsにまとめる方が良さそう
What
可読性の向上
Why
Additional info (optional)
Checklist