Improve factoring of FocusedSkin

src/AnimationWrapper.js

@@ -1,64 +0,0 @@
-import React from "react";
-
-import { delay } from "rxjs/operators";
-import { Subject, combineLatest, timer } from "rxjs";
-import Disposable from "./Disposable";
-
-export default class AnimationWrapper extends React.Component {
-  constructor(props) {
-    super(props);
-    this._disposable = new Disposable();
-    // TODO: Handle the case were we come from a permalink
-    this._webampLoadedEvents = new Subject();
-    this._transitionBeginEvents = new Subject();
-    const transitionComplete = this._transitionBeginEvents.pipe(delay(500));
-
-    // Emit after both Webamp has loaded, and the transition is complete
-    const startWebampFadein = combineLatest([
-      this._webampLoadedEvents,
-      transitionComplete,
-    ]);
-
-    // This value matches the opacity transition timing for `#webamp` in CSS
-    const webampFadeinComplete = startWebampFadein.pipe(delay(400));
-
-    // Once webamp has loaded and the transition is complete, we can start the Webamp fadein
-    // Once the webamp fadein is complete (400ms) we are "loaded"
-
-    this._disposable.add(
-      startWebampFadein.subscribe(() => {
-        document.body.classList.add("webamp-loaded");
-      }),
-      webampFadeinComplete.subscribe(() => {
-        this.props.setLoaded(true);
-      }),
-      () => {
-        document.body.classList.remove("webamp-loaded");
-      }
-    );
-  }
-
-  componentDidMount() {
-    if (this.props.initialPosition != null) {
-      this._disposable.add(
-        timer(0).subscribe(() => {
-          // TODO: Observe DOM and recenter
-          this.props.setCentered(true);
-          this._transitionBeginEvents.next(null);
-        })
-      );
-    } else {
-      this._transitionBeginEvents.next(null);
-    }
-  }
-
-  componentWillUnmount() {
-    this._disposable.dispose();
-  }
-
-  render() {
-    return this.props.children({
-      handleWebampLoaded: () => this._webampLoadedEvents.next(null),
-    });
-  }
-}

src/FocusedSkin.js

@@ -1,17 +1,17 @@
-import React, { useEffect, useMemo, useRef, useState } from "react";
+import React, { useEffect, useMemo, useState } from "react";
 import { useSelector } from "react-redux";
-import { useActionCreator, useWindowSize } from "./hooks";
+import { useActionCreator, useWindowSize, useWebampAnimation } from "./hooks";
 import * as Selectors from "./redux/selectors";
 import * as Actions from "./redux/actionCreators";
 
 import WebampComponent from "./WebampComponent";
 import * as Utils from "./utils";
-import { SCREENSHOT_HEIGHT, SCREENSHOT_WIDTH, API_URL } from "./constants";
+import { SCREENSHOT_HEIGHT, SCREENSHOT_WIDTH } from "./constants";
 import { fromEvent } from "rxjs";
 import Metadata from "./components/Metadata";
 import SkinReadme from "./SkinReadme";
-import AnimationWrapper from "./AnimationWrapper";
 
+// TODO: Move to epic
 function useSkinKeyboardControls() {
   const selectRelativeSkin = useActionCreator(Actions.selectRelativeSkin);
   useEffect(() => {
@@ -33,7 +33,6 @@ function useCenteredState() {
   const { windowWidth, windowHeight } = useWindowSize();
   return useMemo(
     () => ({
-      centered: true,
       top: (windowHeight - SCREENSHOT_HEIGHT) / 2,
       left: (windowWidth - SCREENSHOT_WIDTH) / 2,
       height: SCREENSHOT_HEIGHT,
@@ -43,13 +42,15 @@ function useCenteredState() {
   );
 }
 
-function BaseFocusedSkin({
-  initialPosition,
-  centered,
-  hash,
-  handleWebampLoaded,
-  loaded,
-}) {
+function FocusedSkin() {
+  const hash = useSelector(Selectors.getSelectedSkinHash);
+  const initialPosition = useSelector(Selectors.getSelectedSkinPosition);
+
+  useSkinKeyboardControls();
+
+  const { handleWebampLoaded, loaded, centered } = useWebampAnimation({
+    initialPosition,
+  });
   const [previewLoaded, setPreviewLoaded] = useState(initialPosition != null);
   const centeredState = useCenteredState();
   const closeModal = useActionCreator(Actions.closeModal);
@@ -131,39 +132,4 @@ function BaseFocusedSkin({
   );
 }
 
-function Wrapper({ ...ownProps }) {
-  const hash = useSelector(Selectors.getSelectedSkinHash);
-  const initialPosition = useSelector(Selectors.getSelectedSkinPosition);
-  const [centered, setCentered] = useState(initialPosition == null);
-  const [loaded, setLoaded] = useState(false);
-
-  useSkinKeyboardControls();
-
-  const prevSkinHash = useRef(null);
-  useEffect(() => {
-    if (hash !== prevSkinHash.current) {
-      document.body.classList.remove("webamp-loaded");
-      prevSkinHash.current = hash;
-    }
-  }, [hash]);
-
-  const props = { ...ownProps, hash, initialPosition };
-  return (
-    <AnimationWrapper
-      initialPosition={initialPosition}
-      setCentered={setCentered}
-      setLoaded={setLoaded}
-    >
-      {({ handleWebampLoaded }) => (
-        <BaseFocusedSkin
-          {...props}
-          centered={centered}
-          handleWebampLoaded={handleWebampLoaded}
-          loaded={loaded}
-        />
-      )}
-    </AnimationWrapper>
-  );
-}
-
-export default Wrapper;
+export default FocusedSkin;

src/hooks.js

@@ -2,6 +2,9 @@ import * as Utils from "./utils";
 import { useMemo, useState, useEffect, useCallback } from "react";
 import { useDispatch } from "react-redux";
 
+import { delay } from "rxjs/operators";
+import { Subject, combineLatest, timer } from "rxjs";
+
 export function useWindowSize() {
   const [windowSize, setWindowSize] = useState(Utils.getWindowSize());
   useEffect(() => {
@@ -47,3 +50,53 @@ export function useActionCreator(actionCreator) {
     actionCreator,
   ]);
 }
+
+export function useWebampAnimation({ initialPosition }) {
+  const [loaded, setLoaded] = useState(false);
+  const [centered, setCentered] = useState(initialPosition == null);
+  const [webampLoadedEvents] = useState(() => new Subject());
+  const [transitionBeginEvents] = useState(() => new Subject());
+
+  // Emit after both Webamp has loaded, and the transition is complete
+  const startWebampFadein = useMemo(() => {
+    const transitionComplete = transitionBeginEvents.pipe(delay(500));
+    return combineLatest([webampLoadedEvents, transitionComplete]);
+  }, [transitionBeginEvents, webampLoadedEvents]);
+
+  // setLoaded(true) 400ms after startWebampFadein
+  useEffect(() => {
+    const webampFadeinComplete = startWebampFadein.pipe(delay(400));
+    const subscription = webampFadeinComplete.subscribe(() => setLoaded(true));
+    return () => subscription.unsubscribe();
+  }, [setLoaded, startWebampFadein]);
+
+  useEffect(() => {
+    const subscription = startWebampFadein.subscribe(() => {
+      document.body.classList.add("webamp-loaded");
+    });
+    return () => {
+      document.body.classList.remove("webamp-loaded");
+      subscription.unsubscribe();
+    };
+  }, [startWebampFadein]);
+
+  // TODO: This might fire too frequently
+  useEffect(() => {
+    if (initialPosition != null) {
+      const subscription = timer(0).subscribe(() => {
+        // TODO: Observe DOM and recenter
+        setCentered(true);
+        transitionBeginEvents.next(null);
+      });
+      return () => subscription.unsubscribe();
+    } else {
+      transitionBeginEvents.next(null);
+    }
+  }, [initialPosition, setCentered, transitionBeginEvents]);
+
+  return {
+    centered,
+    loaded,
+    handleWebampLoaded: () => webampLoadedEvents.next(null),
+  };
+}