Fix blur binds not working correctly

killergerbah · Jun 18, 2024 · 14225b8 · 14225b8
1 parent 87c6795
commit 14225b8
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 75 deletions.
diff --git 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<AsbplayerSettings>) => 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
@@ -252,6 +252,7 @@ interface Props {
         cardTextFieldValues: CardTextFieldValues,
         timestamp: number
     ) => void;
+    onSettingsChanged: (settings: Partial<AsbplayerSettings>) => 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<boolean>();
@@ -322,11 +324,9 @@ export default function VideoPlayer({
     const [playMode, setPlayMode] = useState<PlayMode>(PlayMode.normal);
     const [subtitlePlayerHidden, setSubtitlePlayerHidden] = useState<boolean>(false);
     const [appBarHidden, setAppBarHidden] = useState<boolean>(playbackPreferences.theaterMode);
-    const [subtitleAlignment, setSubtitleAlignment] = useState<SubtitleAlignment>(
-        playbackPreferences.subtitleAlignment
-    );
+    const [subtitleAlignment, setSubtitleAlignment] = useState<SubtitleAlignment>(subtitleSettings.subtitleAlignment);
     const [subtitlePositionOffset, setSubtitlePositionOffset] = useState<number>(
-        playbackPreferences.subtitlePositionOffset
+        subtitleSettings.subtitlePositionOffset
     );
     const showSubtitlesRef = useRef<IndexedSubtitleModel[]>([]);
     showSubtitlesRef.current = showSubtitles;
@@ -339,6 +339,7 @@ export default function VideoPlayer({
     const [alertMessage, setAlertMessage] = useState<string>('');
     const [alertSeverity, setAlertSeverity] = useState<Color>('info');
     const [lastMinedRecord, setLastMinedRecord] = useState<MinedRecord>();
+    const [trackCount, setTrackCount] = useState<number>(0);
     const [, forceRender] = useState<any>();
 
     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
@@ -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
@@ -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
@@ -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
@@ -34,13 +34,16 @@ export default class BlurSubtitlesHandler {
     async handle(command: Command<Message>, 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<SettingsUpdatedMessage> = {

diff --git 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,
                 };