[lexical-history][lexical-selection][lexical-react] Fix: #6409 TextNode change detection #42
Conversation
![local-dev-korbit-ai-mentor[bot]](https://avatars.githubusercontent.com/in/318591?s=60&v=4){kind=small}
Add delay option to HistoryPlugin Add test for #6409 Fix lint issues Fix $cloneWithProperties and test expectations Targeted fix for TextNode leaf change
![[local-dev] Korbit - AI Mentor](https://avatars.githubusercontent.com/in/318591?s=40&u=4153787aed90096cc5d05c9c38a439007e524086&v=4){kind=avatar}
|
Clone of the PR facebook/lexical#6420 |
|
My review is in progress 📖 - I will have feedback for you in a few minutes! |
WalkthroughRecent changes enhance the functionality and maintainability of the Lexical framework. Key updates include the introduction of new node types for improved text manipulation, refinements in type handling for cloning functions, and enhancements to history management with optional delay settings. Additionally, adjustments promote stricter type safety and a more responsive filtering mechanism in the website's UI. Overall, these modifications aim to optimize developer experience and expand the capabilities of the editor. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor
participant HistoryManager
participant NodeManager
User->>Editor: Edit Text
Editor->>NodeManager: Update TextNode
NodeManager-->>Editor: Node Updated
Editor->>HistoryManager: Record Change
HistoryManager-->>Editor: History Recorded
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
|
/review |
PR Reviewer Guide 🔍(Review updated until commit 58c0311)
|
|
Persistent review updated to latest commit 58c0311 |
|
@coderabbitai review |
Actions performedReview triggered.
|
|
@coderabbitai full review |
Actions performedFull review triggered. |
![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
Outside diff range, codebase verification and nitpick comments (1)
packages/lexical-clipboard/src/clipboard.ts (1)
270-274: Improved comment clarity.The comments have been updated for grammatical consistency, improving the clarity and professionalism of the code documentation.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- packages/lexical-clipboard/src/clipboard.ts (2 hunks)
- packages/lexical-history/src/tests/unit/LexicalHistory.test.tsx (4 hunks)
- packages/lexical-history/src/index.ts (1 hunks)
- packages/lexical-html/src/index.ts (1 hunks)
- packages/lexical-react/src/LexicalContentEditable.tsx (2 hunks)
- packages/lexical-react/src/LexicalHistoryPlugin.ts (1 hunks)
- packages/lexical-selection/src/lexical-node.ts (2 hunks)
- packages/lexical-website/src/components/Gallery/utils.tsx (1 hunks)
Additional comments not posted (12)
packages/lexical-react/src/LexicalHistoryPlugin.ts (1)
20-28: LGTM! The addition of thedelayparameter enhances flexibility.The changes to the
HistoryPluginfunction by adding an optionaldelayparameter are well-implemented and align with the PR objectives.packages/lexical-website/src/components/Gallery/utils.tsx (1)
54-54: LGTM! The dependency array update improves responsiveness.Including
tagsin the dependency array ofuseFilteredExamplesensures that the component reacts to changes intags, enhancing its responsiveness and accuracy.packages/lexical-react/src/LexicalContentEditable.tsx (1)
18-18: LGTM! Stricter type safety and deprecation strategy are well-handled.The removal of ESLint comments promotes cleaner code, and the adjustments to the
Propstype indicate a thoughtful approach to deprecating theeditorproperty.packages/lexical-html/src/index.ts (1)
106-106: LGTM! Simplified type inference in$cloneWithProperties.The removal of the generic type parameter
<LexicalNode>from the$cloneWithPropertiesfunction call allows TypeScript to automatically infer the type, enhancing flexibility without affecting the function's external interface.packages/lexical-history/src/__tests__/unit/LexicalHistory.test.tsx (4)
46-49: Approved: Addition ofSerializedCustomTextNodetype.The
SerializedCustomTextNodetype effectively extendsSerializedTextNodeto include custom classes, enhancing serialization capabilities.
51-93: Approved: Implementation ofCustomTextNodeclass.The
CustomTextNodeclass provides methods for managing custom classes, which enhances the flexibility and styling capabilities of text nodes.
94-104: Approved: Utility functions forCustomTextNode.The
$createCustomTextNodeand$isCustomTextNodefunctions streamline the creation and identification of custom text nodes, improving usability.
382-439: Approved: Tests forCustomTextNodebehavior.The tests ensure that changes to text content and class attributes of
CustomTextNodeinstances are correctly tracked in the undo history, confirming accurate state management.packages/lexical-history/src/index.ts (1)
196-215: Approved: Refactoring ofisTextNodeUnchangedfunction.The refactoring simplifies the logic for detecting line deletions and ensures accurate node comparison by considering parent nodes and serialized representations.
packages/lexical-clipboard/src/clipboard.ts (1)
259-259: Type parameter removal in$cloneWithProperties.The removal of the type parameter from the
$cloneWithPropertiesfunction call suggests a shift towards inferred type handling. This change is appropriate and aligns with TypeScript's type inference capabilities.packages/lexical-selection/src/lexical-node.ts (2)
39-39: In-place modification for node properties.The return types of
$updateElementNodeProperties,$updateTextNodeProperties, and$updateParagraphNodePropertieshave been changed tovoid, indicating in-place modifications. This change enhances clarity and reduces the cognitive load on developers.Also applies to: 51-51, 61-61
80-85: Adjustment in$cloneWithPropertiesfor in-place updates.The
$cloneWithPropertiesfunction has been updated to reflect the in-place modification changes. The function now directly calls the update functions without expecting a return value, streamlining the cloning process.
Description
TextNodechange detection when only one text leaf is dirty but unchanged$cloneWithProperties- the previous implementation did not correctly handle__textFormatfromParagraphNode. Also updated the very incorrect doc string.$cloneWithPropertiesto the lexical package because the correct implementation was already inlined inLexicalNode.getWritable()A more general fix may be possible, it's unclear whether this hack is really necessary for MaxLengthPlugin but other text node transforms may have similar issues so removing the hack could be backwards incompatible.
Closes #6409
Test plan
Before
History's TextNode change detection did not work correctly for subclasses where only properties that did not exist on TextNode changed.
After
The tests show that the behavior is consistent with expectations
Description by Korbit AI
What change is being made?
Fix text node change detection by updating the
isTextNodeUnchangedfunction, adding a newCustomTextNodeclass, and modifying related test cases and utility functions.Why are these changes being made?
These changes address issue #6409, where text node changes were not being detected properly. The new
CustomTextNodeclass and updated logic ensure that changes to text content and properties are accurately tracked, improving the reliability of the editor's undo/redo functionality.Summary by CodeRabbit
New Features
CustomTextNodeclass for enhanced text styling and manipulation within theLexicalHistorycomponent.delayparameter to theHistoryPlugin, allowing for more control over history management timing.useFilteredExamplesto react to changes intags, improving example filtering capabilities.Bug Fixes
Documentation
Refactor