Actionable comments posted: 2

♻️ Duplicate comments (1)

packages/core/src/view/plugins/FitPlugin.ts (1)

107-125: Typo in JSDoc @param description

The text still says “Optional number” although the parameter is an object:

- * @param options Optional number that specifies the border.
+ * @param options Optional object to tweak fitting behaviour.

Minor, but surfaces in IDE tooltips.

🧹 Nitpick comments (4)

packages/core/src/editor/Editor.ts (2)

62-63: Import path may break tree‑shaking

FitPlugin is imported from '../view/plugins', which relies on the barrel file re‑exporting the symbol.
If a future optimisation removes the re‑export (very common when aiming for tree‑shakable bundles), this line will break while the local module path ('../view/plugins/FitPlugin') would still work.

-import type { FitPlugin } from '../view/plugins';
+import type { FitPlugin } from '../view/plugins/FitPlugin';

Consider importing directly from the concrete module (or keep the barrel but add an explicit re‑export and a unit‑test that guards it).

---

1031-1033: Graceful degradation if the plugin is absent

The optional‑chaining guards against a missing plugin, but from a UX standpoint silently doing nothing can confuse end‑users (the “Fit” toolbar button appears broken).

Recommend emitting a warning when getPlugin('fit') returns undefined so developers immediately notice a missing plugin registration:

- editor.graph.getPlugin<FitPlugin>('fit')?.fit();
+const fit = editor.graph.getPlugin<FitPlugin>('fit');
+if (fit) {
+  fit.fit();
+} else if (process.env.NODE_ENV !== 'production') {
+  // eslint-disable-next-line no-console
+  console.warn('[Editor] FitPlugin not registered – "fit" action skipped.');
+}

(This can be stripped in production builds by dead‑code‑elimination.)

packages/core/src/view/plugins/FitPlugin.ts (2)

21-23: keep2digits introduces unnecessary string allocation

Number(value.toFixed(2)) allocates an intermediate string each time.
For hot paths you can avoid this with simple arithmetic rounding:

-function keep2digits(value: number): number {
-  return Number(value.toFixed(2));
-}
+function keep2digits(value: number): number {
+  return Math.round(value * 100) / 100;
+}

Small, but measurable in very large graphs that call fitCenter frequently.

---

179-217: Round applied scale for deterministic re‑fits

fitCenter ends with keep2digits, but fit applies newScale directly.
Because of floating‑point artefacts successive fit() calls can accumulate tiny translation errors that move the diagram.

Consider re‑using keep2digits here as well:

-          view.scaleAndTranslate(newScale, x0, y0);
+          view.scaleAndTranslate(keep2digits(newScale), x0, y0);

Keeps the behaviour symmetrical with fitCenter and eliminates pixel‑creep.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7cac321 and 5fad3ad.

📒 Files selected for processing (7)

• CHANGELOG.md (1 hunks)
• packages/core/src/editor/Editor.ts (2 hunks)
• packages/core/src/view/Graph.ts (0 hunks)
• packages/core/src/view/plugins/FitPlugin.ts (4 hunks)
• packages/html/stories/OrgChart.stories.js (1 hunks)
• packages/html/stories/ZoomAndFit.stories.ts (2 hunks)
• packages/website/docs/usage/migrate-from-mxgraph.md (1 hunks)

💤 Files with no reviewable changes (1)

• packages/core/src/view/Graph.ts

🔇 Additional comments (6)

packages/html/stories/OrgChart.stories.js (1)

287-288: Updated to use the new FitPlugin API correctly.

The code now properly accesses the fit functionality through the plugin system using graph.getPlugin('fit')?.fit() instead of directly calling graph.fit(). This aligns with the architectural changes where the fit method was moved from Graph to FitPlugin.

CHANGELOG.md (1)

12-14: Good documentation of breaking changes.

The changelog clearly documents that:

1. The Graph.fit method has been moved to FitPlugin
2. The minFitScale and maxFitScale properties have also been moved
3. The method signature has been updated to accept a single parameter object

This provides users with clear information about what has changed and how it impacts their code.

packages/website/docs/usage/migrate-from-mxgraph.md (1)

359-367: Comprehensive migration guidance for the fit functionality.

The documentation accurately explains:

1. The fit() method has moved to FitPlugin
2. It now accepts a single parameter object instead of multiple positional parameters
3. The minFitScale and maxFitScale properties have also moved to FitPlugin

This clear guidance will help users migrate their code to the new API structure.

packages/html/stories/ZoomAndFit.stories.ts (3)

76-77: Good practice: stored plugin reference for reuse.

Storing the FitPlugin reference in a variable is more efficient than calling getPlugin() multiple times, especially when the plugin is used in multiple places.

---

89-99: Successfully migrated to the new options-based API.

The code now correctly uses the new fit and fitCenter methods with option objects, passing the margin parameter and other configuration options like ignoreHeight and ignoreWidth. This approach is more flexible and maintainable compared to positional arguments.

---

17-17: Import properly updated to include FitPlugin type.

The import statement has been updated to include the FitPlugin type, which is necessary for proper TypeScript typing when getting the plugin with getPlugin<FitPlugin>('fit').