Peritext UI events location #899
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
Centralize Peritext UI event types into the json-crdt-extensions/peritext/events package and update UI modules to import from the new location; replace the UI’s local UndoCollector with a new UndoManager interface.
- Introduce
UndoManagerin the UI and moveUndoCollectorinto the shared events defaults - Update dozens of import paths in UI and extension modules to point to the centralized events folder
- Add a local
Rectdefinition in events defaults and adjust related imports
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/json-crdt-peritext-ui/types.ts | Removed UI’s UndoCollector; added UndoManager interface |
| src/json-crdt-peritext-ui/plugins/toolbar/state/ToolbarState.tsx | Updated events imports to new centralized path |
| src/json-crdt-peritext-ui/plugins/toolbar/cursor/caret/CaretBottomOverlay/index.tsx | Fixed PeritextEventDetailMap import path |
| src/json-crdt-peritext-ui/plugins/minimal/RenderPeritext.tsx | Fixed ChangeDetail import path |
| src/json-crdt-peritext-ui/plugins/cursor/state.ts | Fixed ChangeDetail import path |
| src/json-crdt-extensions/peritext/events/types.ts | Changed relative imports to point at json-crdt-extensions root |
| src/json-crdt-extensions/peritext/events/index.ts | Adjusted createDataTransfer and Peritext imports |
| src/json-crdt-extensions/peritext/events/defaults/ui/types.ts | Defined a new Rect interface |
| src/json-crdt-extensions/peritext/events/defaults/ui/UiHandle.ts | Consolidated imports for constants and types |
| src/json-crdt-extensions/peritext/events/defaults/annals.ts | Updated imports for Anchor, Patch, and Range |
| src/json-crdt-extensions/peritext/events/defaults/types.ts | Added shared UndoCollector interface |
| src/json-crdt-extensions/peritext/events/defaults/PeritextEventDefaults.ts | Updated all extension imports, including Cursor, PersistedSlice |
| src/json-crdt-extensions/peritext/events/clipboard/DomClipboard.ts | Updated saveSelection import path |
| src/json-crdt-extensions/peritext/events/tests/marker.spec.ts | Updated test imports for setup utilities |
| src/json-crdt-extensions/peritext/events/tests/insert.spec.ts | Updated test imports for setup utilities |
| src/json-crdt-extensions/peritext/events/tests/inline.spec.ts | Updated test imports for setup utilities |
| src/json-crdt-extensions/peritext/events/tests/delete.spec.ts | Updated test imports for setup utilities |
| src/json-crdt-extensions/peritext/events/tests/cursor.spec.ts | Updated test imports for setup utilities |
| src/json-crdt-extensions/peritext/events/PeritextEventTarget.ts | Fixed import for SubscriptionEventTarget |
Comments suppressed due to low confidence (3)
src/json-crdt-peritext-ui/types.ts:1
- The new UndoManager interface lacks documentation; consider adding JSDoc comments to explain the purpose and usage of push and undo methods for better clarity.
export interface UndoManager {
src/json-crdt-peritext-ui/types.ts:1
- [nitpick] The naming between
UndoManager(UI) andUndoCollector(shared events) is inconsistent; aligning these interface names would improve clarity across modules.
export interface UndoManager {
src/json-crdt-peritext-ui/types.ts:1
- The new
UndoManagerinterface currently lacks unit tests; consider adding tests to cover itspushandundobehavior to maintain test coverage.
export interface UndoManager {
Comment on lines
+3
to
10
|
|
||
| export interface Rect { | ||
| x: number; | ||
| y: number; | ||
| width: number; | ||
| height: number; | ||
| } | ||
|
|
There was a problem hiding this comment.
Defining a Rect interface here duplicates types likely defined elsewhere (e.g., in web/types); consider reusing or aliasing a shared Rect type to prevent divergence.
Suggested change
| export interface Rect { | |
| x: number; | |
| y: number; | |
| width: number; | |
| height: number; | |
| } | |
| import type { Rect } from '../../../../../web/types'; |
Copilot uses AI. Check for mistakes.
| @@ -3,7 +3,7 @@ import * as React from 'react'; | |||
| import {FormattingsManagePane} from '../../../formatting/FormattingsManagePane'; | |||
| import {BottomPanePortal} from '../../util/BottomPanePortal'; | |||
| import {useToolbarPlugin} from '../../../context'; | |||
| import type {PeritextEventDetailMap} from '../../../../../events'; | |||
| import type {PeritextEventDetailMap} from '../../../../../../json-crdt-extensions/peritext/events'; | |||
There was a problem hiding this comment.
[nitpick] This long relative import path can be brittle and hard to maintain; consider configuring TypeScript path aliases to simplify and shorten imports.
Suggested change
| import type {PeritextEventDetailMap} from '../../../../../../json-crdt-extensions/peritext/events'; | |
| import type {PeritextEventDetailMap} from 'json-crdt-extensions/peritext/events'; |
Copilot uses AI. Check for mistakes.
|
🎉 This PR is included in version 17.43.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.