Reviewing files that changed from the base of the PR and between c72ac6e and 22fc76e.

• packages/core/__tests__/view/plugins/FitPlugin.test.ts (1 hunks)

• GitHub Check: build (windows-2022)
• GitHub Check: build (ubuntu-22.04)

packages/core/__tests__/view/plugins/FitPlugin.test.ts (11)

18-18: LGTM: Appropriate import addition.

The Rectangle import is correctly added to support the new test cases that need to set explicit graph bounds.

---

20-49: Well-designed helper function with good reusability.

The createContainer function effectively creates mock DOM elements with configurable dimension properties. The conditional property setting and use of Object.defineProperty with configurable: true are appropriate for testing scenarios.

---

51-60: Good baseline test for zero dimensions scenario.

This test properly covers the edge case where graph dimensions are zero, verifying that the scale defaults to 1 and the expected translation values are applied.

---

78-92: Complex increased scale scenario with margin option.

This test covers scaling up with a custom margin. The expected values (scale: 2.46, translation: 217, -59) should be verified against the fitCenter implementation to ensure the calculation logic is correct.

---

94-111: Good test for maxFitScale constraint.

This test properly verifies that the maxFitScale property limits the scaling when the calculated scale would exceed the maximum. The setup and assertions are clear and appropriate.

---

113-121: Appropriate edge case test for fit method.

This test correctly handles the scenario where both container and graph have zero dimensions, expecting no call to scaleAndTranslate which aligns with the fit method's early return logic.

---

123-137: Test covers basic fit scaling scenario.

The test uses offsetWidth/offsetHeight properties which are correct for the fit method. The expected scale and translation values should be validated against the fit implementation's complex calculation logic.

---

139-155: Good test for minFitScale constraint.

This test effectively verifies the minFitScale property by using zero container dimensions which would normally result in an invalid scale, but the constraint ensures it's limited to 0.5.

---

157-171: Test covers increased scale scenario for fit method.

Similar to the fitCenter tests, this covers scaling up when the container is larger than the graph. The calculations should be verified for accuracy.

---

173-189: Comprehensive test for maxFitScale in fit method.

This test properly verifies that maxFitScale constraints are respected in the fit method, maintaining consistency with the fitCenter tests.

---

62-76:

Details

❌ Incorrect review comment

Verify the scale calculation for decreased scale scenario.

The test expects a scale of 0.02, but let me verify this calculation:

• Container client dimensions: 20x20
• Default margin: 2
• Effective dimensions: (20-2*2) = 16x16
• Graph dimensions: 1000x1000
• Expected scale: min(16/1000, 16/1000) = 0.016

The expected scale of 0.02 seems inconsistent with this calculation.

---

🏁 Script executed:

#!/bin/bash
# Verify the fitCenter calculation logic in the FitPlugin implementation
echo "Checking fitCenter scale calculation logic..."
ast-grep --pattern 'fitCenter($$$) {
  $$$
  let newScale = $$$;
  $$$
}'

Length of output: 176

---

🏁 Script executed:

#!/bin/bash
# Locate FitPlugin source file
fit_file=$(rg --files | grep 'FitPlugin.ts$' | head -n1)
echo "FitPlugin file: $fit_file"
echo "---- defaultOptions / margin ----"
rg -n "defaultOptions" -C2 "$fit_file"
echo
echo "---- fitCenter implementation ----"
rg -n "fitCenter" -C10 "$fit_file"

Length of output: 3832

---

🏁 Script executed:

#!/bin/bash
# Show implementation details around newScale in fitCenter
rg -n fitCenter -C20 packages/core/src/view/plugins/FitPlugin.ts | sed -n '230,280p'
rg -n "scaleAndTranslate" -C5 packages/core/src/view/plugins/FitPlugin.ts

Length of output: 864

---

The test’s expected scale of 0.02 is correct.
The plugin applies keep2digits (i.e. Number(value.toFixed(2))) after computing 16/1000 = 0.016, which rounds to 0.02. No change is needed.

Likely an incorrect or invalid review comment.