Add Music visualiser #2040
Add Music visualiser #2040
Conversation
| @@ -1,9 +1,14 @@ | |||
| <template> | |||
| <v-btn | |||
| :icon="isFavorite ? IMdiHeart : IMdiHeartOutline" | |||
There was a problem hiding this comment.
The play pause button would not be centred in the middle without adding another button. I thought the like button made the most sense to add but to do that I needed to make it resizable otherwise it would be too small. I did that the same way that the other buttons that show on both the minimised media player and the full screen media player do it, but also added a default size to make it backwards compatible.
There was a problem hiding this comment.
Ah I see it now, I think I have figured it out, I wrote this code a while ago so my decision making process isn't super clear anymore but I think the reason why I am using v-if and v-else is because I used the shuffle button as a template which does it that way and I guess I didn't know any better, I'll fix it.
There was a problem hiding this comment.
I think this doesn't need its own component, since it's small enough. This button should toggle a ref inside the music playback page
| @@ -50,11 +51,17 @@ | |||
| <time-slider /> | |||
| </v-row> | |||
| <v-row class="justify-center align-center"> | |||
| <audio-visualise-button size="x-large" /> | |||
There was a problem hiding this comment.
In my opinion, this button should be at the top right in the app bar, since it's not related to playback at all, it's just aesthetic.
There was a problem hiding this comment.
I agree that it should probably be elsewhere, but I feel that it would be even more out of place in the top right since it would be floating alone, in my mind it would make more sense if there was already something there like a casting button. The placement doesn't bother me that much though, so I don't mind changing it if you want.
frontend/src/store/playerElement.ts
Outdated
| const visualiserContainer = | ||
| document.querySelectorAll<HTMLElement>('.visualiser')[0]; | ||
| const audioElement = document.querySelector<HTMLMediaElement>('audio'); |
There was a problem hiding this comment.
This is a really bad practice at Vue. Besides you don't need this at all.
First of all, the visualizer itself warrants it's own component (unlike the button), where the only element is a canvas. Then, you reference that element by accessing it's ref. More info: https://vuejs.org/guide/essentials/template-refs.html
When that button gets clicked in the music player, you can toggle a boolean ref that toggles this component with v-if. in the onMounted hook, you attach the AudioMotionAnalyzer DOM, while on onBeforeUnmount or onUnmounted you destroy it (you don't right now, which will eventually lead to memory leaks).
For accessing the dom element of the media player, you only need to import it from here. Do note that won't work in development, just in production (as noted here), so you must build the client using npm run build.
With that architecture, you don't need to touch the playerlement store for anything and you'd be good with this with perhaps 50 lines only.
There was a problem hiding this comment.
Thanks for the help, I'm aware my code is pretty awful and I really appreciate the guidance, I should be able to get this fixed now I know what to do and how to do it.
|
@flip-dots You can also wrap the music visualizer alongside |
I tried implementing this but it causes the visualiser and carousel to glitch around during the transition and I don't really know how to fix it. I think it might be something to do with the CSS applying after the transition instead of before or maybe something to do with the generation of the canvas but I'm not sure. |
Cloudflare Pages deployment
|
|
Kudos, SonarCloud Quality Gate passed!
|
|
So, I accidentally managed to close this pull request and it wont let me reopen it so I made another one. Sorry… |
This adds a music visualiser option when you fullscreen the music player which looks cool as heck.
Since the discussion I have changed the colours, decreased the number of bars, and increased the smoothing value and sample size to make it look better on 60hz monitors. I still recommend playing around with the parameters though.
Link to discussion
Example Screenshot: