feat(pagination): add enabled option to toggle pagination at runtime#424
feat(pagination): add enabled option to toggle pagination at runtime#424arthrod wants to merge 2 commits into
Conversation
`enabled` (default true) on BasePaginationPlugin. When false, the React layer skips layout recompute and renders no overlay; document never affected. Toggle via editor.setOption(BasePaginationPlugin,'enabled',next). TDD: base option contract (default true, configurable). Changeset: minor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review very carefully and fix |
|
@gemini review very carefully and fix |
|
@Kilo review very carefully and fix |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me gather the PR details and code changes first. 🏁 Script executed: Length of output: 7737 Let me get more context on the existing implementation to do a thorough review. 🏁 Script executed: Length of output: 13848
Review Summary🐛 Bug:
|
There was a problem hiding this comment.
Code Review
This pull request introduces an enabled option to the pagination plugin, allowing users to toggle pagination at runtime. When disabled, the plugin skips layout recomputations and avoids rendering page-break overlays. The changes include updates to the plugin configuration, React hooks, and the addition of unit tests to verify the new functionality. A review comment suggested optimizing the PaginationBreakLines component by moving the enabled check before DOM lookups to prevent unnecessary processing.
| const editable = editor.api.toDOMNode(editor); | ||
| if (!editable || breaks.length === 0) return null; | ||
| if (!enabled || !editable || breaks.length === 0) return null; |
There was a problem hiding this comment.
The enabled check should ideally occur before calling editor.api.toDOMNode(editor) to avoid unnecessary DOM lookups when pagination is disabled.
| const editable = editor.api.toDOMNode(editor); | |
| if (!editable || breaks.length === 0) return null; | |
| if (!enabled || !editable || breaks.length === 0) return null; | |
| if (!enabled) return null; | |
| const editable = editor.api.toDOMNode(editor); | |
| if (!editable || breaks.length === 0) return null; |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Stacked on
codex/pagination-dogfood-fixes. Adds a publicenabledoption to@platejs/paginationso consumers can turn pagination on/off at runtime.Change
BasePaginationPlugingainsenabled: boolean(defaulttrue).enabled === false. Toggling re-renders (subscribed option) so re-enabling recomputes from the dirty registry. The document is never mutated either way.editor.setOption(BasePaginationPlugin, 'enabled', next).minor.TDD
Red→green on the new public contract (base option seam,
createSlateEditor):enables pagination by default— watched it fail (enabledundefined), then declared the option.can be disabled via options.The React overlay/hook gating is thin glue consuming the tested option (the package's React overlay has no unit tests — only pure geometry; no render-test infra, and the testing-strategy skill forbids adding devDeps for it).
Verification
checknot run (itseslint .sweep OOMs here; disproportionate for a one-option package change).🤖 Generated with Claude Code