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: Workflow tools #570

Merged
merged 31 commits into from
Sep 30, 2024
Merged

feat: Workflow tools #570

merged 31 commits into from
Sep 30, 2024

Conversation

ramedina86
Copy link
Collaborator

No description provided.

@ramedina86
Copy link
Collaborator Author

@madeindjs You always share some good advice :) Happy to provide more context for these features if necessary.

Comment on lines +247 to +252
setTimeout(() => {
// Debouncing
if (component.x !== newX) return;
if (component.y !== newY) return;
changeCoordinates(componentId, newX, newY);
}, 200);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's a debouncer you want, you might need to cancel the previous setTimeout using a method like

function useDebouncer<Args extends Array<unknown>, Return>(
  callback: (...args: Args) => void,
  ms: number,
): (...args: Args) => void {
  let id: ReturnType<typeof setTimeout>;

  return (...args: Args) => {
    if (id) clearTimeout(id);
    id = setTimeout(() => callback(...args), ms);
  };
}

const changeCoordinatesDebounced = useDeboucer(changeCoordinates, 200);

and then

Suggested change
setTimeout(() => {
// Debouncing
if (component.x !== newX) return;
if (component.y !== newY) return;
changeCoordinates(componentId, newX, newY);
}, 200);
changeCoordinatesDebounced(componentId, newX, newY)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes that was the other alternative but I wanted to avoid having a single timer id for all components. So I'd have needed Record<Component["id"], ReturnType<typeof setTimeout>> and I wanted to avoid.

However, in the example you declare id inside the function. If it works that'd be great, but how would that behave? Wouldn't future calls get a completely new id since it's scoped to the function?

src/ui/src/components/workflows/base/WorkflowArrow.vue Outdated Show resolved Hide resolved
postChildren.forEach((c) => {
if (!c.outs || c.outs.length == 0) return;
c.outs = c.outs.filter((out) => out.toNodeId !== removedId);
await nextTick();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this nextTick necessary ? It's a bit dangerous to use it inside a watch, no ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because I need to have everything reflected on the DOM before I call refreshArrows, since this function calls getBoundingClientRect() to get precise coordinates.

src/ui/src/components/workflows/WorkflowsWorkflow.vue Outdated Show resolved Hide resolved
src/ui/src/builder/useComponentActions.ts Outdated Show resolved Hide resolved
src/ui/src/builder/useComponentActions.ts Outdated Show resolved Hide resolved
src/ui/src/builder/useComponentActions.ts Outdated Show resolved Hide resolved
@ramedina86 ramedina86 merged commit 0f556ad into dev Sep 30, 2024
18 checks passed
@ramedina86 ramedina86 deleted the feat-workflow-tools1 branch September 30, 2024 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants