Skip to content

Commit

Permalink
Merge branch 'main' into branches-view
Browse files Browse the repository at this point in the history
  • Loading branch information
sroy3 authored Apr 4, 2023
2 parents 75f3031 + 54b10a2 commit dc234f5
Show file tree
Hide file tree
Showing 18 changed files with 325 additions and 213 deletions.
10 changes: 5 additions & 5 deletions extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1659,10 +1659,10 @@
"@types/vscode": "1.64.0",
"@vscode/test-electron": "2.3.0",
"@vscode/vsce": "2.18.0",
"@wdio/cli": "8.6.5",
"@wdio/local-runner": "8.6.3",
"@wdio/mocha-framework": "8.6.1",
"@wdio/spec-reporter": "8.4.0",
"@wdio/cli": "8.6.9",
"@wdio/local-runner": "8.6.9",
"@wdio/mocha-framework": "8.6.8",
"@wdio/spec-reporter": "8.6.8",
"chai": "4.3.7",
"chai-as-promised": "7.1.1",
"clean-webpack-plugin": "4.0.0",
Expand All @@ -1680,7 +1680,7 @@
"ts-loader": "9.4.2",
"vscode-uri": "3.0.7",
"wdio-vscode-service": "5.0.0",
"webdriverio": "8.6.3",
"webdriverio": "8.6.9",
"webpack": "5.76.3",
"webpack-cli": "5.0.1"
},
Expand Down
3 changes: 3 additions & 0 deletions extension/src/plots/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export class Plots extends BaseRepository<TPlotsData> {
this.webviewMessages = this.createWebviewMessageHandler(
this.paths,
this.plots,
this.errors,
experiments
)

Expand Down Expand Up @@ -162,11 +163,13 @@ export class Plots extends BaseRepository<TPlotsData> {
private createWebviewMessageHandler(
paths: PathsModel,
plots: PlotsModel,
errors: ErrorsModel,
experiments: Experiments
) {
const webviewMessages = new WebviewMessages(
paths,
plots,
errors,
experiments,
() => this.getWebview(),
() => this.selectPlots(),
Expand Down
2 changes: 2 additions & 0 deletions extension/src/plots/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export type ComparisonPlot = {

export enum PlotsDataKeys {
COMPARISON = 'comparison',
CLI_ERROR = 'cliError',
CUSTOM = 'custom',
HAS_UNSELECTED_PLOTS = 'hasUnselectedPlots',
HAS_PLOTS = 'hasPlots',
Expand All @@ -168,6 +169,7 @@ export type PlotsData =
| {
[PlotsDataKeys.COMPARISON]?: PlotsComparisonData | null
[PlotsDataKeys.CUSTOM]?: CustomPlotsData | null
[PlotsDataKeys.CLI_ERROR]?: string | null
[PlotsDataKeys.HAS_PLOTS]?: boolean
[PlotsDataKeys.HAS_UNSELECTED_PLOTS]?: boolean
[PlotsDataKeys.SELECTED_REVISIONS]?: Revision[]
Expand Down
5 changes: 5 additions & 0 deletions extension/src/plots/webview/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ import { Title } from '../../vscode/title'
import { reorderObjectList } from '../../util/array'
import { CustomPlotsOrderValue } from '../model/custom'
import { getCustomPlotId } from '../model/collect'
import { ErrorsModel } from '../errors/model'

export class WebviewMessages {
private readonly paths: PathsModel
private readonly plots: PlotsModel
private readonly errors: ErrorsModel
private readonly experiments: Experiments

private readonly getWebview: () => BaseWebview<TPlotsData> | undefined
Expand All @@ -40,13 +42,15 @@ export class WebviewMessages {
constructor(
paths: PathsModel,
plots: PlotsModel,
errors: ErrorsModel,
experiments: Experiments,
getWebview: () => BaseWebview<TPlotsData> | undefined,
selectPlots: () => Promise<void>,
updateData: () => Promise<void>
) {
this.paths = paths
this.plots = plots
this.errors = errors
this.experiments = experiments
this.getWebview = getWebview
this.selectPlots = selectPlots
Expand All @@ -60,6 +64,7 @@ export class WebviewMessages {
const comparison = this.getComparisonPlots(overrideComparison)

void this.getWebview()?.show({
cliError: this.errors.getCliError()?.error || null,
comparison,
custom: this.getCustomPlots(),
hasPlots: !!this.paths.hasPaths(),
Expand Down
1 change: 0 additions & 1 deletion extension/src/test/e2e/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ describe('Plots Webview', function () {
const plots = await webview.vegaVisualization$$

for (const plot of plots) {
await plot.scrollIntoView()
const plotNotEmpty = await webview.plotNotEmpty(plot)
expect(plotNotEmpty).toBe(true)
}
Expand Down
1 change: 1 addition & 0 deletions extension/src/test/suite/plots/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,7 @@ suite('Plots Test Suite', () => {
expect(templateData).to.deep.equal(templatePlotsFixture)

const expectedPlotsData: TPlotsData = {
cliError: null,
comparison: comparisonPlotsFixture,
custom: customPlotsFixture,
hasPlots: true,
Expand Down
1 change: 0 additions & 1 deletion extension/src/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export enum MessageFromWebviewType {
REORDER_PLOTS_COMPARISON_ROWS = 'reorder-plots-comparison-rows',
REORDER_PLOTS_CUSTOM = 'reorder-plots-custom',
REORDER_PLOTS_TEMPLATES = 'reorder-plots-templates',
REFRESH_REVISION = 'refresh-revision',
REFRESH_REVISIONS = 'refresh-revisions',
RESIZE_COLUMN = 'resize-column',
RESIZE_PLOTS = 'resize-plots',
Expand Down
51 changes: 51 additions & 0 deletions webview/src/plots/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,57 @@ describe('App', () => {
mockPostMessage.mockReset()
})

it('should render only get started (buttons: add experiments, add custom plots) when there is a cli error', async () => {
const cliError = 'this is an error thrown by DVC'

renderAppWithOptionalData({
cliError,
hasPlots: false,
hasUnselectedPlots: false,
selectedRevisions: [{} as Revision]
})
const addExperimentsButton = await screen.findByText('Add Experiments')
const addCustomPlotsButton = await screen.findByText('Add Custom Plot')
const errorIcon = await screen.findByTestId('error-icon')
const refreshButton = await screen.findByText('Refresh')

expect(addExperimentsButton).toBeInTheDocument()
expect(screen.queryByText('Add Plots')).not.toBeInTheDocument()
expect(addCustomPlotsButton).toBeInTheDocument()
expect(errorIcon).toBeInTheDocument()
expect(screen.queryByTestId('section-container')).not.toBeInTheDocument()

mockPostMessage.mockReset()

fireEvent.click(addExperimentsButton)

expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.SELECT_EXPERIMENTS
})

mockPostMessage.mockReset()

fireEvent.mouseEnter(errorIcon, { bubbles: true, cancelable: true })

const error = await screen.findByText(cliError)

expect(error).toBeInTheDocument()

fireEvent.click(addCustomPlotsButton)

expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.ADD_CUSTOM_PLOT
})
mockPostMessage.mockReset()

fireEvent.click(refreshButton)

expect(mockPostMessage).toHaveBeenCalledWith({
type: MessageFromWebviewType.REFRESH_REVISIONS
})
mockPostMessage.mockReset()
})

it('should render get started (buttons: add plots, add experiments) and custom section when there are some selected exps, all unselected plots, and added custom plots', async () => {
renderAppWithOptionalData({
custom: customPlotsFixture,
Expand Down
4 changes: 4 additions & 0 deletions webview/src/plots/components/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
} from './templatePlots/templatePlotsSlice'
import {
initialize,
updateCliError,
updateHasPlots,
updateHasUnselectedPlots,
updateSelectedRevisions
Expand Down Expand Up @@ -54,6 +55,9 @@ export const feedStore = (
dispatch(initialize())
for (const key of Object.keys(data.data)) {
switch (key) {
case PlotsDataKeys.CLI_ERROR:
dispatch(updateCliError(data.data[key]))
continue
case PlotsDataKeys.CUSTOM:
dispatch(updateCustomPlots(data.data[key] as CustomPlotsData))
continue
Expand Down
20 changes: 20 additions & 0 deletions webview/src/plots/components/ErrorIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react'
import styles from './styles.module.scss'
import { ErrorTooltip } from '../../shared/components/tooltip/ErrorTooltip'
import { Error } from '../../shared/components/icons'

export const ErrorIcon: React.FC<{ error: string; size: number }> = ({
error: msg,
size
}) => (
<ErrorTooltip error={msg}>
<div>
<Error
className={styles.errorIcon}
data-testid="error-icon"
height={size}
width={size}
/>
</div>
</ErrorTooltip>
)
15 changes: 14 additions & 1 deletion webview/src/plots/components/GetStarted.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
import React from 'react'
import { MessageFromWebviewType } from 'dvc/src/webview/contract'
import { ErrorIcon } from './ErrorIcon'
import { refreshRevisions } from './messages'
import { sendMessage } from '../../shared/vscode'
import { StartButton } from '../../shared/components/button/StartButton'
import { RefreshButton } from '../../shared/components/button/RefreshButton'

export type AddPlotsProps = {
hasUnselectedPlots: boolean
hasNoCustomPlots: boolean
cliError: string | undefined
}

export const AddPlots: React.FC<AddPlotsProps> = ({
cliError,
hasUnselectedPlots,
hasNoCustomPlots
}: AddPlotsProps) => (
<div>
{cliError && <ErrorIcon error={cliError} size={96} />}
<p>No Plots to Display</p>
<div>
<StartButton
Expand All @@ -23,7 +29,7 @@ export const AddPlots: React.FC<AddPlotsProps> = ({
}
text="Add Experiments"
/>
{hasUnselectedPlots && (
{hasUnselectedPlots && !cliError && (
<StartButton
isNested={hasUnselectedPlots}
appearance="secondary"
Expand All @@ -47,6 +53,13 @@ export const AddPlots: React.FC<AddPlotsProps> = ({
text="Add Custom Plot"
/>
)}
{cliError && (
<RefreshButton
onClick={refreshRevisions}
isNested={true}
appearance="secondary"
/>
)}
</div>
</div>
)
Expand Down
8 changes: 4 additions & 4 deletions webview/src/plots/components/Plots.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ import { PlotsState } from '../store'

const PlotsContent = () => {
const dispatch = useDispatch()
const { hasData, hasPlots, hasUnselectedPlots, zoomedInPlot } = useSelector(
(state: PlotsState) => state.webview
)
const { hasData, hasPlots, hasUnselectedPlots, zoomedInPlot, cliError } =
useSelector((state: PlotsState) => state.webview)
const hasComparisonData = useSelector(
(state: PlotsState) => state.comparison.hasData
)
Expand Down Expand Up @@ -71,9 +70,10 @@ const PlotsContent = () => {
<AddPlots
hasUnselectedPlots={hasUnselectedPlots}
hasNoCustomPlots={hasNoCustomPlots}
cliError={cliError}
/>
}
showEmpty={!hasPlots}
showEmpty={!hasPlots && !cliError}
welcome={<Welcome />}
isFullScreen={hasNoCustomPlots}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import { ComparisonPlot } from 'dvc/src/plots/webview/contract'
import styles from './styles.module.scss'
import { RefreshButton } from '../../../shared/components/button/RefreshButton'
import { refreshRevisions, zoomPlot } from '../messages'
import { Error } from '../../../shared/components/icons'
import { ErrorTooltip } from '../../../shared/components/tooltip/ErrorTooltip'
import { ErrorIcon } from '../ErrorIcon'

type ComparisonTableCellProps = {
path: string
Expand All @@ -15,11 +14,9 @@ const MissingPlotTableCell: React.FC<{ plot: ComparisonPlot }> = ({ plot }) => (
<div className={styles.noImageContent}>
{plot.errors?.length ? (
<>
<ErrorTooltip error={plot.errors.join('\n')}>
<div>
<Error height={48} width={48} className={styles.errorIcon} />
</div>
</ErrorTooltip>
<div className={styles.errorIcon}>
<ErrorIcon error={plot.errors.join('\n')} size={48} />
</div>
<RefreshButton onClick={refreshRevisions} />
</>
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ $gap: 4px;
}

.errorIcon {
color: $error-color;
margin: 6px;
}

Expand Down
4 changes: 4 additions & 0 deletions webview/src/plots/components/styles.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,10 @@ $gap: 20px;
}
}

.errorIcon {
color: $error-color;
}

.zoomablePlot {
display: block;
width: 100%;
Expand Down
16 changes: 16 additions & 0 deletions webview/src/plots/components/webviewSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ type ZoomedInPlotState = {
refresh?: boolean
}
export interface WebviewState {
cliError: string | undefined
hasData: boolean
hasPlots: boolean
hasUnselectedPlots: boolean
Expand All @@ -18,6 +19,7 @@ export interface WebviewState {
}

export const webviewInitialState: WebviewState = {
cliError: undefined,
hasData: false,
hasPlots: false,
hasUnselectedPlots: false,
Expand Down Expand Up @@ -61,6 +63,19 @@ export const webviewSlice = createSlice({
Object.assign(state.zoomedInPlot, action.payload)
}
},
updateCliError: (
state: { cliError: string | undefined },
action: PayloadAction<string | undefined | null>
) => {
if (action.payload === undefined) {
return
}
if (action.payload === null) {
state.cliError = undefined
return
}
state.cliError = action.payload
},
updateHasPlots: (
state: { hasPlots: boolean },
action: PayloadAction<boolean>
Expand All @@ -84,6 +99,7 @@ export const webviewSlice = createSlice({

export const {
initialize,
updateCliError,
updateHasPlots,
updateHasUnselectedPlots,
updateSelectedRevisions,
Expand Down
Loading

0 comments on commit dc234f5

Please sign in to comment.