Remove refs from Browser component (#149)

- remove lastGenomeIdRef entirely
- move browserRef from Browser component to BrowserImage
- move code related to reporting track states to BrowserCogList,
   and use postMessage for communication with GenomeBrowser

src/ensembl/src/content/app/browser/Browser.tsx

-import React, { FunctionComponent, useRef, useEffect, useState } from 'react';
+import React, { FunctionComponent, useEffect, useState } from 'react';
 import { withRouter, RouteComponentProps } from 'react-router-dom';
 import { connect } from 'react-redux';
 import { replace, Replace } from 'connected-react-router';
@@ -102,11 +102,9 @@ type BrowserProps = RouteComponentProps<MatchParams> &
 export const Browser: FunctionComponent<BrowserProps> = (
   props: BrowserProps
 ) => {
-  const browserRef: React.RefObject<HTMLDivElement> = useRef(null);
   const [trackStatesFromStorage, setTrackStatesFromStorage] = useState<
     TrackStates
   >({});
-  const lastGenomeIdRef = useRef(props.activeGenomeId);
 
   const { isDrawerOpened, closeDrawer } = props;
 
@@ -115,25 +113,9 @@ export const Browser: FunctionComponent<BrowserProps> = (
     const { focus = null, location = null } = props.browserQueryParams;
     const chrLocation = location ? getChrLocationFromStr(location) : null;
 
-    const lastGenomeId = lastGenomeIdRef.current;
-
-    /*
-      before committing url changes to redux:
-      - make sure we don't already have these same values in redux store;
-      - if we already have these values, it's possible that this is because
-        the user is switching back to a previously viewed species;
-        so check whether the genome id has changed from the previous render
-        (that's the reason for lastGenomeIdRef here)
-
-      TODO: after both genome browser and browser chrome are updated so that
-      we do not update url location while moving or zooming the image; we can
-      remove the genomeId === lastGenomeId check in the if-statement below
-      and move dispatchBrowserLocation(genomeId, chrLocation) above the if-statement
-    */
     if (
       !genomeId ||
-      (genomeId === lastGenomeId &&
-        genomeId === props.activeGenomeId &&
+      (genomeId === props.activeGenomeId &&
         focus === props.activeEnsObjectId &&
         isEqual(chrLocation, props.chrLocation))
     ) {
@@ -153,7 +135,6 @@ export const Browser: FunctionComponent<BrowserProps> = (
     } else if (focus) {
       props.changeFocusObject(focus);
     }
-    lastGenomeIdRef.current = genomeId;
   };
 
   const dispatchBrowserLocation = (
@@ -263,10 +244,13 @@ export const Browser: FunctionComponent<BrowserProps> = (
     return launchbarExpanded ? styles.shorter : styles.taller;
   };
 
-  const BrowserBarNode = (
+  const browserBar = (
     <BrowserBar dispatchBrowserLocation={dispatchBrowserLocation} />
   );
 
+  const shouldShowNavBar =
+    props.browserActivated && props.browserNavOpened && !isDrawerOpened;
+
   return props.activeGenomeId ? (
     <>
       <AppBar
@@ -277,13 +261,13 @@ export const Browser: FunctionComponent<BrowserProps> = (
 
       {!props.browserQueryParams.focus && (
         <section className={styles.browser}>
-          {BrowserBarNode}
+          {browserBar}
           <ExampleObjectLinks {...props} />
         </section>
       )}
       {props.browserQueryParams.focus && (
         <section className={styles.browser}>
-          {BrowserBarNode}
+          {browserBar}
           {props.genomeSelectorActive && (
             <div className={styles.browserOverlay} />
           )}
@@ -294,15 +278,8 @@ export const Browser: FunctionComponent<BrowserProps> = (
           >
             <animated.div style={trackAnimation}>
               <div className={styles.browserImageWrapper} onClick={closeTrack}>
-                {props.browserNavOpened &&
-                !isDrawerOpened &&
-                browserRef.current ? (
-                  <BrowserNavBar />
-                ) : null}
-                <BrowserImage
-                  browserRef={browserRef}
-                  trackStates={trackStatesFromStorage}
-                />
+                {shouldShowNavBar && <BrowserNavBar />}
+                <BrowserImage trackStates={trackStatesFromStorage} />
               </div>
             </animated.div>
             <TrackPanel />

src/ensembl/src/content/app/browser/BrowserCogList.tsx

@@ -14,6 +14,8 @@ import {
   getBrowserActivated,
   getBrowserCogList,
   getBrowserCogTrackList,
+  getTrackConfigNames,
+  getTrackConfigLabel,
   getBrowserSelectedCog
 } from './browserSelectors';
 
@@ -23,6 +25,8 @@ type BrowserCogListProps = {
   browserActivated: boolean;
   browserCogList: number;
   browserCogTrackList: CogList;
+  trackConfigNames: { [key: string]: boolean };
+  trackConfigLabel: { [key: string]: boolean };
   selectedCog: any;
   updateCogList: (cogList: number) => void;
   updateCogTrackList: (track_y: CogList) => void;
@@ -55,6 +59,39 @@ const BrowserCogList = (props: BrowserCogListProps) => {
     return () => subscription.unsubscribe();
   }, []);
 
+  useEffect(() => {
+    if (props.browserCogTrackList) {
+      const ons: string[] = [];
+      const offs: string[] = [];
+
+      Object.keys(props.browserCogTrackList).forEach((name) => {
+        // TODO: notice how we generate strings with suffix ":label" for track names,
+        // and strings with suffix ":names" for track label? That's because the frontend code
+        // and the backend code refer to these things by opposite terms. We will need to unify
+        // the terminology at some point.
+        if (props.trackConfigNames[name]) {
+          ons.push(`${name}:label`);
+        } else {
+          offs.push(`${name}:label`); // by default, track names are not shown
+        }
+
+        if (props.trackConfigLabel[name] !== false) {
+          ons.push(`${name}:names`);
+        } else {
+          offs.push(`${name}:names`); // by default, track label is not shown
+        }
+      });
+      browserMessagingService.send('bpane', {
+        off: offs,
+        on: ons
+      });
+    }
+  }, [
+    props.trackConfigNames,
+    props.trackConfigLabel,
+    props.browserCogTrackList
+  ]);
+
   const cogs = Object.entries(browserCogTrackList).map(([name, pos]) => {
     const posStyle = { top: pos + 'px' };
 
@@ -88,6 +125,8 @@ const mapStateToProps = (state: RootState) => ({
   browserActivated: getBrowserActivated(state),
   browserCogList: getBrowserCogList(state),
   browserCogTrackList: getBrowserCogTrackList(state),
+  trackConfigLabel: getTrackConfigLabel(state),
+  trackConfigNames: getTrackConfigNames(state),
   selectedCog: getBrowserSelectedCog(state)
 });
 

src/ensembl/src/content/app/browser/browser-image/BrowserImage.tsx

 import React, {
   FunctionComponent,
-  RefObject,
+  useRef,
   useEffect,
   useCallback
 } from 'react';
@@ -13,8 +13,6 @@ import { ZmenuController } from 'src/content/app/browser/zmenu';
 
 import browserMessagingService from 'src/content/app/browser/browser-messaging-service';
 import {
-  getTrackConfigNames,
-  getTrackConfigLabel,
   getBrowserCogTrackList,
   getBrowserNavOpened,
   getBrowserActivated
@@ -42,8 +40,6 @@ import { BROWSER_CONTAINER_ID } from '../browser-constants';
 type StateProps = {
   browserCogTrackList: CogList;
   browserNavOpened: boolean;
-  trackConfigNames: any;
-  trackConfigLabel: any;
   browserActivated: boolean;
 };
 
@@ -59,7 +55,6 @@ type DispatchProps = {
 };
 
 type OwnProps = {
-  browserRef: RefObject<HTMLDivElement>;
   trackStates: TrackStates;
 };
 
@@ -83,6 +78,7 @@ const parseLocation = (location: ChrLocation) => {
 export const BrowserImage: FunctionComponent<BrowserImageProps> = (
   props: BrowserImageProps
 ) => {
+  const browserRef = useRef<HTMLDivElement>(null);
   const listenBpaneOut = useCallback((payload: BpaneOutPayload) => {
     const ensObjectId = payload.focus;
     const navIconStates = payload.bumper as BrowserNavStates;
@@ -123,54 +119,12 @@ export const BrowserImage: FunctionComponent<BrowserImageProps> = (
   }, []);
 
   useEffect(() => {
-    const currentEl: HTMLDivElement = props.browserRef
-      .current as HTMLDivElement;
     props.activateBrowser();
 
     return function cleanup() {
-      if (currentEl && currentEl.ownerDocument) {
-        props.updateBrowserActivated(false);
-      }
+      props.updateBrowserActivated(false);
     };
-  }, [props.browserRef]);
-
-  useEffect(() => {
-    if (props.browserCogTrackList) {
-      const ons: string[] = [];
-      const offs: string[] = [];
-
-      /* what the frontend and backend call labels and names is flipped */
-      Object.keys(props.browserCogTrackList).map((name) => {
-        /* undefined means not seen means on for names */
-        if (props.trackConfigNames[name]) {
-          ons.push(name + ':label');
-        } else {
-          offs.push(name + ':label');
-        }
-        /* undefined means not seen means off for labels */
-        if (props.trackConfigLabel[name] !== false) {
-          ons.push(name + ':names');
-        } else {
-          offs.push(name + ':names');
-        }
-      });
-      const stateEvent = new CustomEvent('bpane', {
-        bubbles: true,
-        detail: {
-          off: offs,
-          on: ons
-        }
-      });
-      if (props.browserRef.current) {
-        props.browserRef.current.dispatchEvent(stateEvent);
-      }
-    }
-  }, [
-    props.trackConfigNames,
-    props.trackConfigLabel,
-    props.browserRef,
-    props.browserCogTrackList
-  ]);
+  }, []);
 
   return (
     <>
@@ -183,10 +137,10 @@ export const BrowserImage: FunctionComponent<BrowserImageProps> = (
         <div
           id={BROWSER_CONTAINER_ID}
           className={getBrowserImageClasses(props.browserNavOpened)}
-          ref={props.browserRef}
+          ref={browserRef}
         />
         <BrowserCogList />
-        <ZmenuController browserRef={props.browserRef} />
+        <ZmenuController browserRef={browserRef} />
       </div>
     </>
   );
@@ -205,8 +159,6 @@ function getBrowserImageClasses(browserNavOpened: boolean): string {
 const mapStateToProps = (state: RootState): StateProps => ({
   browserCogTrackList: getBrowserCogTrackList(state),
   browserNavOpened: getBrowserNavOpened(state),
-  trackConfigLabel: getTrackConfigLabel(state),
-  trackConfigNames: getTrackConfigNames(state),
   browserActivated: getBrowserActivated(state)
 });