Farmer app audio support #290
Conversation
…mer-app-audio-support
There was a problem hiding this comment.
Thanks @FaithDaka - all looking good so far.
I tested that the audios could play and also checked adding a second audio element that it would handle stopping one when the next starts (which I think it does as you have created a single audio element within the service, so changing the url kills the previous).
It would be good if you could also add a test div to play audio from a local asset file (like you have added) and check if the implementation works. I've also added a few minor non-blocking comments inline.
| @@ -0,0 +1,8 @@ | |||
| <button (click)="togglePlayback()" class="audio-button"> | |||
| <div *ngIf="isAudioPlaying; else playIcon"> | |||
There was a problem hiding this comment.
nit(non-blocking) - it would be good for the icon to change when play button first clicked (currently only changes when clicked twice, once to start and once to pause), so that the user gets feedback that the audio click was registered. Could also consider putting the icon in a mat-button to get a ripple effect
nit(non-blocking) - feel free if you want to use the @if(...) and @else(...) syntax. Can also probably just put the if/else around the icon and not need additional ng-template
| border: none; | ||
| padding: 5px; | ||
| .mat-icon{ | ||
| color: #7292C7; |
There was a problem hiding this comment.
nit(non-blocking): might be good to link the color to the primary/secondary pallette. You could use var(--color-secondary) to get the deeper blue, or if you want to register a lighter variant you can define it in libs\theme\src\themes\_mixins.scss, testing with something like
--color-secondary-200: #{map-get($accent, 200)};
The full list of colors that could be registered can be seen in libs\theme\src\themes\picsa-default.scss (currently just the 500 and 50 weight variants are in use)
| } | ||
|
|
||
| isPlaying(): boolean { | ||
| return this.audioService.isPlaying(); |
There was a problem hiding this comment.
nit(non-blocking): I like how you have linked the audio play detection to the service, as it's possible that the service will stop component playback (e.g. if the user tries to start a second audio while the first one is playing), however you might need to remove the isAudioPlaying variable from this component to avoid confusion
| providedIn: 'root', | ||
| }) | ||
| export class AudioService { | ||
| private audio: HTMLAudioElement; |
There was a problem hiding this comment.
+1 - nice you were able to do get things working with the standard html audio api. In another project I've used HowlerJS, however it looks like the api should be sufficient for now
…/picsa-apps into farmer-app-audio-support
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 1b9eb91. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
|
@chrismclarke The changes you suggested have been made. The video shows the fix for the icon bug (it was changing after 2 clicks instead of 1) Screen.Recording.2024-11-07.at.10.55.31.mov |
Description
Discussion
Screenshots / Videos
Turn your volume on to hear the audio.
Screen.Recording.2024-06-26.at.12.14.49.mov