search

PRX / Play-Next.js

PRX audio embed player and landing pages.
GNU Affero General Public License v3.0
3 stars 0 forks source link

136: Transcripts closed captions #141

Closed rpeterman-gp closed 6 months ago

rpeterman-gp commented 7 months ago

Closes #136 Closes #137 Closes #139 Closes #131

To Review

...then...

rpeterman-gp commented 6 months ago

@farski Just push up some changes to address most of your notes.

Couldn't address note 1 do to the way the progress bar scrub state is handled. The scrub position is currently only scoped to the progress bar component state. Would need to refactor the root player state to include a scrub position, and could result in hammering a re-render of the entire app as you scrub, which could have performance problems. I will need more time to make that work smoothly. Also, having to find the correct caption cue out of thousands of cues as the user scrubs may not be performant. I am hoping the binary search I just added to the current caption render precess will make this feature possible.

I did my best to resolve note 8, but may still occur, though very briefly. Has to do with how long it takes to rendering hundreds to thousands of caption elements and their children. They have to all render before the current one can be scrolled to. I added a loader message to display when that render slow.

As for notes 6 and 7, those seem to be user preferences. How would we feel about adding a cc settings menu to control highlighting fidelity and caption layout?

rpeterman-gp commented 6 months ago
  1. I should point out that the high fidelity highlighting is styled to not occur for users that have their prefers-reduced-motion set to reduced. You can test this out by opening the dev inspector and press Cmd+Shift+P and filtering for reduced. Select the "Emulate CSS prefers-reduced-motion: reduced" item. Repeat to turn off the emulation.
rpeterman-gp commented 6 months ago

Just pushed up a change that should address 3, 4, and 5, along with some performance optimizations.

rpeterman-gp commented 6 months ago

Pushed up changes to fix the build. Upgrade Node, Next, and React version, on long with several other dependencies. When reviewing, you may want to remove your node_modules directory, then run yarn before starting dev server.

rpeterman-gp commented 6 months ago

Pushed up changes to update CC feed scroll with changes to player current time when audio has not been played. User had to have played the audio with an interaction before timeupdate events would trigger for scrub or skipped time changes.

As for the CC feed scroll position being remembered after a page reload, that is a Firefox bug (https://bugzilla.mozilla.org/show_bug.cgi?id=706792). However, I was able to find a way to handle it by conditionally reseting the scrollTop of the scroll area on init. Should still scroll to the current cue if you scrub or skip time before opening CCs.

rpeterman-gp commented 6 months ago

@farski To be sure that the expected tickets are created accurately, can you create those tickets then give your final approval on this PR when you feel that remaining work has been covered for our future attention? I want to avoid any more delays going back-and-forth with the creation of those tickets.