Remove click listener from experiment name and sha (#4543)

iterative · Aug 21, 2023 · 1c7481b · 1c7481b
1 parent 19cecc3
commit 1c7481b
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 72 deletions.
diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx
@@ -324,10 +324,12 @@ describe('App', () => {
   })
 
   describe('Toggle experiment status', () => {
-    it('should send a message to the extension to toggle an experiment when the row is clicked', () => {
+    it('should send a message to the extension to toggle an experiment when a plot icon is clicked', () => {
       renderTable()
 
-      const testClick = (element: HTMLElement, id: string) => {
+      const testClick = (elementId: string, id: string) => {
+        const element = within(getRow(elementId)).getByTestId('plot-icon')
+
         mockPostMessage.mockReset()
 
         fireEvent.click(element)
@@ -339,56 +341,10 @@ describe('App', () => {
         })
       }
 
-      const [workspace, experimentRunningInWorkspace] = screen.getAllByText(
-        EXPERIMENT_WORKSPACE_ID
-      )
-
-      testClick(workspace, EXPERIMENT_WORKSPACE_ID)
-      testClick(experimentRunningInWorkspace, 'exp-83425')
-      testClick(screen.getAllByText('main')[1], 'main')
-      testClick(screen.getByText('[exp-e7a67]'), 'exp-e7a67')
-    })
-
-    it('should send a message to the extension to toggle an experiment when Enter or Space is pressed on the row', () => {
-      renderTable()
-
-      mockPostMessage.mockClear()
-
-      const testRowLabel = screen.getAllByText('main')[1]
-
-      testRowLabel.focus()
-
-      fireEvent.keyDown(testRowLabel, {
-        bubbles: true,
-        code: 'Enter',
-        key: 'Enter',
-        keyCode: 13
-      })
-      expect(mockPostMessage).toHaveBeenCalledWith({
-        payload: 'main',
-        type: MessageFromWebviewType.TOGGLE_EXPERIMENT
-      })
-      mockPostMessage.mockClear()
-
-      fireEvent.keyDown(testRowLabel, {
-        bubbles: true,
-        charCode: 32,
-        code: 'Space',
-        key: ' ',
-        keyCode: 32
-      })
-      expect(mockPostMessage).toHaveBeenCalledWith({
-        payload: 'main',
-        type: MessageFromWebviewType.TOGGLE_EXPERIMENT
-      })
-      mockPostMessage.mockClear()
-
-      fireEvent.keyDown(testRowLabel, {
-        bubbles: true,
-        code: 'keyA',
-        key: 'a'
-      })
-      expect(mockPostMessage).not.toHaveBeenCalled()
+      testClick(EXPERIMENT_WORKSPACE_ID, EXPERIMENT_WORKSPACE_ID)
+      testClick('[exp-83425]', 'exp-83425')
+      testClick('main', 'main')
+      testClick('[exp-e7a67]', 'exp-e7a67')
     })
 
     it('should not send a message if row label was selected', () => {
@@ -412,13 +368,14 @@ describe('App', () => {
       clearSelection()
       fireEvent.click(getWorkspace())
 
-      expect(mockPostMessage).toHaveBeenCalledTimes(1)
-      expect(mockPostMessage).toHaveBeenCalledWith({
+      expect(mockPostMessage).not.toHaveBeenCalledTimes(1)
+      expect(mockPostMessage).not.toHaveBeenCalledWith({
         payload: testRowId,
         type: MessageFromWebviewType.TOGGLE_EXPERIMENT
       })
     })
-    it('should send a message if some other label is selected', () => {
+
+    it('should not send a message if some other label is selected', () => {
       renderTable()
       mockPostMessage.mockClear()
 
@@ -428,8 +385,8 @@ describe('App', () => {
       createWindowTextSelection(selectedTestRowId, 5)
       fireEvent.click(screen.getAllByText(testRowId)[1])
 
-      expect(mockPostMessage).toHaveBeenCalledTimes(1)
-      expect(mockPostMessage).toHaveBeenCalledWith({
+      expect(mockPostMessage).not.toHaveBeenCalledTimes(1)
+      expect(mockPostMessage).not.toHaveBeenCalledWith({
         payload: testRowId,
         type: MessageFromWebviewType.TOGGLE_EXPERIMENT
       })

diff --git a/webview/src/experiments/components/table/body/Cell.tsx b/webview/src/experiments/components/table/body/Cell.tsx
@@ -6,7 +6,6 @@ import { ExperimentStatusIndicator } from './ExperimentStatusIndicator'
 import styles from '../styles.module.scss'
 import { CellValue, isValueWithChanges } from '../content/Cell'
 import { CellProp, RowProp } from '../../../util/interfaces'
-import { clickAndEnterProps } from '../../../../util/props'
 import { ErrorTooltip } from '../../../../shared/components/tooltip/ErrorTooltip'
 
 const cellHasChanges = (cellValue: CellValue) =>
@@ -51,16 +50,8 @@ export const StubCell: React.FC<
     }
   } = cell
   const {
-    original: {
-      error,
-      executorStatus,
-      gitRemoteStatus,
-      label,
-      id,
-      description = ''
-    }
+    original: { error, executorStatus, gitRemoteStatus, id }
   } = row
-  const { toggleExperiment } = rowActionsProps
 
   return (
     <td className={cx(styles.experimentsTd, styles.experimentCell)}>
@@ -80,11 +71,6 @@ export const StubCell: React.FC<
                 [styles.workspaceChangeText]: changesIfWorkspace,
                 [styles.errorText]: error
               })}
-              {...clickAndEnterProps(
-                toggleExperiment,
-                [label, description],
-                true
-              )}
               data-testid={`id___${id}`}
             >
               {flexRender(columnCell, getContext())}

diff --git a/webview/src/experiments/components/table/body/CellRowActions.tsx b/webview/src/experiments/components/table/body/CellRowActions.tsx
@@ -129,6 +129,7 @@ export const CellRowActions: React.FC<CellRowActionsProps> = ({
         >
           <Icon
             className={styles.plotBox}
+            data-testid="plot-icon"
             style={
               plotColor
                 ? {
-Original file line number
+Diff line change
@@ Expand Up @@
             >
               <Icon
                 className={styles.plotBox}
+                data-testid="plot-icon"
                 style={
                   plotColor
                     ? {
@@ Expand Down @@