Skip to content

feat: add generic font in default font family #931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
tbouffard merged 1 commit into from
Oct 31, 2025
Merged

feat: add generic font in default font family #931

tbouffard merged 1 commit into from
Oct 31, 2025

Conversation

@tbouffard
Copy link
Member

@tbouffard tbouffard commented Oct 31, 2025 •
edited by coderabbitai bot
Loading

This ensures reliability and consistency across diverse environments.

Summary by CodeRabbit

  • Bug Fixes
    • Improved exported image font rendering by expanding the default font family to include a sans-serif fallback, ensuring better text display consistency across different systems and applications.

@tbouffard
feat: add generic font in default font family
0c30495 
This ensures reliability and consistency across diverse environments.
@tbouffard tbouffard added the enhancement New feature or request label Oct 31, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 31, 2025 •
edited
Loading

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.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 31, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@tbouffard tbouffard merged commit 407d526 into main Oct 31, 2025
7 checks passed
@tbouffard tbouffard deleted the feat/add_generic_font_in_default_font_family branch October 31, 2025 09:48
@coderabbitai coderabbitai bot mentioned this pull request Nov 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tbouffard