fix: Resolve the issue of sub canvas drift

[shaohuzhang1]

fix: Resolve the issue of sub canvas drift

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

The code contains several irregularities and improvements that should be considered:

1. Scope Issues: The HtmlPointToCanvasPoint function is trying to access the transformModel properties of the lf.value.nodeModel.graphModel, which might not exist if these values are accessed outside its intended scope.

2. Function Redundancy: The line props.nodeModel.graphModel.transformModel.props = graphModelTransformProps; seems redundant given it's already assigning the same value. It needs further review or simplification based on actual requirements.

3. Potential Security Risks: The code does not include checks for security risks such as input sanitization when handling potentially untrusted data like HTML points.

4. Optimization Suggestions:

   • If scaleX, scaleY, translateX, and translateY are constant throughout rendering, consider declaring them once as variables before calling HtmlPointToCanvasPoint.
   • Use functional programming practices wherever possible, especially with arrow functions and template literals, to improve readability and maintainability. For example, using (scaleFactor * currentScale) / anotherScale instead of multiplying twice could make the logic clearer.

Here is an improved version of the code incorporating some of these optimizations:

const renderGraphData = (data?: any) => {
  // Existing code...
}

function HtmlPointToCanvasPoint(point: { x: number, y: number }): { x: number, y: number } | null {
  if (!point || !point.x || !point.y) {
    return null;
  }

  const transform = lf.value.graphModel.transformModel;

  let scale = transform.SCALE ?? 1;
  let translate = transform.TRANSLATE ?? { x: 0, y: 0 };

  const canvasPoint = [
    ((point.x - translate.x) / scale),
    ((point.y - translate.y) / scale),
  ];

  return canvasPoint;
}

// Bind the function properly without overriding existing prototype property
if (!('HtmlPointToCanvasPoint' in lf.value.graphModel.transformModel)) {
  Object.assign(lf.value.graphModel.transformModel, { HtmlPointToCanvasPoint });
}
lf.value.graphModel.transformModel.HtmlPointToCanvasPoint = HtmlPointToCanvasPoint.bind(
  lf.value.graphModel.transformModel,
);

initDefaultShortcut(lf.value, lf.value.graphModel);

In this revised version:

• Added type checking for point parameters in HtmlPointToCanvasPoint.
• Simplified scaling operations within the function.
• Ensured proper binding of the function to transformModel without overwriting any existing method.