Add experiments from plots ribbon (#1798)

* Add plots ribbon

* Fix tests and event name

* Add tests

* Add react-virtualized

* Add extension test

* Add button in plots ribbon to add experiments

* Add tests

* Add refresh all button

* add refresh all visible action to plots (#1808)

Co-authored-by: mattseddon <[email protected]>

Author: sroy3

extension/src/plots/index.ts

@@ -267,7 +267,9 @@ export class Plots extends BaseRepository<TPlotsData> {
           case MessageFromWebviewType.SELECT_EXPERIMENTS:
             return this.selectExperimentsFromWebview()
           case MessageFromWebviewType.REFRESH_REVISION:
-            return this.attemptToRefreshData(message.payload)
+            return this.attemptToRefreshRevData(message.payload)
+          case MessageFromWebviewType.REFRESH_REVISIONS:
+            return this.attemptToRefreshSelectedData(message.payload)
           case MessageFromWebviewType.TOGGLE_EXPERIMENT:
             return this.setExperimentStatus(message.payload)
           default:
@@ -364,13 +366,26 @@ export class Plots extends BaseRepository<TPlotsData> {
     )
   }
 
-  private attemptToRefreshData(revision: string) {
-    Toast.infoWithOptions(`Attempting to refresh ${revision} plots data.`)
+  private attemptToRefreshRevData(revision: string) {
+    Toast.infoWithOptions(`Attempting to refresh plots data for ${revision}.`)
     this.plots?.setupManualRefresh(revision)
     this.data.managedUpdate()
     sendTelemetryEvent(
       EventName.VIEWS_PLOTS_MANUAL_REFRESH,
-      undefined,
+      { revisions: 1 },
+      undefined
+    )
+  }
+
+  private attemptToRefreshSelectedData(revisions: string[]) {
+    Toast.infoWithOptions('Attempting to refresh visible plots data.')
+    for (const revision of revisions) {
+      this.plots?.setupManualRefresh(revision)
+    }
+    this.data.managedUpdate()
+    sendTelemetryEvent(
+      EventName.VIEWS_PLOTS_MANUAL_REFRESH,
+      { revisions: revisions.length },
       undefined
     )
   }

extension/src/telemetry/constants.ts

@@ -205,7 +205,7 @@ export interface IEventNamePropertyMapping {
   [EventName.VIEWS_PLOTS_CLOSED]: undefined
   [EventName.VIEWS_PLOTS_CREATED]: undefined
   [EventName.VIEWS_PLOTS_FOCUS_CHANGED]: WebviewFocusChangedProperties
-  [EventName.VIEWS_PLOTS_MANUAL_REFRESH]: undefined
+  [EventName.VIEWS_PLOTS_MANUAL_REFRESH]: { revisions: number }
   [EventName.VIEWS_PLOTS_METRICS_SELECTED]: undefined
   [EventName.VIEWS_PLOTS_RENAME_SECTION]: { section: Section }
   [EventName.VIEWS_PLOTS_REVISIONS_REORDERED]: undefined

extension/src/test/suite/plots/index.test.ts

@@ -619,7 +619,7 @@ suite('Plots Test Suite', () => {
       )
     }).timeout(WEBVIEW_TEST_TIMEOUT)
 
-    it('should handle a manual refresh message from the webview', async () => {
+    it('should handle a message to manually refresh a revision from the webview', async () => {
       const { data, plots, plotsModel, mockPlotsDiff } = await buildPlots(
         disposable,
         plotsDiffFixture
@@ -648,13 +648,53 @@ suite('Plots Test Suite', () => {
       expect(mockSendTelemetryEvent).to.be.calledOnce
       expect(mockSendTelemetryEvent).to.be.calledWithExactly(
         EventName.VIEWS_PLOTS_MANUAL_REFRESH,
-        undefined,
+        { revisions: 1 },
         undefined
       )
       expect(mockPlotsDiff).to.be.calledOnce
       expect(mockPlotsDiff).to.be.calledWithExactly(dvcDemoPath, 'main')
     }).timeout(WEBVIEW_TEST_TIMEOUT)
 
+    it('should handle a message to manually refresh all visible plots from the webview', async () => {
+      const { data, plots, mockPlotsDiff } = await buildPlots(
+        disposable,
+        plotsDiffFixture
+      )
+
+      const webview = await plots.showWebview()
+      mockPlotsDiff.resetHistory()
+
+      const mockSendTelemetryEvent = stub(Telemetry, 'sendTelemetryEvent')
+      const mockMessageReceived = getMessageReceivedEmitter(webview)
+
+      const dataUpdateEvent = new Promise(resolve =>
+        data.onDidUpdate(() => resolve(undefined))
+      )
+
+      mockMessageReceived.fire({
+        payload: ['1ba7bcd', '42b8736', '4fb124a', 'main', 'workspace'],
+        type: MessageFromWebviewType.REFRESH_REVISIONS
+      })
+
+      await dataUpdateEvent
+
+      expect(mockSendTelemetryEvent).to.be.calledOnce
+      expect(mockSendTelemetryEvent).to.be.calledWithExactly(
+        EventName.VIEWS_PLOTS_MANUAL_REFRESH,
+        { revisions: 5 },
+        undefined
+      )
+      expect(mockPlotsDiff).to.be.calledOnce
+      expect(mockPlotsDiff).to.be.calledWithExactly(
+        dvcDemoPath,
+        '1ba7bcd',
+        '42b8736',
+        '4fb124a',
+        'main',
+        'workspace'
+      )
+    }).timeout(WEBVIEW_TEST_TIMEOUT)
+
     it('should be able to make the plots webview visible', async () => {
       const { plots, messageSpy, mockPlotsDiff } = await buildPlots(
         disposable,

extension/src/webview/contract.ts

@@ -23,6 +23,7 @@ export enum MessageFromWebviewType {
   REORDER_PLOTS_METRICS = 'reorder-plots-metrics',
   REORDER_PLOTS_TEMPLATES = 'reorder-plots-templates',
   REFRESH_REVISION = 'refresh-revision',
+  REFRESH_REVISIONS = 'refresh-revisions',
   RESIZE_COLUMN = 'resize-column',
   RESIZE_PLOTS = 'resize-plots',
   SORT_COLUMN = 'sort-column',
@@ -137,6 +138,7 @@ export type MessageFromWebview =
   | { type: MessageFromWebviewType.SELECT_EXPERIMENTS }
   | { type: MessageFromWebviewType.SELECT_PLOTS }
   | { type: MessageFromWebviewType.REFRESH_REVISION; payload: string }
+  | { type: MessageFromWebviewType.REFRESH_REVISIONS; payload: string[] }
   | { type: MessageFromWebviewType.SELECT_COLUMNS }
 
 export type MessageToWebview<T extends WebviewData> = {

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

@@ -1552,9 +1552,10 @@ describe('App', () => {
       })
       const ribbon = screen.getByTestId('ribbon')
 
-      const revisions = within(ribbon)
-        .getAllByRole('listitem')
-        .map(item => item.textContent)
+      const revisionBlocks = within(ribbon).getAllByRole('listitem')
+      revisionBlocks.shift() // Remove filter button
+      revisionBlocks.shift() // Remove refresh all
+      const revisions = revisionBlocks.map(item => item.textContent)
       expect(revisions).toStrictEqual(
         comparisonTableFixture.revisions.map(rev =>
           rev.group ? rev.group.slice(1, -1) + rev.revision : rev.revision
@@ -1580,6 +1581,54 @@ describe('App', () => {
       })
     })
 
+    it('should display the number of experiments selected', () => {
+      renderAppWithData({
+        comparison: comparisonTableFixture,
+        sectionCollapsed: DEFAULT_SECTION_COLLAPSED
+      })
+
+      expect(
+        screen.getByText(`${comparisonTableFixture.revisions.length} of 7`)
+      ).toBeInTheDocument()
+    })
+
+    it('should send a message to select the revisions when clicking the filter button', () => {
+      renderAppWithData({
+        comparison: comparisonTableFixture,
+        sectionCollapsed: DEFAULT_SECTION_COLLAPSED
+      })
+
+      const filterButton = within(screen.getByTestId('ribbon')).getAllByRole(
+        'button'
+      )[0]
+
+      fireEvent.click(filterButton)
+
+      expect(mockPostMessage).toBeCalledWith({
+        type: MessageFromWebviewType.SELECT_EXPERIMENTS
+      })
+    })
+
+    it('should send a message to refresh each revision when clicking the refresh all button', () => {
+      renderAppWithData({
+        comparison: comparisonTableFixture,
+        sectionCollapsed: DEFAULT_SECTION_COLLAPSED
+      })
+
+      const refreshAllButton = within(
+        screen.getByTestId('ribbon')
+      ).getAllByRole('button')[1]
+
+      mockPostMessage.mockReset()
+      fireEvent.click(refreshAllButton)
+
+      expect(mockPostMessage).toHaveBeenCalledTimes(1)
+      expect(mockPostMessage).toBeCalledWith({
+        payload: ['workspace', 'main', '4fb124a', '42b8736', '1ba7bcd'],
+        type: MessageFromWebviewType.REFRESH_REVISIONS
+      })
+    })
+
     describe('Copy button', () => {
       const mockWriteText = jest.fn()
       Object.assign(navigator, {
@@ -1588,7 +1637,17 @@ describe('App', () => {
         }
       })
 
-      it('should copy the experiment name when clicking the text', () => {
+      beforeAll(() => {
+        jest.useFakeTimers()
+      })
+
+      afterAll(() => {
+        jest.useRealTimers()
+      })
+
+      it('should copy the experiment name when clicking the text', async () => {
+        mockWriteText.mockResolvedValueOnce('success')
+
         renderAppWithData({
           comparison: comparisonTableFixture,
           sectionCollapsed: DEFAULT_SECTION_COLLAPSED
@@ -1598,14 +1657,14 @@ describe('App', () => {
           screen.getByTestId('ribbon-main')
         ).getAllByRole('button')[0]
 
+        fireEvent.mouseEnter(mainNameButton, { bubbles: true })
         fireEvent.click(mainNameButton)
 
         expect(mockWriteText).toBeCalledWith('main')
+        await screen.findByText(CopyTooltip.COPIED)
       })
 
       it('should display that the experiment was copied when clicking the text', async () => {
-        jest.useFakeTimers()
-
         mockWriteText.mockResolvedValueOnce('success')
 
         renderAppWithData({
@@ -1621,11 +1680,10 @@ describe('App', () => {
         fireEvent.click(mainNameButton)
 
         expect(await screen.findByText(CopyTooltip.COPIED)).toBeInTheDocument()
-        jest.useRealTimers()
       })
 
-      it('should display copy again when hovering the text 2s after clicking the text', () => {
-        jest.useFakeTimers()
+      it('should display copy again when hovering the text 2s after clicking the text', async () => {
+        mockWriteText.mockResolvedValueOnce('success')
 
         renderAppWithData({
           comparison: comparisonTableFixture,
@@ -1636,13 +1694,12 @@ describe('App', () => {
           screen.getByTestId('ribbon-main')
         ).getAllByRole('button')[0]
 
-        fireEvent.click(mainNameButton)
         fireEvent.mouseEnter(mainNameButton, { bubbles: true })
+        fireEvent.click(mainNameButton)
 
         jest.advanceTimersByTime(2001)
 
-        expect(screen.getByText(CopyTooltip.NORMAL)).toBeInTheDocument()
-        jest.useRealTimers()
+        expect(await screen.findByText(CopyTooltip.NORMAL)).toBeInTheDocument()
       })
     })
   })

webview/src/plots/components/ribbon/Ribbon.tsx

@@ -4,20 +4,52 @@ import React from 'react'
 import styles from './styles.module.scss'
 import { RibbonBlock } from './RibbonBlock'
 import { sendMessage } from '../../../shared/vscode'
+import { AllIcons } from '../../../shared/components/Icon'
+import { IconButton } from '../../../shared/components/button/IconButton'
 
 interface RibbonProps {
   revisions: Revision[]
 }
 
+const MAX_NB_EXP = 7
+
 export const Ribbon: React.FC<RibbonProps> = ({ revisions }) => {
   const removeRevision = (revision: string) => {
     sendMessage({
       payload: revision,
       type: MessageFromWebviewType.TOGGLE_EXPERIMENT
     })
   }
+
+  const refreshRevisions = () =>
+    sendMessage({
+      payload: revisions.map(({ revision }) => revision),
+      type: MessageFromWebviewType.REFRESH_REVISIONS
+    })
+
+  const selectRevisions = () => {
+    sendMessage({
+      type: MessageFromWebviewType.SELECT_EXPERIMENTS
+    })
+  }
+
   return (
     <ul className={styles.list} data-testid="ribbon">
+      <li className={styles.addButtonWrapper}>
+        <IconButton
+          onClick={selectRevisions}
+          icon={AllIcons.LINES}
+          text={`${revisions.length} of ${MAX_NB_EXP}`}
+        />
+      </li>
+      <li className={styles.addButtonWrapper}>
+        <IconButton
+          onClick={refreshRevisions}
+          icon={AllIcons.REFRESH}
+          text="Refresh All"
+          appearance="secondary"
+        />
+      </li>
       {revisions.map(revision => (
         <RibbonBlock
           revision={revision}

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

@@ -57,3 +57,8 @@
   padding: 10px 15px;
   margin: 0;
 }
+
+.addButtonWrapper {
+  background-color: $accent-color;
+  list-style: none;
+}

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

@@ -277,15 +277,15 @@ $gap: 20px;
 .dropTarget {
   height: auto;
   opacity: 0.5;
-  border: 3px dashed var(--vscode-focusBorder);
+  border: 3px dashed $accent-color;
   display: flex;
   justify-content: center;
   align-items: center;
 }
 
 .dropIcon {
   border-radius: 100%;
-  border: 3px solid var(--vscode-focusBorder);
+  border: 3px solid $accent-color;
   padding: 20px;
 }
 
@@ -298,7 +298,7 @@ $gap: 20px;
   box-sizing: content-box;
 
   path {
-    fill: var(--vscode-focusBorder);
+    fill: $accent-color;
   }
 }
 

webview/src/shared/variables.scss

@@ -16,3 +16,4 @@ $header-border-color: var(--vscode-tree-tableColumnsBorder);
 $meta-cell-color: var(--vscode-descriptionForeground);
 
 $hover-background-color: var(--vscode-list-hoverBackground);
+$accent-color: var(--vscode-focusBorder);