Skip to content

Conversation

@tbouffard
Copy link
Member

@tbouffard tbouffard commented Nov 24, 2025

Replace imperative property assignment with object literal syntax
in createDefaultVertexStyle() and createDefaultEdgeStyle() methods.
This makes the code more concise and readable without changing behavior.

  • Use direct object literal returns instead of empty object + assignments
  • Add explicit return type annotations (CellStateStyle)

Summary by CodeRabbit

  • Refactor
    • Simplified default style object creation with improved type safety declarations.

✏️ Tip: You can customize this high-level summary in your review settings.

Replace imperative property assignment with object literal syntax
in createDefaultVertexStyle() and createDefaultEdgeStyle() methods.
This makes the code more concise and readable without changing behavior.

- Use direct object literal returns instead of empty object + assignments
- Add explicit return type annotations (CellStateStyle)
@tbouffard tbouffard added the refactor Code refactoring label Nov 24, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 24, 2025

Walkthrough

Two methods in the Stylesheet class (createDefaultVertexStyle and createDefaultEdgeStyle) now return CellStateStyle objects directly as literals with explicit type annotations, replacing intermediate variables and separate property assignments.

Changes

Cohort / File(s) Summary
Stylesheet refactoring
packages/core/src/view/style/Stylesheet.ts
Updated createDefaultVertexStyle() and createDefaultEdgeStyle() method signatures to explicitly return CellStateStyle type. Both methods now return literal CellStateStyle objects directly instead of using intermediate variables.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Simple, repetitive refactoring pattern applied uniformly to two methods
  • Type annotation clarification without behavioral changes
  • No complex logic or control flow modifications

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains what was changed and why, but lacks critical template requirements: no issue reference, no tests mentioned, no screenshots/videos discussion, and incomplete PR checklist items. Add issue reference (closes #xxxx), confirm tests included, explain if screenshots aren't applicable, and fill out all PR checklist items before merge.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: refactoring default style creation methods to use simpler syntax. It is concise, specific, and follows Conventional Commits format.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 refactor/simplify_stylesheet_default_styles

📜 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 8b3a7a3 and 9b75bf2.

📒 Files selected for processing (1)
  • packages/core/src/view/style/Stylesheet.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: tbouffard
Repo: maxGraph/maxGraph PR: 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
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.
🧬 Code graph analysis (1)
packages/core/src/view/style/Stylesheet.ts (1)
packages/core/src/types.ts (1)
  • CellStateStyle (78-899)
⏰ 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 (ubuntu-24.04)
  • GitHub Check: build (macos-14)
  • GitHub Check: build (windows-2022)
  • GitHub Check: build
🔇 Additional comments (2)
packages/core/src/view/style/Stylesheet.ts (2)

66-75: Default vertex style literal looks correct and behavior‑preserving

Returning a CellStateStyle object literal here is equivalent to the previous “empty object + assignments” pattern and keeps the same default properties; explicit return type is clear and consistent with the rest of the file.
Based on learnings, I’m intentionally not suggesting further behavior changes in this refactor.


81-89: Default edge style literal is consistent with previous semantics

The edge default style is now constructed in a concise literal form with the same keys/values and explicit CellStateStyle return type; no behavioral or typing issues stand out.
Based on learnings, this stays focused on the refactor objective without extra changes.


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

@tbouffard tbouffard merged commit c5b90db into main Nov 24, 2025
9 checks passed
@tbouffard tbouffard deleted the refactor/simplify_stylesheet_default_styles branch November 24, 2025 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant