-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IYY-279: Visitors can listen to Posts - aka Audio player component #468
base: iyy
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for dev-component-library-twig ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…except for active setting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works well. A few notes:
- I can't see the progress line or volume line that shows up in your video. The controls still work though. See screenshot.
- On mobile, volume doesn't work at all. Desktop works fine. I suspect since you have mobile physical volume controls that isn't a problem, but what do you think about hiding it on mobile?
@codechefmarc Oh, I see. I'll test this in Chrome and figure out those bugs. Also, on mobile, on Android the volume controls work as they should. I will test on an iPhone (assuming you tested on an iPhone?), though. Thank you! |
Ahh yup, I tested on an iPhone, I should have said. :) |
…s when play is clicked
@codechefmarc Thank you! Apparently, Apple don't allow programmatic adjustments to volume. So, it looks like iPhones are limited to using the physical volume buttons. It does make me wonder if we should hide playback speed and volume on mobile. |
The playback speed worked fine for me on iOS, but, the target was a bit small to tap is all. |
@codechefmarc Could you please give this another look? On mobile, I made both the track and the thumb/dial thicker and easier to move with your thumb. On iOS, the volume slider is now hidden. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferring to @miketullo95 for ultimate verification, but I noticed that the icon next to the speed is clickable, but the actual 1.0x is not to change it? Should this whole "box" be clickable to change the speed?
Also some hover elements don't change mouse icon; should they? Like hovering over the play/pause or the speed so you know you can click it? Also deferring. :) |
Oh, if the audio is finished playing, the pause button stays up, and if you change the location of the audio afterward, it does not continue playing. Should it reset to a play button icon to show it's not playing any more? |
Screen.Recording.2025-03-05.at.11.29.20.AM.movHere is a video showing most all of that above. :D |
IYY-279: Visitors can listen to Posts - aka Audio player component
Description of work
Testing Link(s)
Functional Review Steps
iyy-audio-on-posts.mp4
Design Review
Accessibility Review