From 14225b8e7291455c6970623a2fe34ec4b685d708 Mon Sep 17 00:00:00 2001 From: R-J Lim Date: Mon, 17 Jun 2024 19:09:46 -0700 Subject: [PATCH] Fix blur binds not working correctly --- common/app/components/App.tsx | 2 + common/app/components/VideoPlayer.tsx | 48 ++++++++++++------- common/app/hooks/use-subtitle-styles.ts | 6 +-- common/app/services/playback-preferences.ts | 48 ------------------- common/src/message.ts | 1 + .../handlers/video/blur-subtitles-handler.ts | 17 ++++--- extension/src/services/key-bindings.ts | 1 + 7 files changed, 48 insertions(+), 75 deletions(-) diff --git a/common/app/components/App.tsx b/common/app/components/App.tsx index 93d189d0..71cb7fb0 100755 --- a/common/app/components/App.tsx +++ b/common/app/components/App.tsx @@ -152,6 +152,7 @@ interface RenderVideoProps { cardTextFieldValues: CardTextFieldValues, timestamp: number ) => void; + onSettingsChanged: (settings: Partial) => void; onAnkiDialogRewind: () => void; onError: (error: string) => void; onPlayModeChangedViaBind: (oldPlayMode: PlayMode, newPlayMode: PlayMode) => void; @@ -1149,6 +1150,7 @@ function App({ origin, logoUrl, settings, extension, fetcher, onSettingsChanged, miningContext={miningContext} ankiDialogOpen={ankiDialogOpen} seekRequest={videoPlayerSeekRequest} + onSettingsChanged={onSettingsChanged} onAnkiDialogRequest={handleAnkiDialogRequestFromVideoPlayer} onAnkiDialogRewind={handleAnkiDialogRewindFromVideoPlayer} onError={handleError} diff --git a/common/app/components/VideoPlayer.tsx b/common/app/components/VideoPlayer.tsx index 1888c4e9..c2f90438 100755 --- a/common/app/components/VideoPlayer.tsx +++ b/common/app/components/VideoPlayer.tsx @@ -252,6 +252,7 @@ interface Props { cardTextFieldValues: CardTextFieldValues, timestamp: number ) => void; + onSettingsChanged: (settings: Partial) => void; onAnkiDialogRewind: () => void; onError: (error: string) => void; onPlayModeChangedViaBind: (oldPlayMode: PlayMode, newPlayMode: PlayMode) => void; @@ -284,6 +285,7 @@ export default function VideoPlayer({ onError, onPlayModeChangedViaBind, onAnkiDialogRewind, + onSettingsChanged, }: Props) { const classes = useStyles(); const poppingInRef = useRef(); @@ -322,11 +324,9 @@ export default function VideoPlayer({ const [playMode, setPlayMode] = useState(PlayMode.normal); const [subtitlePlayerHidden, setSubtitlePlayerHidden] = useState(false); const [appBarHidden, setAppBarHidden] = useState(playbackPreferences.theaterMode); - const [subtitleAlignment, setSubtitleAlignment] = useState( - playbackPreferences.subtitleAlignment - ); + const [subtitleAlignment, setSubtitleAlignment] = useState(subtitleSettings.subtitleAlignment); const [subtitlePositionOffset, setSubtitlePositionOffset] = useState( - playbackPreferences.subtitlePositionOffset + subtitleSettings.subtitlePositionOffset ); const showSubtitlesRef = useRef([]); showSubtitlesRef.current = showSubtitles; @@ -339,6 +339,7 @@ export default function VideoPlayer({ const [alertMessage, setAlertMessage] = useState(''); const [alertSeverity, setAlertSeverity] = useState('info'); const [lastMinedRecord, setLastMinedRecord] = useState(); + const [trackCount, setTrackCount] = useState(0); const [, forceRender] = useState(); useEffect(() => { @@ -348,9 +349,9 @@ export default function VideoPlayer({ }, [settings]); useEffect(() => { - setSubtitleAlignment(playbackPreferences.subtitleAlignment); - setSubtitlePositionOffset(playbackPreferences.subtitlePositionOffset); - }, [playbackPreferences]); + setSubtitleAlignment(subtitleSettings.subtitleAlignment); + setSubtitlePositionOffset(subtitleSettings.subtitlePositionOffset); + }, [subtitleSettings]); const autoPauseContext = useMemo(() => { const context = new AutoPauseContext(); @@ -525,6 +526,7 @@ export default function VideoPlayer({ playerChannel.onSubtitles((subtitles) => { setSubtitles(subtitles.map((s, i) => ({ ...s, index: i }))); + setTrackCount(Math.max(...subtitles.map((s) => s.track)) + 1); if (subtitles && subtitles.length > 0) { const s = subtitles[0]; @@ -816,15 +818,27 @@ export default function VideoPlayer({ useEffect(() => { return keyBinder.bindToggleBlurTrack( - (event, track) => { + (event, targetTrack) => { event.preventDefault(); - const originalValue = textSubtitleSettingsForTrack(subtitleSettings, track).subtitleBlur!; - const change = changeForTextSubtitleSetting({ subtitleBlur: !originalValue }, subtitleSettings, track); - setSubtitleSettings({ ...subtitleSettings, ...change }); + let newSubtitleSettings = { ...subtitleSettings }; + + for (let currentTrack = 0; currentTrack < trackCount; ++currentTrack) { + const originalValue = textSubtitleSettingsForTrack(subtitleSettings, currentTrack).subtitleBlur!; + const targetValue = currentTrack === targetTrack ? !originalValue : originalValue; + const change = changeForTextSubtitleSetting( + { subtitleBlur: targetValue }, + newSubtitleSettings, + currentTrack + ); + newSubtitleSettings = { ...newSubtitleSettings, ...change }; + } + + onSettingsChanged(newSubtitleSettings); + setSubtitleSettings(newSubtitleSettings); }, () => false ); - }, [keyBinder, subtitleSettings]); + }, [keyBinder, subtitleSettings, trackCount, onSettingsChanged]); useEffect(() => { return keyBinder.bindOffsetToSubtitle( @@ -1187,9 +1201,9 @@ export default function VideoPlayer({ const handleSubtitleAlignment = useCallback( (alignment: SubtitleAlignment) => { setSubtitleAlignment(alignment); - playbackPreferences.subtitleAlignment = alignment; + onSettingsChanged({ subtitleAlignment: alignment }); }, - [playbackPreferences] + [onSettingsChanged] ); useEffect(() => { @@ -1215,14 +1229,14 @@ export default function VideoPlayer({ setSubtitlePositionOffset((offset) => { const newOffset = shouldIncreaseOffset ? --offset : ++offset; - playbackPreferences.subtitlePositionOffset = newOffset; + onSettingsChanged({ subtitlePositionOffset: newOffset }); return newOffset; }); }; window.addEventListener('wheel', onWheel); return () => window.removeEventListener('wheel', onWheel); - }, [subtitleAlignment, displaySubtitles, playbackPreferences]); + }, [subtitleAlignment, displaySubtitles, playbackPreferences, onSettingsChanged]); const handleClick = useCallback(() => { if (playing()) { @@ -1249,7 +1263,7 @@ export default function VideoPlayer({ }, [showCursor]); const handleAlertClosed = useCallback(() => setAlertOpen(false), []); - const trackStyles = useSubtitleStyles(subtitleSettings); + const trackStyles = useSubtitleStyles(subtitleSettings, trackCount ?? 1); const { getSubtitleDomCache } = useSubtitleDomCache( subtitles, useCallback( diff --git a/common/app/hooks/use-subtitle-styles.ts b/common/app/hooks/use-subtitle-styles.ts index 97d5a5ec..41aac2ad 100644 --- a/common/app/hooks/use-subtitle-styles.ts +++ b/common/app/hooks/use-subtitle-styles.ts @@ -8,10 +8,10 @@ interface TrackStyles { classes: string; } -export const useSubtitleStyles = (settings: SubtitleSettings) => { +export const useSubtitleStyles = (settings: SubtitleSettings, trackCount: number) => { return useMemo(() => { const tracks: TrackStyles[] = []; - for (let track = 0; track <= settings.subtitleTracksV2.length; ++track) { + for (let track = 0; track < trackCount; ++track) { const s = textSubtitleSettingsForTrack(settings, track) as TextSubtitleSettings; tracks.push({ styles: computeStyles(s), @@ -20,5 +20,5 @@ export const useSubtitleStyles = (settings: SubtitleSettings) => { }); } return tracks; - }, [settings]); + }, [settings, trackCount]); }; diff --git a/common/app/services/playback-preferences.ts b/common/app/services/playback-preferences.ts index 619c8d64..abceac26 100644 --- a/common/app/services/playback-preferences.ts +++ b/common/app/services/playback-preferences.ts @@ -6,8 +6,6 @@ const volumeKey = 'volume'; const theaterModeKey = 'theaterMode'; const offsetKey = 'offset'; const displaySubtitlesKey = 'displaySubtitles'; -const subtitleAlignmentKey = 'subtitleAlignment'; -const subtitlePositionOffetKey = 'subtitlePositionOffset'; const defaultVolume = 100; interface PlaybackPrefSettings { @@ -75,52 +73,6 @@ export default class PlaybackPreferences { } } - get subtitleAlignment() { - if (this._extension.supportsAppIntegration) { - return this._settings.subtitleAlignment; - } - - const val = this._storage.get(subtitleAlignmentKey); - - if (val === null) { - return 'bottom'; - } - - return val as SubtitleAlignment; - } - - // FIXME: Leaky abstraction - subtitlAlignmentKey and subtitlePositionOffsetKey must have the same values - // as the keys in the settings.ts for these setters to update the shared settings as well - set subtitleAlignment(alignment: SubtitleAlignment) { - if (this._extension.supportsAppIntegration) { - this._extension.setSettings({ subtitleAlignment: alignment }); - } else { - this._storage.set(subtitleAlignmentKey, String(alignment)); - } - } - - get subtitlePositionOffset() { - if (this._extension.supportsAppIntegration) { - return this._settings.subtitlePositionOffset; - } - - const val = this._storage.get(subtitlePositionOffetKey); - - if (val === null) { - return 100; - } - - return Number(val); - } - - set subtitlePositionOffset(offset: number) { - if (this._extension.supportsAppIntegration) { - this._extension.setSettings({ subtitlePositionOffset: offset }); - } else { - this._storage.set(subtitlePositionOffetKey, String(offset)); - } - } - get displaySubtitles() { const value = this._storage.get(displaySubtitlesKey); diff --git a/common/src/message.ts b/common/src/message.ts index 755aa106..56339afa 100755 --- a/common/src/message.ts +++ b/common/src/message.ts @@ -278,6 +278,7 @@ export interface ToggleSubtitlesMessage extends Message { export interface BlurSubtitlesMessage extends Message { readonly command: 'blur-subtitles'; readonly track: number; + readonly trackCount: number; } export interface ToggleSubtitlesInListFromVideoMessage extends Message { diff --git a/extension/src/handlers/video/blur-subtitles-handler.ts b/extension/src/handlers/video/blur-subtitles-handler.ts index 2e99acab..9f00f5f4 100755 --- a/extension/src/handlers/video/blur-subtitles-handler.ts +++ b/extension/src/handlers/video/blur-subtitles-handler.ts @@ -34,13 +34,16 @@ export default class BlurSubtitlesHandler { async handle(command: Command, sender: chrome.runtime.MessageSender) { const message = command.message as BlurSubtitlesMessage; const subtitleSettings: SubtitleSettings = await this.settings.get(subtitleSettingsKeys); - const oldValue = textSubtitleSettingsForTrack(subtitleSettings, message.track); - const change = changeForTextSubtitleSetting( - { subtitleBlur: !oldValue.subtitleBlur }, - subtitleSettings, - message.track - ); - await this.settings.set(change); + let newSettings = { ...subtitleSettings }; + + for (let currentTrack = 0; currentTrack < message.trackCount; ++currentTrack) { + const originalValue = textSubtitleSettingsForTrack(subtitleSettings, currentTrack).subtitleBlur!; + const targetValue = currentTrack === message.track ? !originalValue : originalValue; + const change = changeForTextSubtitleSetting({ subtitleBlur: targetValue }, newSettings, currentTrack); + newSettings = { ...newSettings, ...change }; + } + + await this.settings.set(newSettings); this.tabRegistry.publishCommandToVideoElements((videoElement) => { const settingsUpdatedCommand: ExtensionToVideoCommand = { diff --git a/extension/src/services/key-bindings.ts b/extension/src/services/key-bindings.ts index 4fcef9f2..d3e9ee0e 100755 --- a/extension/src/services/key-bindings.ts +++ b/extension/src/services/key-bindings.ts @@ -193,6 +193,7 @@ export default class KeyBindings { message: { command: 'blur-subtitles', track: track, + trackCount: Math.max(...context.subtitleController.subtitles.map((s) => s.track)) + 1, }, src: context.video.src, };