Review when plot sections should collapse (#2285)

* Apply comments on plot section collapse

* Self review

Author: sroy3

webview/src/plots/components/App.test.tsx

@@ -44,6 +44,7 @@ import { plotsReducers } from '../store'
 import { vsCodeApi } from '../../shared/api'
 import { createBubbledEvent, dragAndDrop, dragEnter } from '../../test/dragDrop'
 import { DragEnterDirection } from '../../shared/components/dragDrop/util'
+import { clearSelection, createWindowTextSelection } from '../../test/selection'
 
 jest.mock('../../shared/api')
 
@@ -338,6 +339,69 @@ describe('App', () => {
     ).not.toBeInTheDocument()
   })
 
+  it('should not toggle the checkpoint plots section when its header is clicked and its title is selected', async () => {
+    renderAppWithOptionalData({
+      checkpoint: checkpointPlotsFixture
+    })
+
+    const summaryElement = await screen.findByText('Trends')
+
+    createWindowTextSelection('Trends', 2)
+    fireEvent.click(summaryElement, {
+      bubbles: true,
+      cancelable: true
+    })
+
+    expect(mockPostMessage).not.toHaveBeenCalledWith({
+      payload: { [Section.CHECKPOINT_PLOTS]: true },
+      type: MessageFromWebviewType.TOGGLE_PLOTS_SECTION
+    })
+
+    clearSelection()
+    fireEvent.click(summaryElement, {
+      bubbles: true,
+      cancelable: true
+    })
+
+    expect(mockPostMessage).toBeCalledWith({
+      payload: { [Section.CHECKPOINT_PLOTS]: true },
+      type: MessageFromWebviewType.TOGGLE_PLOTS_SECTION
+    })
+  })
+
+  it('should not toggle the checkpoint plots section when its header is clicked and the content of its tooltip is selected', async () => {
+    renderAppWithOptionalData({
+      checkpoint: checkpointPlotsFixture
+    })
+
+    const summaryElement = await screen.findByText('Trends')
+    createWindowTextSelection(
+      // eslint-disable-next-line testing-library/no-node-access
+      SectionDescription['checkpoint-plots'].props.children,
+      2
+    )
+    fireEvent.click(summaryElement, {
+      bubbles: true,
+      cancelable: true
+    })
+
+    expect(mockPostMessage).not.toHaveBeenCalledWith({
+      payload: { [Section.CHECKPOINT_PLOTS]: true },
+      type: MessageFromWebviewType.TOGGLE_PLOTS_SECTION
+    })
+
+    clearSelection()
+    fireEvent.click(summaryElement, {
+      bubbles: true,
+      cancelable: true
+    })
+
+    expect(mockPostMessage).toBeCalledWith({
+      payload: { [Section.CHECKPOINT_PLOTS]: true },
+      type: MessageFromWebviewType.TOGGLE_PLOTS_SECTION
+    })
+  })
+
   it('should toggle the visibility of plots when clicking the metrics in the metrics picker', async () => {
     renderAppWithOptionalData({
       checkpoint: checkpointPlotsFixture

webview/src/plots/components/PlotsContainer.tsx

@@ -1,5 +1,5 @@
 import cx from 'classnames'
-import React, { useEffect, useState } from 'react'
+import React, { MouseEvent, useEffect, useState } from 'react'
 import { PlotSize, Section } from 'dvc/src/plots/webview/contract'
 import { MessageFromWebviewType } from 'dvc/src/webview/contract'
 import { PlotsPicker, PlotsPickerProps } from './PlotsPicker'
@@ -17,6 +17,7 @@ import {
   Info,
   Lines
 } from '../../shared/components/icons'
+import { isSelecting } from '../../util/strings'
 
 export interface CommonPlotsContainerProps {
   onResize: (size: PlotSize) => void
@@ -113,20 +114,22 @@ export const PlotsContainer: React.FC<PlotsContainerProps> = ({
     </div>
   )
 
+  const toggleSection = (e: MouseEvent) => {
+    e.preventDefault()
+    if (!isSelecting([title, SectionDescription[sectionKey].props.children])) {
+      sendMessage({
+        payload: {
+          [sectionKey]: !sectionCollapsed
+        },
+        type: MessageFromWebviewType.TOGGLE_PLOTS_SECTION
+      })
+    }
+  }
+
   return (
     <div className={styles.plotsContainerWrapper} data-testid="plots-container">
       <details open={open} className={styles.plotsContainer}>
-        <summary
-          onClick={e => {
-            e.preventDefault()
-            sendMessage({
-              payload: {
-                [sectionKey]: !sectionCollapsed
-              },
-              type: MessageFromWebviewType.TOGGLE_PLOTS_SECTION
-            })
-          }}
-        >
+        <summary onClick={toggleSection}>
           <Icon
             icon={open ? ChevronDown : ChevronRight}
             data-testid="plots-container-details-chevron"

webview/src/plots/components/comparisonTable/ComparisonTableRow.tsx

@@ -10,6 +10,7 @@ import { sendMessage } from '../../../shared/vscode'
 import { ChevronDown, ChevronRight } from '../../../shared/components/icons'
 import { PlotsState } from '../../store'
 import { CopyButton } from '../../../shared/components/copyButton/CopyButton'
+import { isSelecting } from '../../../util/strings'
 
 export interface ComparisonTableRowProps {
   path: string
@@ -30,11 +31,7 @@ export const ComparisonTableRow: React.FC<ComparisonTableRowProps> = ({
   const [isShown, setIsShown] = useState(true)
 
   const toggleIsShownState = () => {
-    const selection = window.getSelection()
-    if (
-      selection?.focusNode?.nodeValue === path &&
-      selection.anchorOffset !== selection.focusOffset
-    ) {
+    if (isSelecting([path])) {
       return
     }
     setIsShown(!isShown)

webview/src/plots/components/styles.module.scss

@@ -19,6 +19,11 @@ $gap: 20px;
     font-size: 1.25rem;
     display: flex;
     align-items: center;
+    width: max-content;
+
+    svg {
+      cursor: pointer;
+    }
   }
 }
 

webview/src/util/strings.ts

@@ -3,3 +3,12 @@ export const capitalize = (s: string) =>
 
 export const pluralize = (word: string, number: number | undefined) =>
   number === 1 ? word : `${word}s`
+
+export const isSelecting = (text: string[]) => {
+  const selection = window.getSelection()
+
+  return (
+    text.includes(selection?.focusNode?.nodeValue || '') &&
+    selection?.anchorOffset !== selection?.focusOffset
+  )
+}