forem / DEV-ios

DEV Community iOS App
GNU General Public License v3.0
357 stars 97 forks source link

Native audio play #207

Closed fdocr closed 4 years ago

fdocr commented 4 years ago

What type of PR is this?

Description

This PR works on handling messages sent from javascript to trigger native AVPlayer playback of Podcasts. This is the PR with the web updates required to make this feature work.

Related Tickets & Documents

Added to documentation?

What gif best describes this PR or how it makes you feel?

cat djs

fdocr commented 4 years ago

Hey @benhalpern, this PR works with a local development server that sends the webkit messages (dev.to PR in the description above). This makes the app play the audio natively instead from within the WKWebView, but I've been running into some problems in order to keep the Podcast bar UI in order. Would love to hear your thoughts on this:

Once the load & play messages are received, the audio starts to play correctly. The problem comes from the javascript logic that heavily relies on the HTML audio element to keep the current status of the playback. By this I mean things like "if the web audio isn't playing/loading then the progress bars won't be updated" and more importantly to figure out whether the audio is playing or not it is also relying on the audio element tag state.

This causes the web UI to always look like it's "initializing..." when the native audio has already started to play the Podcast (because the web audio tag isn't playing anything):

Simulator Screen Shot - iPhone 11 Pro Max - 2020-02-16 at 21 47 02

Ways I think we can work around this issue:

  1. Refactor app/assets/javascripts/initializers/initializePodcastPlayback.js from thepracticaldev/dev.to to work without relying on the audio HTML element for "playback state management". This way we can send messages from Swift into javascript to update the playback UI when we don't have the HTML audio element playing anything.
  2. Doubling down on a native implementation for audio consumption that would hide the podcast UI altogether when browsing from the app. This would require a native podcast player implementation. I think we can keep the load & play messages to trigger the native podcast player from within the web but the controls would now be native.
  3. Other ideas?

I think we can make either option work, but it comes down to a tradeoff between "slightly larger refactor on web" vs "more in-depth native implementation".

I've already fiddled with a few ideas while trying to make the least amount of changes possible. For instance using mutation observers in JS and from within Swift send status updates on data tags. This would help keep track of timing and other "events", but they quickly start to feel too "hack-ish" and it's still quite a big change to initializePodcastPlayback.js, so in the end it still feels like a refactor.

fdocr commented 4 years ago

We can definitely manage those other events too and I believe we do have the "skip ahead" functionality in place for the web UI. It seems to work by clicking on the progress bar (uses the goToTime() method found here), I'll make sure to include them.

I'll try to refactor a bit the way the UI handles the "state management" in the web (JS) side of things to keep the UI consistent and make sure the play/pause and the audio time works (00:00 -> 00:01 -> 00:02 -> etc). Because like I said before the HTML audio tag is being used to figure out if audio is being played or if is paused, and since we're not playing anything in the webview then the UI always shows the Podcast as "Initializing..."

I'll definitely ping you again to give it another look when I've worked around this and included the other inline comments 👍🏼

fdocr commented 4 years ago

Hey @benhalpern, back at it again. Now this last update includes the following changes:

Screen Shot 2020-02-18 at 13 20 25

benhalpern commented 4 years ago

This looks great!

fdocr commented 4 years ago

Finally taking the PR out from Draft mode. Some thoughts on the current state of the PR:

  1. The Podcast functionality started to bloat the ViewController way too much. I extracted this into a PodcastManager which interacts with the ViewController using a delegate.
  2. Now includes integration with the controls in the "locked screen" to play/pause the podcast.
  3. The only issue here is codeclimate complaining about the length of ViewController (even after extracting the Podcast functionality still 420 lines vs 400 limit). I can continue to clear out some lines here to meet this 400 line limit, but the real fix would be some more functionality extraction.

Right now the PR looks solid from my end after testing.