docs: add more information about perimeters#656
Conversation
Explain more precisely how the connection point is computed on the perimeter. In particular, mention that it is possible to use orthogonal projection, and add various schemas to illustrates the explanations. Update the JSDoc of elements that are involved in the orthogonal projection.
WalkthroughThis pull request introduces modifications to the documentation and type definitions related to orthogonal edge styles and perimeters in the maxGraph library. The changes primarily focus on clarifying the behavior of the Changes
Possibly related PRs
Suggested Labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/website/docs/usage/perimeters.md (2)
58-60: Minor grammar improvement neededConsider revising "based on the center" to "in the center" for better grammatical accuracy.
-| Context | Computation based on the center | +| Context | Computation in the center |🧰 Tools
🪛 LanguageTool
[grammar] ~58-~58: In this expression, the usual preposition is “in”, not “on”. Did you mean “in the center”?
Context: ... | Computation based on the center | |-------------------------...(ON_IN_THE_MIDDLE)
70-70: Add comma for better readabilityAdd a comma before 'but' as it connects two independent clauses.
-By default, this is also applied when `style.orthogonal` is not explicitly set but the edge style belongs to one of the "orthogonal" `EdgeStyle` connectors. +By default, this is also applied when `style.orthogonal` is not explicitly set, but the edge style belongs to one of the "orthogonal" `EdgeStyle` connectors.🧰 Tools
🪛 LanguageTool
[uncategorized] ~70-~70: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...style.orthogonalis not explicitly set but the edge style belongs to one of the "o...(COMMA_COMPOUND_SENTENCE_2)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/website/docs/usage/assets/perimeters/perimeter-point-computation-01-context.pngis excluded by!**/*.pngpackages/website/docs/usage/assets/perimeters/perimeter-point-computation-02-center.pngis excluded by!**/*.pngpackages/website/docs/usage/assets/perimeters/perimeter-point-computation-03-orthogonal_projection.pngis excluded by!**/*.pngpackages/website/docs/usage/assets/perimeters/story-overview.pngis excluded by!**/*.png
📒 Files selected for processing (6)
packages/core/src/types.ts(1 hunks)packages/core/src/view/Graph.ts(1 hunks)packages/core/src/view/GraphView.ts(1 hunks)packages/core/src/view/cell/CellState.ts(12 hunks)packages/website/docs/usage/assets/perimeters/perimeters-diagrams.drawio(1 hunks)packages/website/docs/usage/perimeters.md(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/website/docs/usage/assets/perimeters/perimeters-diagrams.drawio
- packages/core/src/view/Graph.ts
- packages/core/src/view/GraphView.ts
🧰 Additional context used
🪛 LanguageTool
packages/website/docs/usage/perimeters.md
[grammar] ~58-~58: In this expression, the usual preposition is “in”, not “on”. Did you mean “in the center”?
Context: ... | Computation based on the center | |-------------------------...
(ON_IN_THE_MIDDLE)
[uncategorized] ~70-~70: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...style.orthogonal is not explicitly set but the edge style belongs to one of the "o...
(COMMA_COMPOUND_SENTENCE_2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (windows-2022)
🔇 Additional comments (4)
packages/core/src/view/cell/CellState.ts (1)
35-37: Documentation improvements look good!The updated JSDoc comments provide clearer references to properties using
{@link ...}syntax, improving documentation readability.packages/core/src/types.ts (1)
518-524: Excellent documentation enhancement for the orthogonal property!The updated documentation clearly explains:
- The relationship with perimeter computation
- The behavior with
Graph.isOrthogonal- The automatic behavior with orthogonal edge styles
This aligns perfectly with the PR's objective of clarifying perimeter computation.
packages/website/docs/usage/perimeters.md (2)
15-18: Clear and concise introduction to perimeters!The explanation effectively communicates the purpose and configuration of perimeters.
42-80: Excellent documentation of terminal point computation!The new section thoroughly explains:
- How terminal points are computed
- The orthogonal projection alternative
- Visual examples with diagrams
This significantly improves understanding of perimeter behavior.
🧰 Tools
🪛 LanguageTool
[grammar] ~58-~58: In this expression, the usual preposition is “in”, not “on”. Did you mean “in the center”?
Context: ... | Computation based on the center | |-------------------------...(ON_IN_THE_MIDDLE)
[uncategorized] ~70-~70: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...style.orthogonalis not explicitly set but the edge style belongs to one of the "o...(COMMA_COMPOUND_SENTENCE_2)
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/website/docs/usage/perimeters.md (5)
15-21: Consider adding a basic diagram to illustrate the perimeter concept.The explanation is clear, but a simple diagram showing a vertex with its perimeter and an edge connecting to it would help readers quickly grasp the concept.
Would you like me to help create a basic diagram to illustrate this concept?
23-27: Add a code example for disabling perimeters.Consider adding a TypeScript code snippet demonstrating how to disable the perimeter. This would make the documentation more actionable.
Example:
// Disable perimeter for a specific vertex const style = { ...vertexStyle, perimeter: null };
34-40: Enhance the image description.Consider adding more details about what specific perimeter types are shown in the example image. This would help readers understand what they're looking at before visiting the live demo.
58-60: Fix grammar in table header and enhance the comparison.
- Change "Computation based on the center" to "Computation in the center" for better grammar.
- Consider adding brief captions under each image to highlight the key differences between the two approaches.
🧰 Tools
🪛 LanguageTool
[grammar] ~58-~58: In this expression, the usual preposition is “in”, not “on”. Did you mean “in the center”?
Context: ... | Computation based on the center | |-------------------------...(ON_IN_THE_MIDDLE)
Line range hint
134-143: Enhance the custom perimeter example.The current custom perimeter example uses placeholder calculations (
x = 0, y = 0). Consider providing a more practical example that demonstrates actual coordinate calculations, perhaps for a triangle or star shape.Would you like me to provide a more practical custom perimeter implementation example?
🧰 Tools
🪛 LanguageTool
[grammar] ~58-~58: In this expression, the usual preposition is “in”, not “on”. Did you mean “in the center”?
Context: ... | Computation based on the center | |-------------------------...(ON_IN_THE_MIDDLE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/website/docs/usage/perimeters.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/website/docs/usage/perimeters.md
[grammar] ~58-~58: In this expression, the usual preposition is “in”, not “on”. Did you mean “in the center”?
Context: ... | Computation based on the center | |-------------------------...
(ON_IN_THE_MIDDLE)
🔇 Additional comments (1)
packages/website/docs/usage/perimeters.md (1)
Line range hint
1-184: Documentation changes look great!The enhanced documentation provides clear explanations of perimeter concepts, computation methods, and implementation details. The addition of visual aids and live demo references makes it very comprehensive.
🧰 Tools
🪛 LanguageTool
[grammar] ~58-~58: In this expression, the usual preposition is “in”, not “on”. Did you mean “in the center”?
Context: ... | Computation based on the center | |-------------------------...(ON_IN_THE_MIDDLE)
1 participant
Explain more precisely how the connection point is computed on the perimeter.
In particular, mention that it is possible to use orthogonal projection, and add various schemas to illustrates the
explanations.
Update the JSDoc of elements that are involved in the orthogonal projection.
Summary by CodeRabbit
Documentation
orthogonalproperty inCellStateStyleisOrthogonalmethod inGraphclassCellStateclassNew Features
Chores