From fed39c4b0c34a2d1c73baf558202d713798fabb9 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 27 Jul 2023 04:45:55 +1000 Subject: [PATCH] Add button which resets the number of commits to show for a branch to experiments table --- extension/src/experiments/model/index.ts | 5 +++ extension/src/experiments/webview/messages.ts | 24 +++++++++---- extension/src/telemetry/constants.ts | 3 ++ .../src/test/suite/experiments/index.test.ts | 33 +++++++++++++++++ extension/src/webview/contract.ts | 5 +++ webview/icons/codicons.mjs | 1 + .../src/experiments/components/App.test.tsx | 21 ++++++++++- .../table/body/commits/CommitsButton.tsx | 15 ++++---- .../table/body/commits/CommitsNavigation.tsx | 35 ++++++++++++++----- webview/src/experiments/util/messages.ts | 6 ++++ .../src/shared/components/icons/Discard.tsx | 19 ++++++++++ webview/src/shared/components/icons/index.ts | 1 + 12 files changed, 145 insertions(+), 23 deletions(-) create mode 100644 webview/src/shared/components/icons/Discard.tsx diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts index 3c646af8c6..538079580f 100644 --- a/extension/src/experiments/model/index.ts +++ b/extension/src/experiments/model/index.ts @@ -478,6 +478,11 @@ export class ExperimentsModel extends ModelWithPersistence { this.persistNbOfCommitsToShow() } + public resetNbfCommitsToShow(branch: string) { + delete this.numberOfCommitsToShow[branch] + this.persistNbOfCommitsToShow() + } + public getNbOfCommitsToShow(branch: string) { return ( this.numberOfCommitsToShow[branch] || diff --git a/extension/src/experiments/webview/messages.ts b/extension/src/experiments/webview/messages.ts index 6b901ad2c2..93757612a9 100644 --- a/extension/src/experiments/webview/messages.ts +++ b/extension/src/experiments/webview/messages.ts @@ -190,9 +190,11 @@ export class WebviewMessages { case MessageFromWebviewType.SHOW_MORE_COMMITS: return this.changeCommitsToShow(1, message.payload) - case MessageFromWebviewType.SHOW_LESS_COMMITS: return this.changeCommitsToShow(-1, message.payload) + case MessageFromWebviewType.RESET_COMMITS: + return this.resetCommitsToShow(message.payload) + case MessageFromWebviewType.SELECT_BRANCHES: return this.addAndRemoveBranches() @@ -221,6 +223,12 @@ export class WebviewMessages { } private async changeCommitsToShow(change: 1 | -1, branch: string) { + this.experiments.setNbfCommitsToShow( + this.experiments.getNbOfCommitsToShow(branch) + + NUM_OF_COMMITS_TO_INCREASE * change, + branch + ) + await this.update() sendTelemetryEvent( change === 1 ? EventName.VIEWS_EXPERIMENTS_TABLE_SHOW_MORE_COMMITS @@ -228,12 +236,16 @@ export class WebviewMessages { undefined, undefined ) - this.experiments.setNbfCommitsToShow( - this.experiments.getNbOfCommitsToShow(branch) + - NUM_OF_COMMITS_TO_INCREASE * change, - branch - ) + } + + private async resetCommitsToShow(branch: string) { + this.experiments.resetNbfCommitsToShow(branch) await this.update() + sendTelemetryEvent( + EventName.VIEWS_EXPERIMENTS_TABLE_RESET_COMMITS, + undefined, + undefined + ) } private refreshData() { diff --git a/extension/src/telemetry/constants.ts b/extension/src/telemetry/constants.ts index a87eba41ce..0574bf9eaa 100644 --- a/extension/src/telemetry/constants.ts +++ b/extension/src/telemetry/constants.ts @@ -51,6 +51,8 @@ export const EventName = Object.assign( VIEWS_EXPERIMENTS_TABLE_REFRESH: 'views.experimentsTable.refresh', VIEWS_EXPERIMENTS_TABLE_REMOVE_COLUMN_SORT: 'views.experimentsTable.columnSortRemoved', + VIEWS_EXPERIMENTS_TABLE_RESET_COMMITS: + 'views.experimentsTable.resetCommits', VIEWS_EXPERIMENTS_TABLE_RESIZE_COLUMN: 'views.experimentsTable.columnResized', VIEWS_EXPERIMENTS_TABLE_SELECT_BRANCHES: @@ -246,6 +248,7 @@ export interface IEventNamePropertyMapping { [EventName.VIEWS_EXPERIMENTS_TABLE_REMOVE_COLUMN_SORT]: { path: string } + [EventName.VIEWS_EXPERIMENTS_TABLE_RESET_COMMITS]: undefined [EventName.VIEWS_EXPERIMENTS_TABLE_CREATED]: undefined [EventName.VIEWS_EXPERIMENTS_TABLE_FOCUS_CHANGED]: WebviewFocusChangedProperties [EventName.VIEWS_EXPERIMENTS_TABLE_HIDE_COLUMN_PATH]: { diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts index ab1b03ce11..f0e9322ae1 100644 --- a/extension/src/test/suite/experiments/index.test.ts +++ b/extension/src/test/suite/experiments/index.test.ts @@ -1480,6 +1480,39 @@ suite('Experiments Test Suite', () => { expect(setNbfCommitsToShowSpy).to.be.calledWith(3, 'main') }).timeout(WEBVIEW_TEST_TIMEOUT) + it('should handle a message to reset the number of commits shown for a branch', async () => { + const { + experiments, + experimentsModel, + messageSpy, + mockUpdateExperimentsData + } = setupExperimentsAndMockCommands() + + experimentsModel.setNbfCommitsToShow(100, 'main') + const getNumberOfCommitsToShowForMain = () => + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (experimentsModel as any).numberOfCommitsToShow.main + expect(getNumberOfCommitsToShowForMain()).to.equal(100) + + const resetNbfCommitsToShowSpy = spy( + experimentsModel, + 'resetNbfCommitsToShow' + ) + + const webview = await experiments.showWebview() + messageSpy.resetHistory() + const mockMessageReceived = getMessageReceivedEmitter(webview) + + mockMessageReceived.fire({ + payload: 'main', + type: MessageFromWebviewType.RESET_COMMITS + }) + + expect(mockUpdateExperimentsData).to.be.calledOnce + expect(resetNbfCommitsToShowSpy).to.be.calledWithExactly('main') + expect(getNumberOfCommitsToShowForMain()).to.be.undefined + }).timeout(WEBVIEW_TEST_TIMEOUT) + it('should handle a message to select branches', async () => { const { experiments, diff --git a/extension/src/webview/contract.ts b/extension/src/webview/contract.ts index d660c24b55..b464e4b8f8 100644 --- a/extension/src/webview/contract.ts +++ b/extension/src/webview/contract.ts @@ -38,6 +38,7 @@ export enum MessageFromWebviewType { REORDER_PLOTS_TEMPLATES = 'reorder-plots-templates', REFRESH_EXP_DATA = 'refresh-exp-data', REFRESH_REVISIONS = 'refresh-revisions', + RESET_COMMITS = 'reset-commits', RESIZE_COLUMN = 'resize-column', RESIZE_PLOTS = 'resize-plots', SAVE_STUDIO_TOKEN = 'save-studio-token', @@ -119,6 +120,10 @@ export type MessageFromWebview = type: MessageFromWebviewType.REORDER_COLUMNS payload: string[] } + | { + type: MessageFromWebviewType.RESET_COMMITS + payload: string + } | { type: MessageFromWebviewType.RESIZE_COLUMN payload: ColumnResizePayload diff --git a/webview/icons/codicons.mjs b/webview/icons/codicons.mjs index 074688bade..bc80fbb5dd 100644 --- a/webview/icons/codicons.mjs +++ b/webview/icons/codicons.mjs @@ -10,6 +10,7 @@ export const codicons = [ 'cloud-upload', 'cloud', 'copy', + 'discard', 'ellipsis', 'error', 'extensions', diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx index 287f870242..7781c9a945 100644 --- a/webview/src/experiments/components/App.test.tsx +++ b/webview/src/experiments/components/App.test.tsx @@ -1868,7 +1868,7 @@ describe('App', () => { }) }) - describe('Show more commits', () => { + describe('Change number of commits', () => { it('should display a show more commits button', () => { renderTable({ ...tableDataFixture, hasMoreCommits: { main: true } }) @@ -1933,6 +1933,25 @@ describe('App', () => { type: MessageFromWebviewType.SHOW_LESS_COMMITS }) }) + + it('should display a reset commits button', () => { + renderTable({ ...tableDataFixture, hasMoreCommits: { main: true } }) + + expect( + screen.getByLabelText('Reset Commits to Default') + ).toBeInTheDocument() + }) + + it('should send a message to reset commits when the reset commits button is clicked', () => { + renderTable() + + fireEvent.click(screen.getByLabelText('Reset Commits to Default')) + + expect(mockPostMessage).toHaveBeenCalledWith({ + payload: 'main', + type: MessageFromWebviewType.RESET_COMMITS + }) + }) }) describe('Add / Remove branches', () => { diff --git a/webview/src/experiments/components/table/body/commits/CommitsButton.tsx b/webview/src/experiments/components/table/body/commits/CommitsButton.tsx index bf18de6e35..df02eda743 100644 --- a/webview/src/experiments/components/table/body/commits/CommitsButton.tsx +++ b/webview/src/experiments/components/table/body/commits/CommitsButton.tsx @@ -3,26 +3,25 @@ import styles from './styles.module.scss' import { Icon, IconValue } from '../../../../../shared/components/Icon' import Tooltip from '../../../../../shared/components/tooltip/Tooltip' export interface CommitsButtonProps { - icon: IconValue - moreOrLess: 'More' | 'Less' action: () => void disabled: boolean + icon: IconValue + tooltipContent: string } export const CommitsButton: React.FC = ({ - icon, - moreOrLess, action, - disabled + disabled, + icon, + tooltipContent }) => { - const text = `Show ${moreOrLess} Commits` return ( - {text}}> + diff --git a/webview/src/experiments/components/table/body/commits/CommitsNavigation.tsx b/webview/src/experiments/components/table/body/commits/CommitsNavigation.tsx index 929093aecc..3db54ab7cf 100644 --- a/webview/src/experiments/components/table/body/commits/CommitsNavigation.tsx +++ b/webview/src/experiments/components/table/body/commits/CommitsNavigation.tsx @@ -2,9 +2,13 @@ import React from 'react' import { useSelector } from 'react-redux' import styles from './styles.module.scss' import { CommitsButton, CommitsButtonProps } from './CommitsButton' -import { showLessCommits, showMoreCommits } from '../../../../util/messages' +import { + resetCommits, + showLessCommits, + showMoreCommits +} from '../../../../util/messages' import { ExperimentsState } from '../../../../store' -import { Add, Remove } from '../../../../../shared/components/icons' +import { Add, Discard, Remove } from '../../../../../shared/components/icons' interface CommitsNavigationProps { branch: string } @@ -16,26 +20,41 @@ export const CommitsNavigation: React.FC = ({ (state: ExperimentsState) => state.tableData ) - const commitsButtons: CommitsButtonProps[] = [ + const getMoreOrLessValues = ( + moreOrLess: 'More' | 'Less' + ): { key: string; tooltipContent: string } => ({ + key: moreOrLess, + tooltipContent: `Show ${moreOrLess} Commits` + }) + + const commitsButtons: (CommitsButtonProps & { key: string })[] = [ { action: () => showMoreCommits(branch), disabled: !hasMoreCommits[branch], icon: Add, - moreOrLess: 'More' + ...getMoreOrLessValues('More') }, { action: () => showLessCommits(branch), disabled: !isShowingMoreCommits[branch], icon: Remove, - moreOrLess: 'Less' + ...getMoreOrLessValues('Less') + }, + { + action: () => resetCommits(branch), + disabled: false, + icon: Discard, + key: 'Reset', + tooltipContent: 'Reset Commits to Default' } ] return (
- {commitsButtons.map(commitButton => ( - - ))} + {commitsButtons.map(commitButton => { + const { key, ...commitButtonProps } = commitButton + return + })}
) } diff --git a/webview/src/experiments/util/messages.ts b/webview/src/experiments/util/messages.ts index 742524da73..3162b06058 100644 --- a/webview/src/experiments/util/messages.ts +++ b/webview/src/experiments/util/messages.ts @@ -27,6 +27,12 @@ export const reorderColumns = (newOrder: string[]) => type: MessageFromWebviewType.REORDER_COLUMNS }) +export const resetCommits = (branch: string) => + sendMessage({ + payload: branch, + type: MessageFromWebviewType.RESET_COMMITS + }) + export const resizeColumn = (id: string, width: number) => sendMessage({ payload: { id, width }, diff --git a/webview/src/shared/components/icons/Discard.tsx b/webview/src/shared/components/icons/Discard.tsx new file mode 100644 index 0000000000..d3497a5025 --- /dev/null +++ b/webview/src/shared/components/icons/Discard.tsx @@ -0,0 +1,19 @@ +import * as React from 'react' +import type { SVGProps } from 'react' +const Discard = (props: SVGProps) => ( + + + +) +export default Discard diff --git a/webview/src/shared/components/icons/index.ts b/webview/src/shared/components/icons/index.ts index 9b84911d67..73333de81b 100644 --- a/webview/src/shared/components/icons/index.ts +++ b/webview/src/shared/components/icons/index.ts @@ -10,6 +10,7 @@ export { default as Close } from './Close' export { default as Cloud } from './Cloud' export { default as CloudUpload } from './CloudUpload' export { default as Copy } from './Copy' +export { default as Discard } from './Discard' export { default as Ellipsis } from './Ellipsis' export { default as Error } from './Error' export { default as Extensions } from './Extensions'