-
-
Notifications
You must be signed in to change notification settings - Fork 901
Switch from chewie_audio to asset_audio_player #509
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
Conversation
…o_player, modify Podfile to prevent build error
The |
This is working well too on a physical iOS device (I tested on iPad). The only problem I have is that the background blur behind the player controls is messing up the foreground as well, causing the whole widget to be blurred. This was also a problem with chewie though and I am not sure if this is due to external factors perhaps because I do not believe I have seen any other complaints of this type. It applies on iPad and on iOS simulator for me. |
@erickok should I remove this blur? I've seen it a few times on the simulator as well, but a hot reload fixes it if I remember correctly. |
Leave it. We'll see is this problem persists in production. |
@ryan-berger waht do you think of this PR, replacing chewie_audio for asset_audio_player (with our own UI that is the same as chewie_audio actually). I don't love having our own UI - more maintenance - but it swaps a dependency for one that is better maintained. I am not sure what your plan is for chewie_audio... |
@erickok Although I don't like chewie_audio very much, I also don't like the extra maintenance burden this adds. It also feels like we are trading a bad solution for a less worse solution when there is a better solution on the horizon: modularity. If we want to move towards modularity, this would be a good use case for doing so, and I would much prefer maintaining it in this repo as a sub-library than also having to maintain chewie_audio. Then instead of replacing a heavy dependency for a light weight dependency, we can get rid of both but optionally allow this one. A perfect test case in my opinion. I'm going to go update chewie_audio so that we can at least have compatibility with 1.26, but I want to hold off merging this until further discussion of modularization. |
I have quite concrete ideas on how to modularize this lib, very similar to the image API I've proposed, but I want to do that once we get most of the issues/bugs/PRs out of the way. Fair enough we can postpone replacing the audio player until such a point. I'll start a discussion on this modularization soon. |
Closing as Sub6Resources/chewie_audio#12 solves the main reason for this PR |
Fixes #399
BREAKING updates minimum deploment target to iOS 9.0 - however this should not be a huge deal since Flutter officially dropped support for iOS 8.0 (see here)
I borrowed the UI from chewie so that we would keep the Material/Cupertino style audio player, but with this implementation the errors on iOS should be gone now. This implementation would also allow us to potentially load audio from local files as well, if this is a feature we think should be added.
I don't have a physical iOS device but I tested on a Simulator and scrubbing through the progressbar felt a bit janky, if someone could test on a physical device and report their experience that would be great! Edit: I think I fixed the janky behavior in the last commit, but feedback is welcome still.