Merge pull request #333 from samvera-labs/remove-no-markers-msg

Remove no markers message in MarkersDisplay component
samvera-labs · Jan 10, 2024 · ed89c9e · ed89c9e
2 parents f145b8f + 83945a1
commit ed89c9e
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 38 deletions.
diff --git a/src/components/MarkersDisplay/MarkersDisplay.js b/src/components/MarkersDisplay/MarkersDisplay.js
@@ -17,15 +17,16 @@ const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
 
   const { isEditing, hasAnnotationService, annotationServiceId } = playlist;
 
-  const [errorMsg, setErrorMsg] = React.useState();
+  const [canvasPlaylistsMarkers, setCanvasPlaylistsMarkers] = React.useState([]);
 
   const { showBoundary } = useErrorBoundary();
 
   const canvasIdRef = React.useRef();
 
-  let playlistMarkersRef = React.useRef([]);
-  const setPlaylistMarkers = (list) => {
-    playlistMarkersRef.current = list;
+  let canvasPlaylistsMarkersRef = React.useRef([]);
+  const setCanvasMarkers = (list) => {
+    setCanvasPlaylistsMarkers(...list);
+    canvasPlaylistsMarkersRef.current = list;
   };
 
   // Retrieves the CRSF authenticity token when component is embedded in a Rails app.
@@ -48,9 +49,8 @@ const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
 
   React.useEffect(() => {
     if (playlist.markers?.length > 0) {
-      let { canvasMarkers, error } = playlist.markers.filter((m) => m.canvasIndex === canvasIndex)[0];
-      setPlaylistMarkers(canvasMarkers);
-      setErrorMsg(error);
+      let { canvasMarkers } = playlist.markers.filter((m) => m.canvasIndex === canvasIndex)[0];
+      setCanvasMarkers(canvasMarkers);
     }
 
     if (manifest) {
@@ -67,22 +67,22 @@ const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
 
   const handleSubmit = (label, time, id) => {
     // Re-construct markers list for displaying in the player UI
-    let editedMarkers = playlistMarkersRef.current.map(m => {
+    let editedMarkers = canvasPlaylistsMarkersRef.current.map(m => {
       if (m.id === id) {
         m.value = label;
         m.timeStr = time;
         m.time = timeToS(time);
       }
       return m;
     });
-    setPlaylistMarkers(editedMarkers);
+    setCanvasMarkers(editedMarkers);
     manifestDispatch({ updatedMarkers: editedMarkers, type: 'setPlaylistMarkers' });
   };
 
   const handleDelete = (id) => {
-    let remainingMarkers = playlistMarkersRef.current.filter(m => m.id != id);
+    let remainingMarkers = canvasPlaylistsMarkersRef.current.filter(m => m.id != id);
     // Update markers in state for displaying in the player UI
-    setPlaylistMarkers(remainingMarkers);
+    setCanvasMarkers(remainingMarkers);
     manifestDispatch({ updatedMarkers: remainingMarkers, type: 'setPlaylistMarkers' });
   };
 
@@ -93,8 +93,8 @@ const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
   };
 
   const handleCreate = (newMarker) => {
-    setPlaylistMarkers([...playlistMarkersRef.current, newMarker]);
-    manifestDispatch({ updatedMarkers: playlistMarkersRef.current, type: 'setPlaylistMarkers' });
+    setCanvasMarkers([...canvasPlaylistsMarkersRef.current, newMarker]);
+    manifestDispatch({ updatedMarkers: canvasPlaylistsMarkersRef.current, type: 'setPlaylistMarkers' });
   };
 
   const toggleIsEditing = (flag) => {
@@ -130,7 +130,7 @@ const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
           csrfToken={csrfToken}
         />
       )}
-      {playlistMarkersRef.current.length > 0 && (
+      {canvasPlaylistsMarkersRef.current.length > 0 && (
         <table className="ramp--markers-display_table" data-testid="markers-display-table">
           <thead>
             <tr>
@@ -140,7 +140,7 @@ const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
             </tr>
           </thead>
           <tbody>
-            {playlistMarkersRef.current.map((marker, index) => (
+            {canvasPlaylistsMarkersRef.current.map((marker, index) => (
               <MarkerRow
                 key={index}
                 marker={marker}
@@ -155,14 +155,6 @@ const MarkersDisplay = ({ showHeading = true, headingText = 'Markers' }) => {
           </tbody>
         </table>
       )}
-      {playlistMarkersRef.current.length == 0 && (
-        <div
-          className="ramp--markers-display__markers-empty"
-          data-testid="markers-empty"
-        >
-          <p>{errorMsg}</p>
-        </div>
-      )}
     </div>
   );
 };

diff --git a/src/components/MarkersDisplay/MarkersDisplay.test.js b/src/components/MarkersDisplay/MarkersDisplay.test.js
@@ -31,7 +31,6 @@ describe('MarkersDisplay component', () => {
     test('renders successfully', () => {
       expect(screen.queryByTestId('markers-display')).toBeInTheDocument();
       expect(screen.queryByTestId('markers-display-table')).toBeInTheDocument();
-      expect(screen.queryByTestId('markers-empty')).not.toBeInTheDocument();
     });
 
     test('renders all marker information properly', () => {
@@ -161,7 +160,7 @@ describe('MarkersDisplay component', () => {
       });
 
       test('user actions have csrf token in header when it is present in DOM', () => {
-        document.head.innerHTML = '<meta name="csrf-token" content="csrftoken">'
+        document.head.innerHTML = '<meta name="csrf-token" content="csrftoken">';
         const deleteFetchSpy = jest.spyOn(global, 'fetch').mockResolvedValueOnce({
           status: 200,
         });
@@ -200,10 +199,6 @@ describe('MarkersDisplay component', () => {
       );
       expect(screen.queryByTestId('markers-display')).toBeInTheDocument();
       expect(screen.queryByTestId('markers-display-table')).not.toBeInTheDocument();
-      expect(screen.queryByTestId('markers-empty')).toBeInTheDocument();
-      waitFor(() => {
-        expect(screen.queryByText('No markers were found in the Canvas')).toBeInTheDocument();
-      });
     });
   });
 });
diff --git a/src/services/playlist-parser.js b/src/services/playlist-parser.js
@@ -40,8 +40,7 @@ export function getIsPlaylist(manifest) {
  *      timeStr: String,
  *      canvasId: String,
  *      value: String
- *    }],
- *    error: String,
+ *    }]
  * }]
  *
  */
@@ -55,7 +54,7 @@ export function parsePlaylistAnnotations(manifest) {
       canvases.map((canvas, index) => {
         let annotations = parseAnnotations(canvas.__jsonld['annotations'], 'highlighting');
         if (!annotations || annotations.length === 0) {
-          allMarkers.push({ canvasMarkers: [], canvasIndex: index, error: 'No markers were found in the Canvas' });
+          allMarkers.push({ canvasMarkers: [], canvasIndex: index });
         } else if (annotations.length > 0) {
           let canvasMarkers = [];
           annotations.map((a) => {
@@ -64,7 +63,7 @@ export function parsePlaylistAnnotations(manifest) {
               canvasMarkers.push(marker);
             }
           });
-          allMarkers.push({ canvasMarkers, canvasIndex: index, error: '' });
+          allMarkers.push({ canvasMarkers, canvasIndex: index });
         }
       });
     }

diff --git a/src/services/playlist-parser.test.js b/src/services/playlist-parser.test.js
@@ -50,19 +50,14 @@ describe('playlist-parser', () => {
       const canvases = playlistParser.parsePlaylistAnnotations(manifest);
       expect(canvases[0].canvasMarkers).toHaveLength(0);
       expect(canvases[1].canvasMarkers).toHaveLength(0);
-      expect(canvases[0].error).toEqual('No markers were found in the Canvas');
-      expect(canvases[1].error).toEqual('No markers were found in the Canvas');
     });
 
     it('returns markers information for a canvas with markers', () => {
       const canvases = playlistParser.parsePlaylistAnnotations(playlistManifest);
 
       expect(canvases[0].canvasMarkers).toHaveLength(0);
-      expect(canvases[0].error).toEqual('No markers were found in the Canvas');
       expect(canvases[1].canvasMarkers).toHaveLength(2);
-      expect(canvases[1].error).toEqual('');
       expect(canvases[2].canvasMarkers).toHaveLength(2);
-      expect(canvases[2].error).toEqual('');
 
       expect(canvases[1].canvasMarkers[0]).toEqual({
         time: 2.836,