docs: add a Guides section for task oriented pages - #1207
Conversation
The documentation explained how each feature works, but never how to extend maxGraph, and no page walked a reader through one complete task. Add a `Guides` section for that kind of page, ordered by what a reader needs first, and move `migrate-from-mxgraph.md` into it with a redirect keeping its published URL alive. `extend-maxgraph.md` documents the extension points users actually reach for, with the rules the types do not expose, such as a shape having to survive construction with no argument. `reduce-bundle-size.md` takes the procedure out of `usage/tree-shaking.md`, which contradicted it on `getDefaultPlugins()` and the `registerDefault*` functions: a guide now says what to do and the reference page what to know. `configure-basegraph.md` covers the opposite trajectory, for which the material existed, spread over the graph, plugins and tree-shaking pages, but nothing said in which order to decide. Every factual claim was checked against `packages/core`, which invalidated about forty of them, and each guide was validated by following it literally to build an application. The two custom shapes of `packages/ts-example` are rewritten along the way, since their constructor defaults were wiped at the first style change, which is exactly what the new page tells readers to avoid. The extending guide also advises flat style properties holding simple types, because the natural reflex, one object gathering related values, is the shape the API handles worst: `setCellStyles` assigns a top level key only, and it first clones the style through a hand written recursive copy that corrupts a `Date`, a `Map`, a `Set` and an object created with `Object.create(null)`, and throws when a constructor requires an argument. Measured against the built library, since the source suggests a plain deep copy. The published anchor `#guide-improving-the-tree-shaking-of-an-application-using-graph` disappears with this work, deliberately, an anchor being impossible to redirect.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThis PR adds BaseGraph configuration and bundle-size guides, revises guidance for extending maxGraph, and updates the mxGraph migration guide. It adds website documentation rules, updates reference pages and links, and changes a TypeScript custom-shape example to restore defaults after style resets. ChangesWebsite documentation and examples
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Other Merge Risk: ⚪ Minimal · up to The examples are listed under their correct language and build-tool sections, and custom-shape defaults survive style resets. No material merge risk remains beyond normal checks. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 6 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
Review findings on the Guides section, each checked against the sources rather than taken as a wording preference. The default vertex and edge styles are returned by reference from the `Stylesheet` map, so one property can be assigned in place. Spreading the whole object into `putDefaultVertexStyle` restated every value to change one, and needed a warning about the merge that the shorter form makes pointless. The class JSDoc already documents the in place form. The over-trimming table said an unrouted edge becomes a straight line between its terminals. `GraphView.updatePoints` falls back to the waypoints of the geometry when no edge style resolves, so that is only true of an edge without any. The same table named the arrow head alone, where `ConnectorShape.createMarker` runs for `startArrow` and `endArrow` alike and the symbol a registration carries can be any shape. The sizes of step 7 are tied to a version, which the page said in a paragraph placed after them, so a first reader met the numbers before learning what they were measured on. Moved ahead of them. `registerDefaultStyleElements()` leaves the tree-shaking milestone table: it groups four registration calls for convenience and removes nothing from a bundle, so it is not an improvement of that kind.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 9dc88b8f-23dc-42b3-b168-5518d768c8d3
📒 Files selected for processing (3)
packages/website/docs/guides/configure-basegraph.mdpackages/website/docs/guides/reduce-bundle-size.mdpackages/website/docs/usage/tree-shaking.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/website/docs/guides/reduce-bundle-size.md
- packages/website/docs/usage/tree-shaking.md
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
The milestone table named the change made in each release but never its effect, so a reader could not tell which versions mattered to them. Add a column stating what leaves the bundle, or what a bundler can newly do. Writing that column meant reading the release notes of every version from 0.6.0 on, which is what the table was missing: the CHANGELOG records breaking changes only, and most of this work breaks nothing. Four milestones were absent, each one claimed and measured by its own release. The i18n provider becoming configurable in 0.17.0 drops the `Translations` machinery for applications that never translate. `EdgeStyle` and `Perimeter` becoming namespaces in 0.19.0 removes a shape that defeated the module level analysis of several bundlers. The library no longer naming the built-in edge styles in 0.20.0 is what finally lets an unregistered one be dropped, and what the per style helpers of 0.24.0 are built on. `fit` moved to a plugin in 0.21.0. The 0.24.0 row was incomplete, `EdgeHandler` having also stopped importing `EdgeStyle` that release, and the 0.12.0 row claimed no side effects where the declaration excepts the CSS files. The row about the dedicated registries of 0.20.0 is dropped. Comparing the 0.19.0 tree showed the perimeters were already separate modules and the registry it replaced imported no implementation, so the change was a consistency one with no bundle gain to report. The enum removal of the same release and the `Dictionary` removal of 0.21.0 are left out for the opposite reason: both shrink every bundle unconditionally, where this table lists what an application can choose not to load.
The example replacing the default `perimeter` and `endArrow` read as one gesture, where the two lines differ: `'none'` names no marker and removes a registration, while `'ellipsePerimeter'` is a perimeter like any other and has to be registered in turn, so it only pays off for an application that registers it anyway. Nothing said so, and nothing said the name has to match the shapes the cells draw either, although an ellipse perimeter on a rectangular vertex computes the attach point on an ellipse that is not there. Raised by a reviewer reading the snippet as copyable, which it is.
The previous commit claimed a perimeter has no equivalent of the `'none'` marker value, which is wrong: leaving the property at `null` or `undefined` uses no perimeter and registers nothing, as `GraphView.getPerimeterFunction` returns null and the terminal point is then never computed. The JSDoc of `CellStateStyle.perimeter` and the Perimeters page both say so already, so the guide now points at the latter instead of contradicting it. Framing it as an option also balances the warning next to it: edges meeting their terminals at the centre is a symptom when it comes from a registration left out by mistake, and a legitimate choice when it is wanted.
A re-read of the page against `packages/core` found three statements that do not hold, among roughly 170 checked. `createGraphDataModel` and `createStylesheet` were described as running only when the constructor received neither a model nor a stylesheet. The two fallbacks are independent, so passing only a model still runs `createStylesheet`. `baseStyleNames` was called the only array among the built-in style properties. `portConstraint`, `sourcePortConstraint` and `targetPortConstraint` are typed `DirectionValue | DirectionValue[]`, so three others accept one. It is the only property that is always an array, which is the claim the advice around it actually needs. The clone table said an instance whose constructor requires an argument makes the copy throw. The copy calls the constructor with no argument, so a constructor that only stores its parameter yields an object with undefined fields instead, which is just as wrong for the reader and worth naming. The row now covers both outcomes, and applies to any class rather than only that one shape of constructor. Two wordings were also too wide. The five `create*` factories belong to `Graph`, `BaseGraph` having none, and the constraint handler snippet is adapted from its story rather than taken from it, since it adds a modifier and a return type the story does not carry.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Move the cross-guide paragraph below “Before you start”. · configure-basegraph.md:17-19
packages/website/docs/guides/configure-basegraph.md:17-19
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove the cross-guide paragraph below “Before you start”.
The guide rule requires the “Before you start” block to follow the goal. This cross-guide paragraph is separate guidance, not part of the goal. Its current position places an alternate workflow before the guide’s prerequisites.
Suggested fix
-[Reduce the Bundle Size of an Application](./reduce-bundle-size.md) covers the opposite direction, an application -already built on `Graph` and moved over. Follow that one if this is your case: it starts from everything and removes -progressively, which is the safer order when the application already works. - ## Before you start @@ This guide assumes you know how the two graph classes differ, see [Graph vs BaseGraph](../usage/graph.md#graph-vs-basegraph). +[Reduce the Bundle Size of an Application](./reduce-bundle-size.md) covers the opposite direction, an application +already built on `Graph` and moved over. Follow that one if this is your case: it starts from everything and removes +progressively, which is the safer order when the application already works. + ## 1. Start from the bare graph
🟡 Minor · Move non-goal prose below Before you start. · reduce-bundle-size.md:18-19
packages/website/docs/guides/reduce-bundle-size.md:18-19
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove non-goal prose below
Before you start.The cross-guide paragraph is navigation content, not part of the goal. The rule requires the block to follow the goal immediately. This file also places the version note before the block, so moving only the cross-guide paragraph would not satisfy the rule. Move both paragraphs below the block.
Suggested fix
-This guide was written and verified with `maxGraph` 0.25.0, which step 2 requires since it uses -`registerDefaultStyleElements`. Earlier versions are not documented here. - -A new application does not need this procedure, since it has nothing to remove: -[Set Up an Application on BaseGraph](./configure-basegraph.md) builds the same configuration from the other end. - ## Before you start @@ This guide assumes you know how the two graph classes differ, see [Graph vs BaseGraph](../usage/graph.md#graph-vs-basegraph). +This guide was written and verified with `maxGraph` 0.25.0, which step 2 requires since it uses +`registerDefaultStyleElements`. Earlier versions are not documented here. + +A new application does not need this procedure, since it has nothing to remove: +[Set Up an Application on BaseGraph](./configure-basegraph.md) builds the same configuration from the other end. +
🟡 Minor · State that earlier versions are not covered. · reduce-bundle-size.md:15-16
packages/website/docs/guides/reduce-bundle-size.md:15-16
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winState that earlier versions are not covered.
“Not documented here” does not make clear that earlier versions are outside this guide’s scope. Use the required wording.
Suggested fix
-`registerDefaultStyleElements`. Earlier versions are not documented here. +`registerDefaultStyleElements`. Earlier versions are not covered.
🟡 Minor · Add the required page-level version boundary. · extend-maxgraph.md:12-16
packages/website/docs/guides/extend-maxgraph.md:12-16
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the required page-level version boundary.
The
Sincenotes apply only to individual features. AddThis guide was written and verified with \maxGraph` . Earlier versions are not covered.` using the version actually used for the guide. Without this statement, readers cannot identify the supported version for the recipe collection and may apply a recipe to an untested release.
🟡 Minor · Place “Before you start” directly after the guide goal. · migrate-from-mxgraph.md:46
packages/website/docs/guides/migrate-from-mxgraph.md:46
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPlace “Before you start” directly after the guide goal.
This linear guide must put its prerequisite block immediately after the goal. The current warning, table of contents, background, and information block delay the hard-gate checklist. Move the block before the warning. Update “notice above” to “notice below” so the checklist keeps its meaning.
Suggested fix
This documentation provides instructions for migrating from `mxGraph` to `maxGraph`. It includes information about application setup changes, code changes, styles, event handling and other relevant information. +## Before you start + +- **An `mxGraph` application that builds and runs**, since every section below changes it in place and you check it as + you go. +- **Node.js and npm**, the first step replacing the dependency. +- **An inventory of the `mx` prefixed names your code uses**, classes, constants and style keys, which the sections + below rename family by family. +- **The maxGraph version you are migrating to**, using the migration notice below to identify what changed after 0.18.0. + +This guide assumes you know `mxGraph` itself: it states what changed, not how a diagram works. It migrates to `Graph`, +see [Graph vs BaseGraph](../usage/graph.md#graph-vs-basegraph). + :::danger[Important Notice] As of version `0.18.0`, this migration guide isn't updated with the latest `maxGraph` API changes, particularly breaking ones. @@ -## Before you start - -- **An `mxGraph` application that builds and runs**, since every section below changes it in place and you check it as - you go. -- **Node.js and npm**, the first step replacing the dependency. -- **An inventory of the `mx` prefixed names your code uses**, classes, constants and style keys, which the sections - below rename family by family. -- **The maxGraph version you are migrating to**, the notice above listing what changed after 0.18.0. - -This guide assumes you know `mxGraph` itself: it states what changed, not how a diagram works. It migrates to `Graph`, -see [Graph vs BaseGraph](../usage/graph.md#graph-vs-basegraph). - ## Application setup
🟡 Minor · State the guide's written-and-verified version. · migrate-from-mxgraph.md:13-18
packages/website/docs/guides/migrate-from-mxgraph.md:13-18
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winState the guide's written-and-verified version.
The notice identifies
0.18.0as the point where updates stopped, but it does not state the version with which the guide was written and verified. Add the actual verified version in the required page-level notice. Without it, readers cannot determine whichmaxGraphversion the migration steps were validated against.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
e9235c48-f3b9-4885-9ab7-15cd7e95fd32
📒 Files selected for processing (3)
packages/website/docs/guides/configure-basegraph.mdpackages/website/docs/guides/extend-maxgraph.mdpackages/website/docs/usage/tree-shaking.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/website/docs/guides/configure-basegraph.md
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
The rule asking for a demo link paired with a source link said nothing about which ref the source link uses, which leaves it looking like an oversight next to the habit of pinning a repository link to a commit so it keeps showing what it showed when written. It is a choice, so write it down with its reason: the two links of a pair have to agree, and the demo always runs the implementation on `main`, so pinning the source would send the reader to code the demo no longer runs. The usual risk of an unpinned link barely applies to a story file, which is renamed rarely and moved almost never. The twenty five story links already in the documentation all point at `main`, so nothing changes in the pages.
The release page pointed at `blob/development/.github/release.yml`. There is no `development` branch on the remote, so the link has been returning a 404 to anyone following the release procedure. Pinned to the commit that last changed that file rather than moved to `main`, since it is a configuration file and not a Storybook story, the only link the documentation deliberately leaves on a branch.
A review read the guides against their own rule and found three gaps, two in the pages and one in the rule itself. `reduce-bundle-size.md` closed its version sentence with "Earlier versions are not documented here" where the rule prescribes "Earlier versions are not covered", the wording `configure-basegraph.md` already used. The two siblings now agree. `extend-maxgraph.md` carried no page level version at all. Its three `Since 0.25.0` markers cover single features, so a reader had nothing telling them which release the recipes as a whole were verified against. The exemption that page holds is from the prerequisites block, not from naming a version. The rule asked for that block "right after the goal" while two guides put a redirect to their sibling in between, so it read as a violation rather than as the deliberate choice it is: a reader on the wrong page should leave before working through prerequisites that do not apply to them. The rule now says what may stand there, and records why `migrate-from-mxgraph.md` puts its danger notice first.
|
@coderabbitai four of the six outside-diff comments are addressed in Taken. Taken, but by amending the rule rather than the pages. The two comments about moving a paragraph below Declined, both on Moving Stating a written-and-verified version on that page would be inaccurate. The page announces that it is not maintained past 0.18.0, so claiming it was verified against a release would assert exactly the maintenance it disclaims. The notice already gives the honest boundary, together with what a reader should do about it. |
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd --hidden --type f '^(guides-structure|configure-basegraph|reduce-bundle-size|extend-maxgraph|migrate-from-mxgraph)\.md$' . --exclude .git | sort | while IFS= read -r file; do
printf '\n--- %s ---\n' "$file"
if [[ "$file" == *guides-structure.md ]]; then
cat -n "$file"
else
sed -n '1,100p' "$file"
fi
doneLength of output: 26770
The four accepted comments are addressed:
My suggestion to move the version sentence below the prerequisites also contradicted the existing preamble rule. That suggestion was incorrect. I withdraw both comments on No further changes are needed for these six comments. ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd --hidden --type f '^(guides-structure|configure-basegraph|reduce-bundle-size|extend-maxgraph|migrate-from-mxgraph)\.md$' . --exclude .git | sort | while IFS= read -r file; do
printf '\n--- %s ---\n' "$file"
if [[ "$file" == *guides-structure.md ]]; then
cat -n "$file"
else
sed -n '1,85p' "$file"
fi
doneLength of output: 24741
The four accepted comments are addressed:
My suggestion to move the version sentence below the prerequisites contradicted the existing rule. That suggestion was incorrect. I withdraw both comments on
No further changes are needed for these six comments. 🧠 Learnings usedYou are interacting with an AI system. |
1 participant
Why
The documentation explains how each feature works, but never how to extend maxGraph, and no page walks a reader through one complete task from end to end. Someone who wants to write a custom shape, move an application off
Graph, or configure aBaseGraphfrom scratch has to assemble the answer from three or four reference pages, in an order nothing states.What
A new
Guidessection, placed beforeUsage, for pages that walk a reader through one task. It holds four pages, ordered by what a reader needs first:configure-basegraph.mdBaseGraphfrom the bare graph up, deciding one concern at a timereduce-bundle-size.mdGraphtoBaseGraphextend-maxgraph.mdShape, declaring your ownCellstyle propertiesmigrate-from-mxgraph.mdusage/, since it is a task rather than a featureextend-maxgraph.mdis new and is the substance of the change.reduce-bundle-size.mdtakes the procedure out ofusage/tree-shaking.md, which contradicted it ongetDefaultPlugins()and theregisterDefault*functions; the reference page keeps the per-family reference and each prohibition now names the migration as its exception.configure-basegraph.mdgathers material that existed, spread over the graph, plugins and tree-shaking pages, but with nothing saying in which order to decide.usage/plugins.mdgains aKindcolumn, answering what each built-in plugin does when the application never calls it, and anIdcolumn, which is what a reader needs to map agetPlugincall back to a class.The milestone table of
usage/tree-shaking.mdgains aWhat it buyscolumn, since it named the change made in each release but never its effect. Writing it meant reading the release notes of every version from 0.6.0 on, the CHANGELOG recording breaking changes only: four milestones were missing, 0.17.0, 0.19.0, 0.20.0 and 0.21.0, each claimed and measured by its own release. One row was dropped, the 0.20.0 registry consolidation, whose bundle gain could not be shown against the 0.19.0 tree.How the content was verified
Every factual claim was checked against
packages/corerather than written from memory, over several rounds, which invalidated about forty of them. Among the corrections worth knowing while reviewing: the paint hooks are called byShape.paintVertexShape, so they run on every shape that does not replace it, and the shapes that do replace it include the wholeAbstractPathShapefamily;augmentBoundingBoxis almost never reached,useSvgBoundingBoxdefaulting totrue; the mixin members are properties rather than methods, exceptinsertVertexandinsertEdge; insideregisterDefaults()the prototype members are callable and only the container and the five collaborators are missing; the bundle floor comes from importingGraph, not from instantiating it.A final pass re-checked
extend-maxgraph.mdline by line after the review round, which invalidated three more: the two collaborator factories are skipped per argument rather than together,baseStyleNamesis the only style property that is always an array rather than the only one that can be, and the clone misstatement above.Each guide was then validated by following it literally to build an application, which corrected it further: the first snippet compiles, the steps that ask the reader to check the result have a diagram to look at, the stylesheet import appears in the snippet that needs it, and the default styles are set before the cells that depend on them.
The advice on custom style properties rests on behaviour measured against the built library rather than read off the source, since the source suggests a plain deep copy:
Cell.getClonedStyle()is a hand written recursive copy that gives back the current date for aDate, an emptyMaporSet,nullfor an object created withObject.create(null), and calls every other constructor with no argument at all, so an instance comes back with its fields undefined, or the clone throws outright when that constructor reads its parameter.Points a reviewer should decide on
#guide-improving-the-tree-shaking-of-an-application-using-graphdisappears with the split ofusage/tree-shaking.md. An anchor cannot be redirected, since the browser never sends the fragment to the server, so the alternatives were to keep a stub heading or to accept the loss. The page itself keeps its URL, andmigrate-from-mxgraph.mdkeeps its own through a{ from, to }redirect entry..claude/rules/documentation/website.mdnext to the redirect rule it depends on, with the structure the guides follow inguides-structure.md.packages/ts-examplechanges too. Its two custom shapes setstrokeWidthandisRoundedin a constructor the registry calls with no argument, and whichresetStyles()wipes at the first style change. They are rewritten the way the new page recommends, since an example that contradicts the guide is worse than no example. The same fix is open on the examples repository, maxgraph-integration-examples#312.Related
The
Patching a prototypesection of the new guide gives the replacement advice that #420 asks the JSDoc to point at.This work also produced twelve issues, #1193 to #1204, and a comment on #418; none of them is a prerequisite for this pull request.
Summary by CodeRabbit
New Features
BaseGraph, extending maxGraph, migrating from mxGraph, and reducing bundle size.Documentation