feat: add generic font in default font family by tbouffard · Pull Request #931

feat: add generic font in default font family by tbouffard · Pull Request #931 · maxGraph/maxGraph

Walkthrough

The PR updates the DEFAULT_FONTFAMILY constant from "Arial,Helvetica" to "Arial,Helvetica,sans-serif" and updates corresponding test expectations to reflect this change.

Changes

Cohort / File(s)	Summary
Constant Update `packages/core/src/util/Constants.ts`	Updated `DEFAULT_FONTFAMILY` constant value from `'Arial,Helvetica'` to `'Arial,Helvetica,sans-serif'`
Test Updates `packages/core/__tests__/view/image/ImageExport.test.ts`	Updated font-family values in rendered output expectations (3 occurrences) from "Arial,Helvetica" to "Arial,Helvetica,sans-serif" for XmlCanvas2D and SvgCanvas2D exports

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Straightforward constant value update with matching test snapshot updates
No logic changes or control flow modifications
Changes are localized and consistent across files

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The PR description is severely incomplete and fails to follow the required template structure. The author provided only a single generic sentence ("This ensures reliability and consistency across diverse environments.") without any of the required sections: the PR Checklist is entirely missing, no issue reference is provided (no "closes #" or "fixes #" keyword), there is no explanation of the problem being solved or why the change is beneficial, and no information about tests, screenshots, or documentation changes. The description is too vague to understand the specific rationale for this change or verify that the standard PR requirements have been met.	The author should provide a complete PR description that includes a filled-out checklist confirming that tests have been added, an issue number is referenced with an appropriate keyword (e.g., "closes #931"), and a detailed overview section explaining why adding the generic "sans-serif" fallback improves reliability and consistency across different environments and browsers. The description should be specific rather than generic and address all sections of the repository's PR template.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The PR title "feat: add generic font in default font family" directly and accurately describes the main change in the changeset. The developer is adding a generic sans-serif fallback to the DEFAULT_FONTFAMILY constant, changing it from 'Arial,Helvetica' to 'Arial,Helvetica,sans-serif'. The title follows Conventional Commits format with the "feat:" prefix and uses clear, concise language that a teammate can quickly understand when scanning the project history.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/add_generic_font_in_default_font_family

📜 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 8011534 and 0c30495.

📒 Files selected for processing (2)

packages/core/__tests__/view/image/ImageExport.test.ts (3 hunks)
packages/core/src/util/Constants.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 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.

📚 Learning: 2025-04-29T13:30:18.686Z

Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 791
File: packages/website/docs/usage/edge-styles.md:59-63
Timestamp: 2025-04-29T13:30:18.686Z
Learning: In the EdgeStyles documentation, tbouffard prefers the existing text "A perimeter is a function matching the `EdgeStyleFunction` type" over suggestions that might create redundancy, even if terminology might be technically misaligned.

Applied to files:

packages/core/__tests__/view/image/ImageExport.test.ts

🔇 Additional comments (2)

packages/core/src/util/Constants.ts (1)

263-263: LGTM! Excellent addition of generic font fallback.

Adding sans-serif as a generic font family follows CSS best practices and ensures text renders appropriately across diverse environments when Arial or Helvetica are unavailable.

packages/core/__tests__/view/image/ImageExport.test.ts (1)

99-99: LGTM! Test expectations correctly updated.

All test expectations have been properly updated to reflect the new DEFAULT_FONTFAMILY value, ensuring tests validate the correct font-family rendering in both XmlCanvas2D and SvgCanvas2D exports.

Also applies to: 220-220, 227-227, 237-237

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}