Merge pull request #653 from samvera-labs/cleanup-593

Add comments, cleanup Videojs component options, use cx lib to build classnames

src/components/AutoAdvanceToggle/AutoAdvanceToggle.js

@@ -1,8 +1,15 @@
-import React from 'react';
+import React, { useMemo } from 'react';
 import PropTypes from 'prop-types';
 import { useManifestState, useManifestDispatch } from '../../context/manifest-context';
 import './AutoAdvanceToggle.scss';
 
+/**
+ * A toggle button to enable/disable auto-play across multiple
+ * canvases
+ * @param {Object} props
+ * @param {String} props.label
+ * @param {Boolean} props.showLabel
+ */
 const AutoAdvanceToggle = ({ label = "Autoplay", showLabel = true }) => {
   const { autoAdvance } = useManifestState();
   const manifestDispatch = useManifestDispatch();
@@ -11,7 +18,7 @@ const AutoAdvanceToggle = ({ label = "Autoplay", showLabel = true }) => {
     manifestDispatch({ autoAdvance: e.target.checked, type: "setAutoAdvance" });
   };
 
-  const toggleButton = React.useMemo(() => {
+  const toggleButton = useMemo(() => {
     return (<input
       data-testid="auto-advance-toggle"
       name="auto-advance-toggle"

src/components/IIIFPlayer/IIIFPlayer.js

@@ -6,6 +6,17 @@ import ErrorMessage from '@Components/ErrorMessage/ErrorMessage';
 import PropTypes from 'prop-types';
 import '../../styles/main.scss';
 
+/**
+ * Component with wrapped in React Contexts to provide access
+ * to global state across its children
+ * @param {Object} props
+ * @param {String} props.manifestUrl
+ * @param {Object} props.manifest
+ * @param {String} props.customErrorMessage
+ * @param {String} props.emptyManifestMessage
+ * @param {String} props.startCanvasId
+ * @param {String} props.startCanvasTime 
+ */
 export default function IIIFPlayer({
   manifestUrl,
   manifest,

src/components/IIIFPlayerWrapper.js

@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useEffect, useState } from 'react';
 import { useManifestDispatch } from '../context/manifest-context';
 import { usePlayerDispatch } from '../context/player-context';
 import PropTypes from 'prop-types';
@@ -20,7 +20,7 @@ export default function IIIFPlayerWrapper({
   children,
   manifest: manifestValue,
 }) {
-  const [manifest, setManifest] = React.useState(manifestValue);
+  const [manifest, setManifest] = useState(manifestValue);
   const manifestDispatch = useManifestDispatch();
   const playerDispatch = usePlayerDispatch();
 
@@ -32,7 +32,7 @@ export default function IIIFPlayerWrapper({
   const fetchManifest = async (url) => {
     controller = new AbortController();
     let requestOptions = {
-      // NOTE: try thin in Avalon
+      // NOTE: try this in Avalon
       //credentials: 'include',
       // headers: { 'Avalon-Api-Key': '' },
     };
@@ -65,7 +65,7 @@ export default function IIIFPlayerWrapper({
     }
   };
 
-  React.useEffect(() => {
+  useEffect(() => {
     setAppErrorMessage(customErrorMessage);
     setAppEmptyManifestMessage(emptyManifestMessage);
 
@@ -79,7 +79,7 @@ export default function IIIFPlayerWrapper({
     };
   }, []);
 
-  React.useEffect(() => {
+  useEffect(() => {
     if (manifest) {
       // Set customStart and rendering files in state before setting Manifest
       const renderingFiles = getRenderingFiles(manifest);
@@ -100,7 +100,7 @@ export default function IIIFPlayerWrapper({
   if (!manifest) {
     return <Spinner />;
   } else {
-    return <React.Fragment>{children}</React.Fragment>;
+    return <>{children}</>;
   }
 }
 

src/components/MarkersDisplay/MarkerUtils/CreateMarker.js

@@ -1,21 +1,31 @@
-import React from 'react';
+import React, { useCallback, useEffect, useState } from 'react';
 import PropTypes from 'prop-types';
+import cx from 'classnames';
 import { parseMarkerAnnotation } from '@Services/playlist-parser';
 import { validateTimeInput, timeToS, timeToHHmmss } from '@Services/utility-helpers';
 import { SaveIcon, CancelIcon } from '@Services/svg-icons';
 import { useMediaPlayer } from '@Services/ramp-hooks';
 
+/**
+ * Build and handle creation of new markers for playlists. This component is rendered
+ * on page when the user has permissions to create new markers in a given playlist Manifest.
+ * @param {Object} props
+ * @param {String} props.newMarkerEndpoint annotationService to POST create markers request
+ * @param {Number} props.canvasId URI of the current Canvas
+ * @param {Function} props.handleCreate callback function to update global state
+ * @param {String} props.csrfToken token to authenticate POST request
+ */
 const CreateMarker = ({ newMarkerEndpoint, canvasId, handleCreate, csrfToken }) => {
-  const [isOpen, setIsOpen] = React.useState(false);
-  const [isValid, setIsValid] = React.useState(false);
-  const [saveError, setSaveError] = React.useState(false);
-  const [errorMessage, setErrorMessage] = React.useState('');
-  const [markerTime, setMarkerTime] = React.useState();
+  const [isOpen, setIsOpen] = useState(false);
+  const [isValid, setIsValid] = useState(false);
+  const [saveError, setSaveError] = useState(false);
+  const [errorMessage, setErrorMessage] = useState('');
+  const [markerTime, setMarkerTime] = useState();
   let controller;
 
   const { getCurrentTime } = useMediaPlayer();
 
-  React.useEffect(() => {
+  useEffect(() => {
     // Close new marker form on Canvas change
     setIsOpen(false);
 
@@ -31,7 +41,7 @@ const CreateMarker = ({ newMarkerEndpoint, canvasId, handleCreate, csrfToken })
     setIsOpen(true);
   };
 
-  const handleCreateSubmit = React.useCallback((e) => {
+  const handleCreateSubmit = useCallback((e) => {
     e.preventDefault();
     const form = e.target;
     const formData = new FormData(form);
@@ -80,7 +90,7 @@ const CreateMarker = ({ newMarkerEndpoint, canvasId, handleCreate, csrfToken })
       });
   }, [canvasId]);
 
-  const handleCreateCancel = React.useCallback(() => {
+  const handleCreateCancel = useCallback(() => {
     setIsOpen(false);
     setIsValid(false);
     setErrorMessage('');
@@ -127,7 +137,10 @@ const CreateMarker = ({ newMarkerEndpoint, canvasId, handleCreate, csrfToken })
                     id="new-marker-time"
                     data-testid="create-marker-timestamp"
                     type="text"
-                    className={`ramp--markers-display__create-marker ${isValid ? 'time-valid' : 'time-invalid'}`}
+                    className={cx(
+                      'ramp--markers-display__create-marker',
+                      isValid ? 'time-valid' : 'time-invalid'
+                    )}
                     name="time"
                     value={markerTime}
                     onChange={validateTime} />
@@ -171,6 +184,7 @@ CreateMarker.propTypes = {
   newMarkerEndpoint: PropTypes.string.isRequired,
   canvasId: PropTypes.string,
   handleCreate: PropTypes.func.isRequired,
+  csrfToken: PropTypes.string
 };
 
 export default CreateMarker;

src/components/MarkersDisplay/MarkerUtils/MarkerRow.js

@@ -1,47 +1,58 @@
-import React from 'react';
+import React, { useCallback, useEffect, useRef, useState } from 'react';
 import PropTypes from 'prop-types';
+import cx from 'classnames';
 import { CancelIcon, EditIcon, DeleteIcon, SaveIcon } from '@Services/svg-icons';
 import { validateTimeInput, timeToS } from '@Services/utility-helpers';
 import { useMarkers, useMediaPlayer } from '@Services/ramp-hooks';
 
+/**
+ * Build a table row for each 'highlighting; annotation in the current Canvas in the Manifest.
+ * These are timepoint annotations. When user has permissions to edit annotations, an actions
+ * column is populated for each annotation with edit and delete actions.
+ * @param {Object} props
+ * @param {Object} props.marker each marker parsed from annotations
+ * @param {Function} props.handleSubmit callback func to update state on marker edit action
+ * @param {Function} props.handleDelete callback func to update state on marker delete action
+ * @param {Function} props.toggleIsEditing callback function to update global state
+ * @param {String} props.csrfToken token to authenticate POST request
+ */
 const MarkerRow = ({
   marker,
   handleSubmit,
   handleDelete,
-  hasAnnotationService,
   toggleIsEditing,
   csrfToken
 }) => {
-  const [editing, setEditing] = React.useState(false);
-  const [isValid, setIsValid] = React.useState(true);
-  const [tempMarker, setTempMarker] = React.useState();
-  const [deleting, setDeleting] = React.useState(false);
-  const [saveError, setSaveError] = React.useState(false);
-  const [errorMessage, setErrorMessage] = React.useState('');
+  const [editing, setEditing] = useState(false);
+  const [isValid, setIsValid] = useState(true);
+  const [tempMarker, setTempMarker] = useState();
+  const [deleting, setDeleting] = useState(false);
+  const [saveError, setSaveError] = useState(false);
+  const [errorMessage, setErrorMessage] = useState('');
   let controller;
 
-  const { isDisabled } = useMarkers();
+  const { hasAnnotationService, isDisabled } = useMarkers();
   const { player } = useMediaPlayer();
 
   // Remove all fetch requests on unmount
-  React.useEffect(() => {
+  useEffect(() => {
     return () => {
       controller?.abort();
     };
   }, []);
 
-  React.useEffect(() => {
+  useEffect(() => {
     setMarkerLabel(marker.value);
     setMarkerTime(marker.timeStr);
   }, [marker]);
 
-  let markerLabelRef = React.useRef(marker.value);
+  let markerLabelRef = useRef(marker.value);
   const setMarkerLabel = (label) => {
     markerLabelRef.current = label;
   };
 
-  let markerOffsetRef = React.useRef(timeToS(marker.timeStr));
-  let markerTimeRef = React.useRef(marker.timeStr);
+  let markerOffsetRef = useRef(timeToS(marker.timeStr));
+  let markerTimeRef = useRef(marker.timeStr);
   const setMarkerTime = (time) => {
     markerTimeRef.current = time;
     markerOffsetRef.current = timeToS(time);
@@ -164,7 +175,7 @@ const MarkerRow = ({
     toggleIsEditing(false);
   };
 
-  const handleMarkerClick = React.useCallback((e) => {
+  const handleMarkerClick = useCallback((e) => {
     e.preventDefault();
     const currentTime = parseFloat(e.target.dataset['offset']);
     if (player) {
@@ -187,7 +198,10 @@ const MarkerRow = ({
         </td>
         <td>
           <input
-            className={`ramp--markers-display__edit-marker ${isValid ? 'time-valid' : 'time-invalid'}`}
+            className={cx(
+              'ramp--markers-display__edit-marker',
+              isValid ? 'time-valid' : 'time-invalid'
+            )}
             id="time"
             data-testid="edit-timestamp"
             defaultValue={markerTimeRef.current}
@@ -303,8 +317,8 @@ MarkerRow.propTypes = {
   marker: PropTypes.object.isRequired,
   handleSubmit: PropTypes.func.isRequired,
   handleDelete: PropTypes.func.isRequired,
-  hasAnnotationService: PropTypes.bool.isRequired,
   toggleIsEditing: PropTypes.func.isRequired,
+  csrfToken: PropTypes.string
 };
 
 export default MarkerRow;

src/components/MarkersDisplay/MarkersDisplay.js

@@ -7,18 +7,23 @@ import MarkerRow from './MarkerUtils/MarkerRow';
 import { useErrorBoundary } from "react-error-boundary";
 import './MarkersDisplay.scss';
 
+/**
+ * Display timepoint annotations associated with the current Canvas
+ * in a tabular format.
+ * @param {Object} props
+ * @param {Boolean} props.showHeading
+ * @param {String} props.headingText
+ */
 const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
   const { allCanvases, canvasIndex, playlist } = useManifestState();
   const manifestDispatch = useManifestDispatch();
 
   const { hasAnnotationService, annotationServiceId, markers } = playlist;
-
-  const [canvasPlaylistsMarkers, setCanvasPlaylistsMarkers] = useState([]);
-
+  const [_, setCanvasPlaylistsMarkers] = useState([]);
   const { showBoundary } = useErrorBoundary();
-
   const canvasIdRef = useRef();
 
+  // Using a ref updates markers table immediately after marker edit/creation
   let canvasPlaylistsMarkersRef = useRef([]);
   const setCanvasMarkers = (list) => {
     setCanvasPlaylistsMarkers(...list);
@@ -107,7 +112,6 @@ const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
                 marker={marker}
                 handleSubmit={handleSubmit}
                 handleDelete={handleDelete}
-                hasAnnotationService={hasAnnotationService}
                 toggleIsEditing={toggleIsEditing}
                 csrfToken={csrfToken} />
             ))}

src/components/MediaPlayer/MediaPlayer.js

@@ -13,6 +13,17 @@ import en from 'video.js/dist/lang/en.json';
 
 const PLAYER_ID = "iiif-media-player";
 
+/**
+ * Parse resource related information form the current canvas in manifest,
+ * and build an options object for Video.js using that information.
+ * @param {Object} props
+ * @param {Boolean} props.enableFileDownload
+ * @param {Boolean} props.enablePIP
+ * @param {Boolean} props.enablePlaybackRate
+ * @param {Boolean} props.enableTitleLink
+ * @param {Boolean} props.withCredentials
+ * @param {String} props.language
+ */
 const MediaPlayer = ({
   enableFileDownload = false,
   enablePIP = false,
@@ -160,9 +171,9 @@ const MediaPlayer = ({
             files: renderingFiles,
           },
           videoJSPreviousButton: isMultiCanvased &&
-            { canvasIndex, switchPlayer, playerFocusElement },
+            { canvasIndex, switchPlayer },
           videoJSNextButton: isMultiCanvased &&
-            { canvasIndex, lastCanvasIndex, switchPlayer, playerFocusElement },
+            { canvasIndex, lastCanvasIndex, switchPlayer },
           videoJSTrackScrubber: (hasStructure || isPlaylist) &&
             { trackScrubberRef, timeToolRef, isPlaylist }
         },

src/components/MediaPlayer/VideoJS/VideoJSPlayer.js

@@ -1,5 +1,6 @@
 import React, { useMemo, useRef } from 'react';
 import PropTypes from 'prop-types';
+import cx from 'classnames';
 import videojs from 'video.js';
 import throttle from 'lodash/throttle';
 import 'videojs-markers-plugin/dist/videojs-markers-plugin';
@@ -33,6 +34,24 @@ import VideoJSTitleLink from './components/js/VideoJSTitleLink';
 import VideoJSTrackScrubber from './components/js/VideoJSTrackScrubber';
 // import vjsYo from './vjsYo';
 
+/**
+ * Module to setup VideoJS instance on initial page load and update
+ * on successive player reloads on Canvas changes.
+ * @param {Object} props
+ * @param {Boolean} props.isVideo
+ * @param {Boolean} props.isPlaylist
+ * @param {Object} props.trackScrubberRef
+ * @param {Object} props.scrubberTooltipRef
+ * @param {Array} props.tracks
+ * @param {String} props.placeholderText
+ * @param {Array} props.renderingFiles
+ * @param {Boolean} props.enableFileDownload
+ * @param {Function} props.loadPrevOrNext
+ * @param {Number} props.lastCanvasIndex
+ * @param {Boolean} props.enableTitleLink
+ * @param {String} props.videoJSLangMap
+ * @param {Object} props.options
+ */
 function VideoJSPlayer({
   enableFileDownload,
   enableTitleLink,
@@ -776,7 +795,10 @@ function VideoJSPlayer({
             </div>
             {canvasIndex != lastCanvasIndex &&
               <p data-testid="inaccessible-message-timer"
-                className={`ramp--media-player_inaccessible-message-timer ${autoAdvanceRef.current ? '' : 'hidden'}`}>
+                className={cx(
+                  'ramp--media-player_inaccessible-message-timer',
+                  autoAdvanceRef.current ? '' : 'hidden'
+                )}>
                 {`Next item in ${messageTime} second${messageTime === 1 ? '' : 's'}`}
               </p>}
           </div>
@@ -785,7 +807,10 @@ function VideoJSPlayer({
           data-testid={`videojs-${isVideo ? 'video' : 'audio'}-element`}
           data-canvasindex={cIndexRef.current}
           ref={videoJSRef}
-          className={`video-js vjs-big-play-centered vjs-theme-ramp vjs-disabled ${IS_ANDROID ? 'is-mobile' : ''}`}
+          className={cx(
+            'video-js vjs-big-play-centered vjs-theme-ramp vjs-disabled',
+            IS_ANDROID ? 'is-mobile' : ''
+          )}
           onTouchStart={saveTouchStartCoords}
           onTouchEnd={mobilePlayToggle}
           style={{ display: `${canvasIsEmptyRef.current ? 'none' : ''}` }}