containers / podman-desktop

Podman Desktop - A graphical tool for developing on containers and Kubernetes
https://podman-desktop.io
Apache License 2.0
4.33k stars 275 forks source link

Tasks Epic #7401

Open deboer-tim opened 1 month ago

deboer-tim commented 1 month ago

Epic domain

There are a number of issues related to task UI/usability and technical debt. They are not all related and do not all need to be done in order or at the same time, but opening this epic to keep track off all the issues.

Issues:

Will list other task-related issues here:

Additional context

No response

axel7083 commented 2 days ago

Proposal

Currently tasks are under 3 forms

https://github.com/containers/podman-desktop/blob/cfeb27546799febf3383b236e5602bbcee204970/packages/api/src/task.ts#L22

https://github.com/containers/podman-desktop/blob/cfeb27546799febf3383b236e5602bbcee204970/packages/api/src/task.ts#L28

https://github.com/containers/podman-desktop/blob/cfeb27546799febf3383b236e5602bbcee204970/packages/api/src/task.ts#L40

I am not sure of what is the difference between a NotificationTask and a StatefulTask but my proposal would be related to the action system. I am wondering if we can get rid of those differents interface, and only keep the Task.

The action and gotoTask properties should be completely removed from the Task, I would probably rename the interface to differentiate the interface on the main and renderer package.

My proposal would be to enhance the Task by creating the following

// packages/api/src/task.ts
export interface TaskInfo {
    id: string;
    action?: name;
    name: string;
    started: number;
}

On the backend side, we could create a class TaskImpl similar to the CancellationTokenImpl.

export interface TaskCreationOptions {
    name: string;
    action?: { name: string, callback: () => void },
    progress?: undefined,
    ...
}

export class TaskImpl {
    constructor(options: TaskCreationOptions ) {...}

    public onAction(): {
         this.callback?.();
    }

    public toTaskInfo(): TaskInfo  { ... }
}

The TaskImpl would only be stored on the main package, and would have a public method toTaskInfo required to send it to the renderer.

The renderer would probably have some new kind of method, like windows.executeTaskAction(task.id);. The task registry, holding the TaskImpl would get the corresponding TaskImpl and cal the appropriate methods.

cc @deboer-tim @benoitf @jeffmaury

deboer-tim commented 2 days ago

I am not sure of what is the difference between a NotificationTask and a StatefulTask but my proposal would be related to the action system. I am wondering if we can get rid of those differents interface, and only keep the Task.

IIRC correctly notifications are literally just that, e.g. if you wanted to tell the user that something happened unrelated to a task, e.g. 'Lost network connection'. I think we still need two related things and there is overlap, but interface could change.

The action and gotoTask properties should be completely removed from the Task, I would probably rename the interface to differentiate the interface on the main and renderer package.

I'm ok with that, but as per the discussion in #7799 I'm not sure what mechanism would replace it. We don't need uber PR that does everything at once, but there needs to be some path/understanding of how we get to a replacement.

My proposal would be to enhance the Task by creating the following

// packages/api/src/task.ts
export interface TaskInfo {
    id: string;
    action?: name;
    name: string;
    started: number;
}

It's not clear what the type of action is.

started is a number? It's not clear if you want to handle state and progress in the same variable but I think I would split it with optional progress.

On the backend side, we could create a class TaskImpl similar to the CancellationTokenImpl.

export interface TaskCreationOptions {
    name: string;
    action?: { name: string, callback: () => void },
    progress?: undefined,
    ...
}

export class TaskImpl {
    constructor(options: TaskCreationOptions ) {...}

    public onAction(): {
         this.callback?.();
    }

    public toTaskInfo(): TaskInfo  { ... }
}

The TaskImpl would only be stored on the main package, and would have a public method toTaskInfo required to send it to the renderer.

The renderer would probably have some new kind of method, like windows.executeTaskAction(task.id);. The task registry, holding the TaskImpl would get the corresponding TaskImpl and cal the appropriate methods.

Is this just to trigger the action related to a task? From an API standpoint it sounds ok, but since the action is likely something like 'show this build on the Build form' it's not clear to me where the action would live and the separation between main and renderer. It almost feels to me like build/pulls task should be entirely created/updated by main and renderer would register the action independently like a 'hey, for that type of task you can see details here'.

axel7083 commented 1 day ago

It's not clear what the type of action is.

Instead of having the following

https://github.com/containers/podman-desktop/blob/cfeb27546799febf3383b236e5602bbcee204970/packages/api/src/task.ts#L33-L36

We would only define the name, and having the name define would mean there is an action linked, so the renderer can check if action is !== undefined to display it, and then call the main package with windows.executeTaskAction.

started is a number? It's not clear if you want to handle state and progress in the same variable but I think I would split it with optional progress.

I just copy past the existing interface

https://github.com/containers/podman-desktop/blob/cfeb27546799febf3383b236e5602bbcee204970/packages/api/src/task.ts#L25

Is this just to trigger the action related to a task? From an API standpoint it sounds ok, but since the action is likely something like 'show this build on the Build form' it's not clear to me where the action would live and the separation between main and renderer.

*never on the renderer this is the place I am standing, there is no action on the renderer side. If I want the build task to redirect to the build page, it should do something like

// main package
createTask({
  action: {
      name: 'Goto build page',
      execute: () => navigationManager.navigateToBuild(...)
  }
  ...
} as TaskCreationOptions 

The creator of the task, should define the action, and if it needs to redirect to some page, it should use the navigation API.