iframe-proxy

tbouffard · 2025-07-25T13:40:34Z

Test options: border, ignore height, ignore width, keepOrigin, margin

Summary by CodeRabbit

Tests
- Improved clarity in test variable naming for better readability.
- Added new test cases covering scale limits, ignoring dimensions, special cases, and options like border, keep origin, and margin when fitting content.
Refactor
- Simplified fit method with early returns and clearer control flow without changing core functionality.

Test options: border, keepOrigin, margin

coderabbitai · 2025-07-25T13:40:41Z

Walkthrough

The FitPlugin was refactored to improve the fit method by introducing early returns for invalid containers or graph bounds and reducing nested conditionals. The test suite was renamed for clarity and expanded with numerous new test cases covering edge cases and additional options for the fit and fitCenter methods. No changes were made to exported or public entity declarations.

Changes

File(s)	Change Summary
FitPlugin source `packages/core/src/view/plugins/FitPlugin.ts`	Refactored `fit` method to use early returns and reduce nesting; clarified control flow and return paths without changing core logic.
FitPlugin tests `packages/core/__tests__/view/plugins/FitPlugin.test.ts`	Renamed Jest spy variable for clarity; expanded test coverage with nested describes and new cases for scale limits, margins, keepOrigin, ignoring dimensions, and special edge cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

test: add more tests to FitPlugin #834: Adds extensive tests for FitPlugin but without refactoring the fit method; related mainly at test coverage level.
fix(FitPlugin): handle edge cases in fitCenter calculation #831: Adds safeguards and edge case tests for zero-dimension bounds in fitCenter; related in edge case handling but not overlapping in refactor.
feat: let "fit center" with the new FitPlugin #733: Introduces initial implementation and documentation of FitPlugin and fitCenter; main PR builds upon this foundation.

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 156799c and 9ea3615.

📒 Files selected for processing (2)

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

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: tbouffard
PR: maxGraph/maxGraph#715
File: packages/html/stories/ZoomAndFit.stories.ts:0-0
Timestamp: 2025-03-14T06:01:22.326Z
Learning: For future improvements to the Graph component in maxGraph, tbouffard is interested in refactoring the `fit` method to use an object parameter pattern instead of multiple positional parameters, as the current approach makes the API harder to use when many parameters need to be specified with default values.

Learnt from: tbouffard
PR: maxGraph/maxGraph#857
File: packages/core/src/view/plugins/SelectionHandler.ts:1081-1092
Timestamp: 2025-06-26T13:27:19.012Z
Learning: User tbouffard prefers to keep PRs focused on their main objective and defer implementation improvements or logic fixes to future dedicated PRs. For maxGraph, refactoring PRs like Dictionary→Map migration should not include unrelated logic improvements even when valid issues are identified in the migrated code.

Learnt from: tbouffard
PR: maxGraph/maxGraph#776
File: packages/core/src/view/AbstractGraph.ts:492-495
Timestamp: 2025-04-24T12:34:56.726Z
Learning: PR #776 focuses on refactoring to introduce AbstractGraph as a base class and moving code from Graph to AbstractGraph. Type improvements and other refinements should be addressed in future PRs to keep this one focused on the core architectural change.

Learnt from: tbouffard
PR: maxGraph/maxGraph#849
File: packages/html/stories/DragSource.stories.js:98-101
Timestamp: 2025-06-13T07:48:10.300Z
Learning: User tbouffard prefers answers in English; avoid switching to other languages in future replies.

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

Learnt from: tbouffard
PR: #715
File: packages/html/stories/ZoomAndFit.stories.ts:0-0
Timestamp: 2025-03-14T06:01:22.326Z
Learning: For future improvements to the Graph component in maxGraph, tbouffard is interested in refactoring the fit method to use an object parameter pattern instead of multiple positional parameters, as the current approach makes the API harder to use when many parameters need to be specified with default values.

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

Learnt from: tbouffard
PR: #582
File: packages/core/tests/view/handler/config.test.ts:0-0
Timestamp: 2024-11-24T17:28:08.023Z
Learning: In packages/core/__tests__/view/handler/config.test.ts, when writing Jest tests in TypeScript for configuration reset functions, small duplication is acceptable if it maintains test clarity and readability. Avoid refactoring tests in ways that could make them harder to understand, especially when future use cases might require specific checks.

Learnt from: tbouffard
PR: #715
File: packages/html/stories/ZoomAndFit.stories.ts:0-0
Timestamp: 2025-03-14T06:01:22.326Z
Learning: For future improvements to the Graph component in maxGraph, tbouffard is interested in refactoring the fit method to use an object parameter pattern instead of multiple positional parameters, as the current approach makes the API harder to use when many parameters need to be specified with default values.

Learnt from: tbouffard
PR: #826
File: packages/js-example-nodejs/src/index.cjs:64-69
Timestamp: 2025-05-13T12:54:55.231Z
Learning: For example code in the maxGraph repository, maintainers prefer to keep scripts simple without error handling to focus on demonstrating core functionality, especially in demonstration scripts like those in packages/js-example-nodejs.

Learnt from: tbouffard
PR: #791
File: packages/ts-example/vite.config.js:30-30
Timestamp: 2025-04-29T13:25:31.494Z
Learning: In the maxGraph project, each example package (ts-example, ts-example-selected-features, ts-example-without-defaults) implements different use cases with varying features, resulting in different application sizes. Therefore, each package has its own specific chunkSizeWarningLimit value in its vite.config.js file, calibrated to its expected bundle size.

Learnt from: tbouffard
PR: #598
File: packages/website/docs/manual/getting-started.md:70-70
Timestamp: 2024-12-15T18:19:56.236Z
Learning: In code examples within the documentation, such as in packages/website/docs/manual/getting-started.md, we assume that the graph-container element exists and is an HTMLElement, and we avoid adding error handling for its initialization to keep the code simple.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: build
GitHub Check: build (macos-14)
GitHub Check: build (windows-2022)
GitHub Check: build (ubuntu-24.04)

🔇 Additional comments (8)

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

140-146: LGTM: Good refactoring with early returns.

The early returns for missing container and invalid graph bounds improve code readability by reducing nesting. The conditions correctly handle edge cases where fitting is not possible.

148-160: LGTM: Clear variable naming and logic organization.

The variable names (w1, h1 for container dimensions and later w2, h2 for graph dimensions) are clear in context. The keepOrigin handling is properly implemented by adjusting the bounds to start from origin.

162-187: LGTM: Scale calculation logic is sound.

The scale calculation properly handles:

Original scale normalization

Background image size consideration

Border and margin adjustments

Min/max scale clamping

Conditional logic for ignoring width/height

188-230: LGTM: Improved control flow for scaling operations.

The refactored control flow clearly separates the different scaling scenarios:

When keepOrigin is false: proper translation calculation with scrollbar handling

When keepOrigin is true: simple scale setting

When enabled is false: returning calculated scale without applying it

The logic correctly preserves the original behavior while improving readability.

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

54-62: LGTM: Good test setup with initial scale verification.

The test correctly sets an initial scale and verifies it's returned unchanged when graph has zero dimensions. The spy verification ensures scaleAndTranslate is called with the original scale.

72-78: LGTM: Spy rename addresses previous feedback.

The spy has been renamed from viewMock to scaleAndTranslateSpy as requested in previous reviews, making the purpose clearer.

117-248: LGTM: Excellent test organization with comprehensive coverage.

The test restructuring with nested describe blocks addresses previous feedback perfectly:

"no ignored dimensions" groups the main test cases

Individual tests for "ignore width" and "ignore height"

"special cases" for edge conditions

The new test cases provide excellent coverage for:

Scale limiting (minFitScale/maxFitScale)

Border and margin options

keepOrigin behavior (correctly verifies setScale vs scaleAndTranslate)

All scenarios have proper spy verification

282-322: LGTM: Comprehensive edge case testing.

The special cases section excellently tests the early return scenarios from the refactored fit method:

No container case properly hacks around the constructor limitation

Parameterized test for non-positive width/height dimensions

Both cases correctly verify no scaling methods are called and original scale is returned

This provides complete coverage for the new early return logic.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch test/more_fit_tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

add test for "ignore height", ignore width also improve tests for fit center

sonarqubecloud · 2025-07-28T06:27:45Z

test: add tests for FitPlugin.fit

156799c

Test options: border, keepOrigin, margin

tbouffard added the chore Build, CI/CD or repository tasks (issues/PR maintenance, environments, ...) label Jul 25, 2025

tbouffard commented Jul 27, 2025

View reviewed changes

Comment thread packages/core/__tests__/view/plugins/FitPlugin.test.ts Outdated

tbouffard commented Jul 27, 2025

View reviewed changes

Comment thread packages/core/__tests__/view/plugins/FitPlugin.test.ts Outdated

tbouffard changed the title ~~test: add tests for FitPlugin.fit~~ test: add moe tests for FitPlugin.fit Jul 27, 2025

tbouffard changed the title ~~test: add moe tests for FitPlugin.fit~~ test: add more tests for FitPlugin.fit Jul 27, 2025

tbouffard added 3 commits July 27, 2025 21:01

rename variable to scaleAndTranslateSpy

813a23b

add describe for tests with "no ignored dimensions"

2ad4bfa

Add various tests and simplify the implementation

9ea3615

add test for "ignore height", ignore width also improve tests for fit center

tbouffard merged commit 95cdbc8 into main Jul 28, 2025
7 checks passed

tbouffard deleted the test/more_fit_tests branch July 28, 2025 09:02

coderabbitai Bot mentioned this pull request May 27, 2026

test(FitPlugin): simplify container dimension setup #1079

Merged

1 task

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: add more tests for `FitPlugin.fit`#888

test: add more tests for `FitPlugin.fit`#888
tbouffard merged 4 commits into
mainfrom
test/more_fit_tests

tbouffard commented Jul 25, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 25, 2025 •

edited

Loading

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jul 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

tbouffard commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jul 28, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tbouffard commented Jul 25, 2025 •

edited

Loading

coderabbitai Bot commented Jul 25, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)