diff --git a/__tests__/src/components/ScrollTo.test.js b/__tests__/src/components/ScrollTo.test.js index 9e4ac8234ea4a096672b8c72cbc0b8787273906d..fd4f34f087645e865eb56725a97705c40b80f7ca 100644 --- a/__tests__/src/components/ScrollTo.test.js +++ b/__tests__/src/components/ScrollTo.test.js @@ -1,4 +1,4 @@ -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(); }); diff --git a/src/components/CanvasAnnotations.js b/src/components/CanvasAnnotations.js index 307b8b19f5bf7b4df46113bab8202373d4ca2751..ba5f5cb2689892ea4bef9e0d669dfe766a6c4671 100644 --- a/src/components/CanvasAnnotations.js +++ b/src/components/CanvasAnnotations.js @@ -70,30 +70,30 @@ export class CanvasAnnotations extends Component { </Typography> <MenuList autoFocusItem variant="selectedMenu"> {annotations.map((annotation) => ( - <MenuItem - component={listContainerComponent} - variant="multiline" - divider - sx={{ - '&:hover,&:focus': { - backgroundColor: 'action.hover', - }, - backgroundColor: hoveredAnnotationIds.includes(annotation.id) ? 'action.hover' : '', - }} - key={annotation.id} - annotationid={annotation.id} + <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} - 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} + <MenuItem + component={listContainerComponent} + variant="multiline" + divider + sx={{ + '&:hover,&:focus': { + backgroundColor: 'action.hover', + }, + backgroundColor: hoveredAnnotationIds.includes(annotation.id) ? 'action.hover' : '', + }} + key={annotation.id} + annotationid={annotation.id} + onClick={(e) => this.handleClick(e, annotation)} + onFocus={() => this.handleAnnotationHover(annotation)} + onBlur={this.handleAnnotationBlur} + onMouseEnter={() => this.handleAnnotationHover(annotation)} + onMouseLeave={this.handleAnnotationBlur} > <ListItemText primaryTypographyProps={{ variant: 'body2' }}> <SanitizedHtml ruleSet={htmlSanitizationRuleSet} htmlString={annotation.content} /> @@ -103,8 +103,8 @@ export class CanvasAnnotations extends Component { ))} </div> </ListItemText> - </ScrollTo> - </MenuItem> + </MenuItem> + </ScrollTo> ))} </MenuList> </> diff --git a/src/components/ScrollTo.js b/src/components/ScrollTo.js index c4aed0adb7fb84bfeba51494c2e5ba635d138fb8..4e11892d7267729ba6619f090c407910785d3612 100644 --- a/src/components/ScrollTo.js +++ b/src/components/ScrollTo.js @@ -1,5 +1,6 @@ -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 }); } } diff --git a/src/components/SidebarIndexList.js b/src/components/SidebarIndexList.js index 9d6b80f50ca0cfe002abaa4a0ded679d3cc69890..babb1838417b15b37b7f1521824018714d383a41 100644 --- a/src/components/SidebarIndexList.js +++ b/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 ( - <MenuItem - key={canvas.id} - sx={{ - paddingRight: 1, - position: 'initial', - }} - divider - onClick={onClick} - component="li" + <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)} > - <ScrollTo - containerRef={containerRef} - key={`${canvas.id}-${variant}`} - offsetTop={96} // offset for the height of the form above - scrollTo={selectedCanvasIds.includes(canvas.id)} + <MenuItem + key={canvas.id} + disableGutters + sx={{ + paddingLeft: 2, + paddingRight: 1, + position: 'initial', + }} + divider + onClick={onClick} + component="li" > <Item label={canvas.label} canvas={canvases[canvasIndex]} /> - </ScrollTo> - </MenuItem> + </MenuItem> + </ScrollTo> ); }) }