Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add webview for when dvc is not available or not initialized #2861

Merged
merged 12 commits into from
Dec 1, 2022
12 changes: 6 additions & 6 deletions extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1306,11 +1306,6 @@
}
],
"dvc-views": [
{
"id": "dvc.views.webviews",
"name": "Views",
"when": "dvc.commands.available && dvc.project.available"
},
{
"id": "dvc.views.experimentsColumnsTree",
"name": "Columns",
Expand All @@ -1321,6 +1316,11 @@
"name": "Welcome",
"when": "!dvc.commands.available || dvc.cli.incompatible || !dvc.project.available"
},
{
"id": "dvc.views.webviews",
"name": "Views",
"when": "true"
},
{
"id": "dvc.views.experimentsTree",
"name": "Experiments",
Expand Down Expand Up @@ -1351,7 +1351,7 @@
{
"view": "dvc.views.webviews",
"contents": "[Show Experiments](command:dvc.showExperiments)\n[Show Plots](command:dvc.showPlots)\n[Show Experiments and Plots](command:dvc.showExperimentsAndPlots)",
"when": "dvc.commands.available && dvc.project.available"
"when": "true"
Copy link
Member

@mattseddon mattseddon Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Q] Would it be easier to show the same view but with different underlying commands under different circumstances? Would that simplify the code elsewhere?

i.e

      {
        "view": "dvc.views.webviews",
        "contents": "[Show Experiments](command:dvc.showExperiments)\n[Show Plots](command:dvc.showPlots)\n[Show Experiments and Plots](command:dvc.showExperimentsAndPlots)",
        "when": "dvc.commands.available && dvc.project.available"
      },
      {
        "view": "dvc.views.webviews",
        "contents": "[Show Experiments](command:dvc.showEmptyWebview)\n[Show Plots](command:dvc.showEmptyWebview)\n[Show Experiments and Plots](command:dvc.showEmptyWebview)",
        "when": "!dvc.commands.available || !dvc.project.available"
     }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right. I'll try that. Thanks!

},
{
"view": "dvc.views.welcome",
Expand Down
3 changes: 3 additions & 0 deletions extension/src/experiments/workspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { buildMockedEventEmitter } from '../test/util/jest'
import { OutputChannel } from '../vscode/outputChannel'
import { Title } from '../vscode/title'
import { Args } from '../cli/dvc/constants'
import { ResourceLocator } from '../resourceLocator'

const mockedShowWebview = jest.fn()
const mockedDisposable = jest.mocked(Disposable)
Expand Down Expand Up @@ -60,6 +61,8 @@ describe('Experiments', () => {
mockedInternalCommands,
mockedUpdatesPaused,
buildMockMemento(),
{} as ResourceLocator,
() => true,
{
'/my/dvc/root': {
getDvcRoot: () => mockedDvcRoot,
Expand Down
10 changes: 9 additions & 1 deletion extension/src/experiments/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,18 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews<
internalCommands: InternalCommands,
updatesPaused: EventEmitter<boolean>,
workspaceState: Memento,
resourceLocator: ResourceLocator,
getAvailable: () => boolean,
experiments?: Record<string, Experiments>,
checkpointsChanged?: EventEmitter<void>
) {
super(internalCommands, workspaceState, experiments)
super(
internalCommands,
workspaceState,
resourceLocator,
getAvailable,
experiments
)

this.updatesPaused = updatesPaused

Expand Down
15 changes: 13 additions & 2 deletions extension/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,19 @@ export class Extension extends Disposable implements IExtension {
new WorkspaceExperiments(
this.internalCommands,
this.updatesPaused,
context.workspaceState
context.workspaceState,
this.resourceLocator,
() => this.getAvailable()
)
)

this.plots = this.dispose.track(
new WorkspacePlots(this.internalCommands, context.workspaceState)
new WorkspacePlots(
this.internalCommands,
context.workspaceState,
this.resourceLocator,
() => this.getAvailable()
)
)

this.repositories = this.dispose.track(
Expand Down Expand Up @@ -433,6 +440,10 @@ export class Extension extends Disposable implements IExtension {
return available
}

public getAvailable() {
return this.cliAccessible
}

private setCommandsAvailability(available: boolean) {
setContextValue('dvc.commands.available', available)
}
Expand Down
8 changes: 8 additions & 0 deletions extension/src/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ export const EventName = Object.assign(
VIEWS_EXPERIMENTS_TABLE_SORT_COLUMN:
'views.experimentsTable.columnSortAdded',

VIEWS_GET_STARTED_CLOSE: 'view.GET_STARTED.closed',
VIEWS_GET_STARTED_CREATED: 'view.GET_STARTED.created',
VIEWS_GET_STARTED_FOCUS_CHANGED: 'views.GET_STARTED.focusChanged',

VIEWS_PLOTS_CLOSED: 'views.plots.closed',
VIEWS_PLOTS_COMPARISON_ROWS_REORDERED:
'views.plots.comparisonRowsReordered',
Expand Down Expand Up @@ -252,4 +256,8 @@ export interface IEventNamePropertyMapping {
[EventName.VIEWS_TERMINAL_CREATED]: undefined

[EventName.VIEWS_TRACKED_EXPLORER_TREE_OPENED]: DvcRootCount

[EventName.VIEWS_GET_STARTED_CLOSE]: undefined
[EventName.VIEWS_GET_STARTED_CREATED]: undefined
[EventName.VIEWS_GET_STARTED_FOCUS_CHANGED]: undefined
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
FilterItem
} from '../../../../../experiments/model/filterBy/tree'
import { starredFilter } from '../../../../../experiments/model/filterBy/constants'
import { ResourceLocator } from '../../../../../resourceLocator'

suite('Experiments Filter By Tree Test Suite', () => {
const disposable = Disposable.fn()
Expand Down Expand Up @@ -357,6 +358,8 @@ suite('Experiments Filter By Tree Test Suite', () => {
internalCommands,
disposable.track(new EventEmitter()),
buildMockMemento(),
{} as ResourceLocator,
() => true,
{ [dvcDemoPath]: experiments },
disposable.track(new EventEmitter())
)
Expand Down
7 changes: 6 additions & 1 deletion extension/src/test/suite/experiments/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { FileSystemData } from '../../../fileSystem/data'
import * as Watcher from '../../../fileSystem/watcher'
import { ExperimentsModel } from '../../../experiments/model'
import { ColumnsModel } from '../../../experiments/columns/model'
import { ResourceLocator } from '../../../resourceLocator'

const hasCheckpoints = (data: ExperimentsOutput) => {
const [experimentsWithBaseline] = Object.values(omit(data, 'workspace'))
Expand Down Expand Up @@ -104,6 +105,8 @@ export const buildMultiRepoExperiments = (disposer: Disposer) => {
internalCommands,
updatesPaused,
buildMockMemento(),
resourceLocator,
() => true,
{
'other/dvc/root': mockExperiments
}
Expand Down Expand Up @@ -132,7 +135,9 @@ export const buildSingleRepoExperiments = (disposer: Disposer) => {
new WorkspaceExperiments(
internalCommands,
updatesPaused,
buildMockMemento()
buildMockMemento(),
{} as ResourceLocator,
() => true
)
)
const [experiments] = workspaceExperiments.create(
Expand Down
23 changes: 21 additions & 2 deletions extension/src/webview/constants.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
import { distPath, react, experiments, plots } from 'dvc-vscode-webview'
import {
distPath,
react,
experiments,
plots,
getStarted
} from 'dvc-vscode-webview'
import { EventName, IEventNamePropertyMapping } from '../telemetry/constants'

export enum ViewKey {
EXPERIMENTS = 'dvc-experiments',
PLOTS = 'dvc-plots'
PLOTS = 'dvc-plots',
GET_STARTED = 'dvc-getStarted'
}

type Name = keyof IEventNamePropertyMapping
Expand Down Expand Up @@ -46,5 +53,17 @@ export const WebviewDetails: {
scripts: [react, plots],
title: 'Plots',
viewKey: ViewKey.PLOTS
},
[ViewKey.GET_STARTED]: {
contextKey: 'dvc.getStarted.webviewActive',
distPath,
eventNames: {
closedEvent: EventName.VIEWS_GET_STARTED_CLOSE,
createdEvent: EventName.VIEWS_GET_STARTED_CREATED,
focusChangedEvent: EventName.VIEWS_GET_STARTED_FOCUS_CHANGED
},
scripts: [react, getStarted],
title: 'DVC is not initialized',
viewKey: ViewKey.GET_STARTED
}
} as const
10 changes: 6 additions & 4 deletions extension/src/webview/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ export const isValidDvcRoot = (dvcRoot?: string): dvcRoot is string => !!dvcRoot
const create = (
viewKey: ViewKey,
webviewPanel: WebviewPanel,
dvcRoot: string
dvcRoot: string,
bypassDvcRoot?: boolean
) => {
if (!isValidDvcRoot(dvcRoot)) {
if (!bypassDvcRoot && !isValidDvcRoot(dvcRoot)) {
throw new Error(`trying to set invalid state into ${viewKey}`)
}

Expand All @@ -25,7 +26,8 @@ export const createWebview = async (
viewKey: ViewKey,
dvcRoot: string,
iconPath: Resource,
viewColumn?: ViewColumn
viewColumn?: ViewColumn,
bypassDvcRoot?: boolean
) => {
const { title, distPath } = WebviewDetails[viewKey]

Expand All @@ -42,7 +44,7 @@ export const createWebview = async (

webviewPanel.iconPath = iconPath

const view = create(viewKey, webviewPanel, dvcRoot)
const view = create(viewKey, webviewPanel, dvcRoot, bypassDvcRoot)
await view.isReady()
return view
}
Expand Down
28 changes: 28 additions & 0 deletions extension/src/webview/workspace.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { Memento, ViewColumn } from 'vscode'
import { BaseRepository } from './repository'
import { WebviewData } from './contract'
import { createWebview } from './factory'
import { ViewKey } from './constants'
import { InternalCommands } from '../commands/internal'
import { BaseWorkspace } from '../workspace'
import { ResourceLocator } from '../resourceLocator'

export abstract class BaseWorkspaceWebviews<
T extends BaseRepository<U>,
Expand All @@ -12,22 +15,37 @@ export abstract class BaseWorkspaceWebviews<

protected focusedWebviewDvcRoot: string | undefined

private resourceLocator: ResourceLocator
private getAvailable: () => boolean

constructor(
internalCommands: InternalCommands,
workspaceState: Memento,
resourceLocator: ResourceLocator,
getAvailable: () => boolean,
repositories?: Record<string, T>
) {
super(internalCommands)

this.workspaceState = workspaceState

this.resourceLocator = resourceLocator

this.getAvailable = getAvailable

if (repositories) {
this.repositories = repositories
}
}

public async showWebview(overrideRoot: string, viewColumn?: ViewColumn) {
if (!this.getAvailable() || this.getDvcRoots().length === 0) {
this.showEmptyWebview(viewColumn)
return
}

const dvcRoot = overrideRoot || (await this.getOnlyOrPickProject())

if (!dvcRoot) {
return
}
Expand All @@ -48,5 +66,15 @@ export abstract class BaseWorkspaceWebviews<
return overrideRoot || (await this.getFocusedOrOnlyOrPickProject())
}

protected async showEmptyWebview(viewColumn?: ViewColumn) {
await createWebview(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a simple webview for now, but I've already started creating a class for it to add messages.

ViewKey.GET_STARTED,
'',
this.resourceLocator.dvcIcon,
viewColumn,
true
)
}

abstract getFocusedOrOnlyOrPickProject(): string | Promise<string | undefined>
}
3 changes: 2 additions & 1 deletion webview/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ module.exports = {
files: [
'src/experiments/index.tsx',
'src/plots/index.tsx',
'src/shared/components/icons/index.ts'
'src/shared/components/icons/index.ts',
'src/getStarted/index.tsx'
],
rules: {
'check-file/no-index': 'off'
Expand Down
1 change: 1 addition & 0 deletions webview/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export const distPath: string
export const experiments: string
export const plots: string
export const getStarted: string
export const react: string
1 change: 1 addition & 0 deletions webview/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ const path = require('path')
module.exports.distPath = path.join(__dirname, 'dist')
module.exports.experiments = path.join(__dirname, 'dist/experiments.js')
module.exports.plots = path.join(__dirname, 'dist/plots.js')
module.exports.getStarted = path.join(__dirname, 'dist/getStarted.js')
module.exports.react = path.join(__dirname, 'dist/react.js')
12 changes: 12 additions & 0 deletions webview/src/getStarted/components/App.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import React from 'react'
import { EmptyState } from '../../shared/components/emptyState/EmptyState'

export const App: React.FC = () => {
return (
<EmptyState>
<div>
<h1>DVC is not available or not initialized</h1>
</div>
</EmptyState>
)
}
7 changes: 7 additions & 0 deletions webview/src/getStarted/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import React from 'react'
import ReactDOM from 'react-dom/client'
import '../shared/style.scss'
import { App } from './components/App'

const root = ReactDOM.createRoot(document.querySelector('#root') as HTMLElement)
root.render(<App />)
6 changes: 5 additions & 1 deletion webview/webpack.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ export default {
entry: {
experiments: { dependOn: 'react', import: r('src/experiments/index.tsx') },
plots: { dependOn: 'react', import: r('src/plots/index.tsx') },
react: ['react', 'react-dom']
react: ['react', 'react-dom'],
getStarted: {
dependOn: 'react',
import: r('src/getStarted/index.tsx')
}
},
module: {
rules: [
Expand Down