docs: add a Guides section for task oriented pages - #1207
redfish4ktc wants to merge 2 commits into
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. WalkthroughThis PR adds guides for BaseGraph setup, bundle-size reduction, and extending maxGraph. It updates migration guidance, plugin and tree-shaking references, website documentation rules, and related links. It also changes a TypeScript custom-shape example to restore style defaults after resets. ChangesWebsite documentation and examples
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Other Merge Risk: 🔵 Low · up to Readers who copy this setup for rectangular or mixed-shape diagrams may see edges attach incorrectly. Keep the rectangular default or scope the ellipse override; this is a localized documentation issue. 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 |
| | 0.23.0 | Graph mixins start moving into plugins: `TooltipMixin` disappears, its methods becoming those of `TooltipHandler` | | ||
| | 0.24.0 | A dedicated registration helper per built-in `EdgeStyle`, and the image bundle feature moves to a plugin | | ||
| | 0.25.0 | The cell handlers move from `AbstractGraph` to the `SelectionCellsHandler` plugin, so an application that does not register that plugin no longer bundles `VertexHandler`, `EdgeHandler`, `ElbowEdgeHandler` and `EdgeSegmentHandler` | | ||
| | 0.25.0 | The cell handlers move from `AbstractGraph` to the `SelectionCellsHandler` plugin, so an application that does not register that plugin no longer bundles `VertexHandler`, `EdgeHandler`, `ElbowEdgeHandler` and `EdgeSegmentHandler`. `registerDefaultStyleElements()` also appears, grouping the four style `registerDefault*` functions in a single call | |
There was a problem hiding this comment.
nitpick: remove refs to registerDefaultStyleElements, not related to tree-shaking. This is an helpers that avoid to call several functions.
There was a problem hiding this comment.
Agreed for this table, which lists what each version removed from a bundle. registerDefaultStyleElements() groups four registration calls and removes nothing, so it does not belong in it. Dropped in f587301.
I kept the two other mentions on the page, since both warn against the helper rather than present it as a gain: one in the section contrasting the broad shortcut with the narrow registration, the other in the list of what not to load, where calling it "to be safe" is named as an anti-pattern. Tell me if you want those gone as well.
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.
| const stylesheet = graph.getStylesheet(); | ||
|
|
||
| const defaultVertexStyle = stylesheet.getDefaultVertexStyle(); | ||
| defaultVertexStyle.perimeter = 'ellipsePerimeter'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '145,205p' packages/website/docs/guides/configure-basegraph.md
sed -n '45,95p' packages/core/src/view/style/Stylesheet.ts
rg -n 'ellipsePerimeter|rectanglePerimeter|defaultVertexStyle' packages/website/docs/guides/configure-basegraph.mdRepository: maxGraph/maxGraph
Length of output: 5467
Match the default perimeter to the documented vertex shapes.
The guide documents both plain rectangles and ellipse styles, but the shared default vertex style still uses shape: 'rectangle'. Setting only defaultVertexStyle.perimeter to ellipsePerimeter can give rectangular vertices incorrect edge endpoints unless every rectangle overrides its perimeter.
Keep rectanglePerimeter as the default for mixed-shape graphs, or state that this override applies only to an all-ellipse graph and show the required ellipse style override.
This branch has not been deployed
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.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.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 throws when a constructor requires an argument.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