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

feat: allowing extension to provide routeId to progress task #8902

Merged
merged 3 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions packages/extension-api/src/extension-api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,39 @@ declare module '@podman-desktop/api' {
* button.
*/
cancellable?: boolean;

/**
* You may specify a navigation object, making the task having a
* navigate action that the user can trigger.
* @example
* ```ts
* import { window, type ProgressLocation } from '@podman-desktop/api';
*
* await window.withProgress<string>(
* {
* location: ProgressLocation.TASK_WIDGET,
* title: 'My task',
* details: {
* routeId: 'dummy-route-id',
* routeArgs: ['hello', 'world'],
* }
* },
* async () => {
* return 'dummy result';
* },
* );
* ```
*/
details?: {
/**
* The routeId used in {@link navigation.register}
*/
routeId: string;
/**
* The arguments to provide the route
*/
routeArgs: string[];
};
}

/**
Expand Down
45 changes: 43 additions & 2 deletions packages/main/src/plugin/extension-loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import type { Proxy } from './proxy.js';
import type { ExtensionSecretStorage, SafeStorageRegistry } from './safe-storage/safe-storage-registry.js';
import type { StatusBarRegistry } from './statusbar/statusbar-registry.js';
import type { NotificationRegistry } from './tasks/notification-registry.js';
import type { ProgressImpl } from './tasks/progress-impl.js';
import { type ProgressImpl, ProgressLocation } from './tasks/progress-impl.js';
import type { Telemetry } from './telemetry/telemetry.js';
import type { TrayMenuRegistry } from './tray-menu-registry.js';
import type { IDisposable } from './types/disposable.js';
Expand Down Expand Up @@ -141,7 +141,9 @@ const trayMenuRegistry: TrayMenuRegistry = {} as unknown as TrayMenuRegistry;

const messageBox: MessageBox = {} as MessageBox;

const progress: ProgressImpl = {} as ProgressImpl;
const progress: ProgressImpl = {
withProgress: vi.fn(),
} as unknown as ProgressImpl;

const statusBarRegistry: StatusBarRegistry = {} as unknown as StatusBarRegistry;

Expand Down Expand Up @@ -2286,6 +2288,45 @@ test('when registering a navigation route, should be pushed to disposables', ()
expect(disposables.length).toBe(1);
});

test('withProgress should add the extension id to the routeId', async () => {
vi.mocked(progress.withProgress).mockResolvedValue(undefined);
const disposables: IDisposable[] = [];

const api = extensionLoader.createApi(
'/path',
{
publisher: 'pub',
name: 'dummy',
},
disposables,
);
expect(api).toBeDefined();

await api.window.withProgress<void>(
{
location: ProgressLocation.TASK_WIDGET,
title: 'Dummy title',
details: {
routeId: 'dummy-route-id',
routeArgs: ['hello', 'world'],
},
},
async () => {},
);

expect(progress.withProgress).toHaveBeenCalledWith(
{
location: ProgressLocation.TASK_WIDGET,
title: 'Dummy title',
details: {
routeId: 'pub.dummy.dummy-route-id',
routeArgs: ['hello', 'world'],
},
},
expect.any(Function),
);
});

describe('loading extension folders', () => {
const fileEntry = {
isDirectory: () => false,
Expand Down
13 changes: 12 additions & 1 deletion packages/main/src/plugin/extension-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,18 @@ export class ExtensionLoader {
token: containerDesktopAPI.CancellationToken,
) => Promise<R>,
): Promise<R> => {
return progress.withProgress(options, task);
return progress.withProgress(
{
...options,
details: options.details
? {
routeArgs: options.details.routeArgs,
routeId: `${extensionInfo.id}.${options.details.routeId}`,
}
: undefined,
},
task,
);
},

showNotification: (notificationInfo: containerDesktopAPI.NotificationOptions): containerDesktopAPI.Disposable => {
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ export class PluginSystem {
apiSender,
trayMenuRegistry,
messageBox,
new ProgressImpl(taskManager),
new ProgressImpl(taskManager, navigationManager),
statusBarRegistry,
kubernetesClient,
fileSystemMonitoring,
Expand Down
56 changes: 51 additions & 5 deletions packages/main/src/plugin/tasks/progress-impl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import type { Event } from '@podman-desktop/api';
import { beforeEach, expect, test, vi } from 'vitest';

import type { NavigationManager } from '/@/plugin/navigation/navigation-manager.js';
import type { Task, TaskAction, TaskUpdateEvent } from '/@/plugin/tasks/tasks.js';
import type { TaskState, TaskStatus } from '/@api/taskInfo.js';

Expand All @@ -31,6 +32,11 @@ const taskManager = {
createTask: vi.fn(),
} as unknown as TaskManager;

const navigationManager = {
hasRoute: vi.fn(),
navigateToRoute: vi.fn(),
} as unknown as NavigationManager;

class TestTaskImpl implements Task {
constructor(
public readonly id: string,
Expand Down Expand Up @@ -62,7 +68,7 @@ test('Should create a task and report update', async () => {
const task = new TestTaskImpl('test-task-id', 'test-title', 'running', 'in-progress');
vi.mocked(taskManager.createTask).mockReturnValue(task);

const progress = new ProgressImpl(taskManager);
const progress = new ProgressImpl(taskManager, navigationManager);
await progress.withProgress({ location: ProgressLocation.TASK_WIDGET, title: 'My task' }, async () => 0);

expect(task.status).toBe('success');
Expand All @@ -72,7 +78,7 @@ test('Should create a task and report progress', async () => {
const task = new TestTaskImpl('test-task-id', 'test-title', 'running', 'in-progress');
vi.mocked(taskManager.createTask).mockReturnValue(task);

const progress = new ProgressImpl(taskManager);
const progress = new ProgressImpl(taskManager, navigationManager);
await progress.withProgress({ location: ProgressLocation.TASK_WIDGET, title: 'My task' }, async progress => {
progress.report({ increment: 50 });
});
Expand All @@ -85,7 +91,7 @@ test('Should create a task and propagate the exception', async () => {
const task = new TestTaskImpl('test-task-id', 'test-title', 'running', 'in-progress');
vi.mocked(taskManager.createTask).mockReturnValue(task);

const progress = new ProgressImpl(taskManager);
const progress = new ProgressImpl(taskManager, navigationManager);

await expect(
progress.withProgress({ location: ProgressLocation.TASK_WIDGET, title: 'My task' }, async () => {
Expand All @@ -101,7 +107,7 @@ test('Should create a task and propagate the result', async () => {
const task = new TestTaskImpl('test-task-id', 'test-title', 'running', 'in-progress');
vi.mocked(taskManager.createTask).mockReturnValue(task);

const progress = new ProgressImpl(taskManager);
const progress = new ProgressImpl(taskManager, navigationManager);

const result: string = await progress.withProgress<string>(
{ location: ProgressLocation.TASK_WIDGET, title: 'My task' },
Expand All @@ -118,7 +124,7 @@ test('Should update the task name', async () => {
const task = new TestTaskImpl('test-task-id', 'test-title', 'running', 'in-progress');
vi.mocked(taskManager.createTask).mockReturnValue(task);

const progress = new ProgressImpl(taskManager);
const progress = new ProgressImpl(taskManager, navigationManager);

await progress.withProgress<void>({ location: ProgressLocation.TASK_WIDGET, title: 'My task' }, async progress => {
progress.report({ message: 'New title' });
Expand All @@ -127,3 +133,43 @@ test('Should update the task name', async () => {
expect(task.name).toBe('New title');
expect(task.status).toBe('success');
});

test('Should create a task with a navigation action', async () => {
vi.mocked(navigationManager.hasRoute).mockReturnValue(true);

const task = new TestTaskImpl('test-task-id', 'test-title', 'running', 'in-progress');
const progress = new ProgressImpl(taskManager, navigationManager);

let taskAction: TaskAction | undefined;
vi.mocked(taskManager.createTask).mockImplementation(options => {
taskAction = options?.action;
return task;
});

await progress.withProgress<string>(
{
location: ProgressLocation.TASK_WIDGET,
title: 'My task',
details: {
routeId: 'dummy-route-id',
routeArgs: ['hello', 'world'],
},
},
async () => {
return 'dummy result';
},
);

await vi.waitFor(() => {
expect(taskAction).toBeDefined();
});

expect(taskAction?.name).toBe('View');
expect(taskAction?.execute).toBeInstanceOf(Function);

// execute the task action
taskAction?.execute(task);

// ensure the arguments and routeId is properly used
expect(navigationManager.navigateToRoute).toHaveBeenCalledWith('dummy-route-id', 'hello', 'world');
});
29 changes: 27 additions & 2 deletions packages/main/src/plugin/tasks/progress-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import type * as extensionApi from '@podman-desktop/api';

import { findWindow } from '/@/electron-util.js';
import type { NavigationManager } from '/@/plugin/navigation/navigation-manager.js';
import type { TaskAction } from '/@/plugin/tasks/tasks.js';

import { CancellationTokenImpl } from '../cancellation-token.js';
import type { TaskManager } from './task-manager.js';
Expand All @@ -35,7 +37,10 @@ export enum ProgressLocation {
}

export class ProgressImpl {
constructor(private taskManager: TaskManager) {}
constructor(
private taskManager: TaskManager,
private navigationManager: NavigationManager,
) {}

/**
* Execute a task with progress, based on the provided options and task function.
Expand Down Expand Up @@ -78,14 +83,34 @@ export class ProgressImpl {
);
}

protected getTaskAction(options: extensionApi.ProgressOptions): TaskAction | undefined {
if (!options.details) return undefined;

if (!this.navigationManager.hasRoute(options.details.routeId)) {
console.warn(`cannot created task action for unknown routeId ${options.details.routeId}`);
return undefined;
}

return {
name: 'View',
Copy link
Collaborator

@benoitf benoitf Sep 17, 2024

Choose a reason for hiding this comment

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

I still believe View details or Inspect is more accurate but as it's not part of the spec (.d.ts file), it can be easily changed over time (or not) later.

can merge and discuss it during UX discussions

execute: (): unknown => {
if (!options.details) return;
return this.navigationManager.navigateToRoute(options.details.routeId, ...options.details.routeArgs);
},
};
}

async withWidget<R>(
options: extensionApi.ProgressOptions,
task: (
progress: extensionApi.Progress<{ message?: string; increment?: number }>,
token: extensionApi.CancellationToken,
) => Promise<R>,
): Promise<R> {
const t = this.taskManager.createTask({ title: options.title });
const t = this.taskManager.createTask({
title: options.title,
action: this.getTaskAction(options),
});

return task(
{
Expand Down