Merge pull request #33 from dbmdz/toggle-perf

Improve performance of selection/visibility toggles

__tests__/components/PageTextDisplay.test.js

@@ -128,17 +128,11 @@ describe('PageTextDisplay', () => {
   });
 
   it('should render text invisible if visibility is disabled', () => {
-    const { container, rerender } = renderPage();
-    container.querySelectorAll('rect').forEach(
-      (rect) => expect(rect).toHaveAttribute('style', 'fill: rgba(255, 255, 255, 0.75);'),
-    );
-    let firstLine = screen.getByText(svgTextMatcher('a firstWord on a line'));
-    expect(firstLine).toHaveAttribute('style', 'fill: rgba(0, 0, 0, 0.75);');
-    renderPage({ visible: false }, rerender);
+    const { container } = renderPage({ visible: false });
     container.querySelectorAll('rect').forEach(
       (rect) => expect(rect).toHaveAttribute('style', 'fill: rgba(255, 255, 255, 0);'),
     );
-    firstLine = screen.getByText(svgTextMatcher('a firstWord on a line'));
+    const firstLine = screen.getByText(svgTextMatcher('a firstWord on a line'));
     expect(firstLine).toHaveAttribute('style', 'fill: rgba(0, 0, 0, 0);');
   });
 
@@ -170,6 +164,13 @@ describe('PageTextDisplay', () => {
     expect(topCallback).toHaveBeenCalled();
   });
 
+  it('should disable text selection if selection is disabled', () => {
+    const { ref, container } = renderPage();
+    expect(container.querySelectorAll('svg')[1]).toHaveStyle('user-select: text');
+    ref.current.updateSelectability(false);
+    expect(container.querySelectorAll('svg')[1]).not.toHaveStyle('user-select: text');
+  });
+
   it('should render spans as <text> elements when running under Gecko', () => {
     const prevAgent = global.navigator.userAgent;
     global.navigator.userAgent = 'Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0';

src/components/MiradorTextOverlay.js

@@ -32,7 +32,7 @@ class MiradorTextOverlay extends Component {
   /** Register OpenSeadragon callback when viewport changes */
   componentDidUpdate(prevProps) {
     const {
-      enabled, viewer, pageTexts, textColor, bgColor, useAutoColors, visible,
+      enabled, viewer, pageTexts, textColor, bgColor, useAutoColors, visible, selectable,
     } = this.props;
     let { opacity } = this.props;
 
@@ -50,10 +50,17 @@ class MiradorTextOverlay extends Component {
       this.onUpdateViewport();
     }
 
+    if (selectable !== prevProps.selectable) {
+      this.renderRefs
+        .filter((ref) => ref.current)
+        .forEach((ref) => ref.current.updateSelectability(selectable));
+    }
+
     // Manually update SVG colors for performance reasons
     // eslint-disable-next-line require-jsdoc
     const hasPageColors = (text) => text.textColor !== undefined;
-    if (opacity !== prevProps.opacity
+    if (visible !== prevProps.visible
+        || opacity !== prevProps.opacity
         || bgColor !== prevProps.bgColor
         || textColor !== prevProps.textColor
         || useAutoColors !== prevProps.useAutoColors
@@ -148,10 +155,8 @@ class MiradorTextOverlay extends Component {
 
   /** If the overlay should be rendered at all */
   shouldRender(props) {
-    const {
-      enabled, visible, selectable, pageTexts,
-    } = props ?? this.props;
-    return (enabled && (visible || selectable) && pageTexts.length > 0);
+    const { enabled, pageTexts } = props ?? this.props;
+    return (enabled && pageTexts.length > 0);
   }
 
   /** Update container dimensions and page scale/offset every time the OSD viewport changes. */
@@ -169,7 +174,13 @@ class MiradorTextOverlay extends Component {
       return null;
     }
     return ReactDOM.createPortal(
-      <div ref={this.containerRef} style={{ position: 'absolute' }}>
+      <div
+        ref={this.containerRef}
+        style={{
+          position: 'absolute',
+          display: (selectable || visible) ? null : 'none',
+        }}
+      >
         {pageTexts
           .map(({
             lines, source, width: pageWidth, height: pageHeight,

src/components/PageTextDisplay.js

@@ -51,12 +51,8 @@ class PageTextDisplay extends React.Component {
    * but this way we can more precisely control when we re-render.
    */
   shouldComponentUpdate(nextProps) {
-    const { source, selectable, visible } = this.props;
-    return (
-      nextProps.source !== source
-      || nextProps.selectable !== selectable
-      || nextProps.visible !== visible
-    );
+    const { source } = this.props;
+    return nextProps.source !== source;
   }
 
   /** Swallow pointer events if selection is enabled */
@@ -111,6 +107,17 @@ class PageTextDisplay extends React.Component {
     }
   }
 
+  /** Update the selectability of the text nodes.
+   *
+   * Again, intended to be called from the parent, again for performance reasons.
+   */
+  updateSelectability(selectable) {
+    if (!this.textContainerRef.current) {
+      return;
+    }
+    this.textContainerRef.current.parentElement.style.userSelect = selectable ? 'text' : 'none';
+  }
+
   /** Render the page overlay */
   render() {
     const {

src/components/TextOverlaySettingsBubble.js

@@ -78,7 +78,7 @@ const OpacityWidget = ({ opacity, onChange, t }) => {
     >
       <Slider
         orientation="vertical"
-        min={0}
+        min={1}
         max={100}
         value={opacity * 100}
         getAriaValueText={(value) => t('opacityCurrentValue', { value })}

src/index.js

@@ -36,17 +36,10 @@ export default [
         updateWindow(windowId, { textOverlay: options }),
       ),
     }),
-    mapStateToProps: (state, { windowId, PluginComponents }) => {
-      // Check if mirador-image-tools plugin is available. We can't rely on the presence of
-      // the `imageToolsEnabled` window option, since the plugin is currently enabled by default,
-      // even in the absence of the option.
-      // FIXME: This should be removed once ProjectMirador/mirador-image-tools#23 is merged
-      const imageToolsPresent = PluginComponents
-        .filter(({ WrappedComponent: { name } }) => name === 'MiradorImageTools')
-        .length > 0;
+    mapStateToProps: (state, { windowId }) => {
       const { imageToolsEnabled } = getWindowConfig(state, { windowId });
       return {
-        imageToolsEnabled: imageToolsEnabled ?? imageToolsPresent,
+        imageToolsEnabled,
         textsAvailable: getTextsForVisibleCanvases(state, { windowId }).length > 0,
         textsFetching: getTextsForVisibleCanvases(state, { windowId }).some((t) => t.isFetching),
         pageColors: getTextsForVisibleCanvases(state, { windowId })