Update ScrollTo to wrap the list/menu items without injecting additional DOM elements

__tests__/src/components/ScrollTo.test.js

-import { render, screen } from 'test-utils';
+import { render } from 'test-utils';
 import { createRef } from 'react';
 import { ScrollTo } from '../../../src/components/ScrollTo';
 
@@ -22,12 +22,6 @@ describe('ScrollTo', () => {
   const scrollToElBelowBoundingRect = { bottom: 601, top: 501 };
   const visibleScrollToElBoundingRect = { bottom: 300, top: 200 };
 
-  it('wraps the given children in a div element', () => {
-    render(<ScrollTo data-testid="subject" scrollTo>Child Prop</ScrollTo>);
-
-    expect(screen.getByTestId('subject')).toHaveTextContent('Child Prop');
-  });
-
   describe('when updating the scrollTo prop', () => {
     beforeEach(() => {
       jest.spyOn(ScrollTo.prototype, 'elementToScrollTo').mockImplementation(() => ({ offsetTop: 450 }));
@@ -38,13 +32,13 @@ describe('ScrollTo', () => {
           ...scrollToElAboveBoundingRect,
         }));
 
-        const { rerender } = render(<ScrollTo scrollTo containerRef={containerRef}>Child</ScrollTo>);
+        const { rerender } = render(<ScrollTo scrollTo containerRef={containerRef}><div>Child</div></ScrollTo>);
 
         // It is called once when initially rendered w/ true
         expect(scrollTo).toHaveBeenCalled();
         scrollTo.mockReset();
 
-        rerender(<ScrollTo containerRef={containerRef}>Child</ScrollTo>);
+        rerender(<ScrollTo containerRef={containerRef}><div>Child</div></ScrollTo>);
 
         // But it is not called on the re-render w/ false
         expect(scrollTo).not.toHaveBeenCalled();
@@ -56,9 +50,9 @@ describe('ScrollTo', () => {
         jest.spyOn(ScrollTo.prototype, 'scrollToBoundingRect').mockImplementation(() => ({
           ...scrollToElAboveBoundingRect,
         }));
-        const { rerender } = render(<ScrollTo containerRef={containerRef}>Child</ScrollTo>);
+        const { rerender } = render(<ScrollTo containerRef={containerRef}><div>Child</div></ScrollTo>);
 
-        rerender(<ScrollTo scrollTo containerRef={containerRef}>Child</ScrollTo>);
+        rerender(<ScrollTo scrollTo containerRef={containerRef}><div>Child</div></ScrollTo>);
 
         expect(scrollTo).toHaveBeenCalledWith(0, 230);
       });
@@ -68,9 +62,9 @@ describe('ScrollTo', () => {
           ...scrollToElBelowBoundingRect,
         }));
 
-        const { rerender } = render(<ScrollTo containerRef={containerRef}>Child</ScrollTo>);
+        const { rerender } = render(<ScrollTo containerRef={containerRef}><div>Child</div></ScrollTo>);
 
-        rerender(<ScrollTo scrollTo containerRef={containerRef}>Child</ScrollTo>);
+        rerender(<ScrollTo scrollTo containerRef={containerRef}><div>Child</div></ScrollTo>);
 
         expect(scrollTo).toHaveBeenCalledWith(0, 230);
       });
@@ -80,9 +74,9 @@ describe('ScrollTo', () => {
           ...visibleScrollToElBoundingRect,
         }));
 
-        const { rerender } = render(<ScrollTo containerRef={containerRef}>Child</ScrollTo>);
+        const { rerender } = render(<ScrollTo containerRef={containerRef}><div>Child</div></ScrollTo>);
 
-        rerender(<ScrollTo scrollTo containerRef={containerRef}>Child</ScrollTo>);
+        rerender(<ScrollTo scrollTo containerRef={containerRef}><div>Child</div></ScrollTo>);
 
         expect(scrollTo).not.toHaveBeenCalled();
       });

src/components/CanvasAnnotations.js

@@ -70,6 +70,13 @@ export class CanvasAnnotations extends Component {
         </Typography>
         <MenuList autoFocusItem variant="selectedMenu">
           {annotations.map((annotation) => (
+            <ScrollTo
+              containerRef={containerRef}
+              key={`${annotation.id}-scroll`}
+              offsetTop={96} // offset for the height of the form above
+              scrollTo={selectedAnnotationId === annotation.id}
+              selected={selectedAnnotationId === annotation.id}
+            >
               <MenuItem
                 component={listContainerComponent}
                 variant="multiline"
@@ -82,18 +89,11 @@ export class CanvasAnnotations extends Component {
                 }}
                 key={annotation.id}
                 annotationid={annotation.id}
-              selected={selectedAnnotationId === annotation.id}
                 onClick={(e) => this.handleClick(e, annotation)}
                 onFocus={() => this.handleAnnotationHover(annotation)}
                 onBlur={this.handleAnnotationBlur}
                 onMouseEnter={() => this.handleAnnotationHover(annotation)}
                 onMouseLeave={this.handleAnnotationBlur}
-            >
-              <ScrollTo
-                containerRef={containerRef}
-                key={`${annotation.id}-scroll`}
-                offsetTop={96} // offset for the height of the form above
-                scrollTo={selectedAnnotationId === annotation.id}
               >
                 <ListItemText primaryTypographyProps={{ variant: 'body2' }}>
                   <SanitizedHtml ruleSet={htmlSanitizationRuleSet} htmlString={annotation.content} />
@@ -103,8 +103,8 @@ export class CanvasAnnotations extends Component {
                     ))}
                   </div>
                 </ListItemText>
-              </ScrollTo>
               </MenuItem>
+            </ScrollTo>
           ))}
         </MenuList>
       </>

src/components/ScrollTo.js

-import { createRef, Component } from 'react';
+import { cloneElement, createRef, Component } from 'react';
 import PropTypes from 'prop-types';
+import isEmpty from 'lodash/isEmpty';
 
 /**
  * ScrollTo ~
@@ -107,13 +108,9 @@ export class ScrollTo extends Component {
       children, containerRef, offsetTop, scrollTo, nodeId, ...otherProps
     } = this.props;
 
-    if (!scrollTo) return children;
+    if (!scrollTo && isEmpty(otherProps)) return children;
 
-    return (
-      <div ref={this.scrollToRef} {...otherProps}>
-        {children}
-      </div>
-    );
+    return cloneElement(children, { ref: this.scrollToRef, ...otherProps });
   }
 }
 

src/components/SidebarIndexList.js

@@ -48,26 +48,28 @@ export class SidebarIndexList extends Component {
             const onClick = () => { setCanvas(windowId, canvas.id); }; // eslint-disable-line require-jsdoc, max-len
 
             return (
+              <ScrollTo
+                containerRef={containerRef}
+                key={`${canvas.id}-${variant}`}
+                offsetTop={96} // offset for the height of the form above
+                selected={selectedCanvasIds.includes(canvas.id)}
+                scrollTo={selectedCanvasIds.includes(canvas.id)}
+              >
                 <MenuItem
                   key={canvas.id}
+                  disableGutters
                   sx={{
+                    paddingLeft: 2,
                     paddingRight: 1,
                     position: 'initial',
                   }}
                   divider
                   onClick={onClick}
                   component="li"
-                selected={selectedCanvasIds.includes(canvas.id)}
-              >
-                <ScrollTo
-                  containerRef={containerRef}
-                  key={`${canvas.id}-${variant}`}
-                  offsetTop={96} // offset for the height of the form above
-                  scrollTo={selectedCanvasIds.includes(canvas.id)}
                 >
                   <Item label={canvas.label} canvas={canvases[canvasIndex]} />
-                </ScrollTo>
                 </MenuItem>
+              </ScrollTo>
             );
           })
         }