search

ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
8.28k stars 3.59k forks source link

Fixed delete range fixing inside attribute elements. #16293

Closed niegowski closed 2 weeks ago

niegowski commented 2 weeks ago

Suggested merge commit message (convention)

Fix (autoformat): Pressing the backspace after autoformat should retain the typed content after undoing the block format change. Closes #16240.

Internal (typing): Fixed the target range size verification for backspace handling. See #16240.

Additional information

For example – encountered issues, assumptions you had to make, other affected tickets, etc.

coderabbitai[bot] commented 2 weeks ago

Walkthrough

The updates to the ckeditor5-typing package involve refining the deletion logic within the editor. Modifications include enhanced element counting in deletion scenarios and adjustments to test setups to better encapsulate different content structures. These changes aim to improve the accuracy and flexibility of deletion operations across various text and element configurations.

Changes

File Path Change Summary
.../ckeditor5-typing/src/deleteobserver.ts Enhanced logic for counting elements during deletion based on nextPosition and item properties.
.../ckeditor5-typing/tests/deleteobserver.js Updated viewSetData calls to include <container:p> tags and added new test cases for diverse content structures.

Assessment against linked issues

Objective Addressed Explanation
It's impossible to undo creating a list item while retaining the list marker when there is already some content in the editable (#16240) ❓ The provided summary does not explicitly mention changes related to handling list item creation or undo scenarios. Further review of the full code changes would be needed to confirm if the issue is addressed.

The assessment indicates that while the changes improve element handling and testing scenarios, it is unclear from the summary alone whether the specific issue related to undoing list item creation while retaining the list marker is directly addressed. Further detailed review of the code changes would be necessary to provide a definitive answer.

Recent Review Details **Configuration used: CodeRabbit UI** **Review profile: CHILL**
Commits Files that changed from the base of the PR and between 2916738bf55259d899c58e37f6f4c96b34e83c4c and 5617c30f0ddec63f47107a9cfedef4f7979430d0.
Files selected for processing (2) * packages/ckeditor5-typing/src/deleteobserver.ts (2 hunks) * packages/ckeditor5-typing/tests/deleteobserver.js (10 hunks)
Additional comments not posted (8)
packages/ckeditor5-typing/src/deleteobserver.ts (1)
Line range hint `345-360`: Refined handling of target ranges in delete operations. The modifications to the loop in `shouldUseTargetRanges` function enhance the precision in counting relevant elements, which is crucial for the correct functionality of backspace operations in various contexts. Good job on covering different types of elements and conditions.
packages/ckeditor5-typing/tests/deleteobserver.js (7)
`402-402`: Updated test case to reflect changes in the handling of delete operations. The update to use `` tags in `viewSetData` calls aligns with the changes in the main codebase and ensures that the tests accurately reflect the application's expected structure. --- `422-422`: Ensure comprehensive testing of delete operations on combined symbols. The test case correctly checks the behavior of delete operations on combined symbols, ensuring that the deletion handles code points accurately. --- `441-441`: Added test case for multi-character deletion scenarios. This test case is crucial for verifying that the delete operation correctly handles scenarios where multiple characters are selected. It ensures that the `selectionToRemove` is set appropriately. --- `468-468`: Test case for single emoji sequence deletion. Proper testing of emoji sequence handling in delete operations is essential for ensuring that such sequences are treated as single code points, which this test case successfully verifies. --- `487-487`: Test case for deletion of multiple emoji sequences. This test case effectively checks the behavior when multiple emoji sequences are involved in the deletion, ensuring that the entire selection is handled correctly. --- `514-514`: Test for handling collapsed target ranges in delete operations. It's important to verify that collapsed ranges are handled correctly, ensuring that no content is removed unintentionally. This test case addresses that scenario effectively. --- `552-552`: Test case for deletions spanning different parent nodes. This test case is essential for ensuring that deletions spanning across different parent nodes are handled correctly, setting the `selectionToRemove` appropriately to reflect the actual content that needs to be deleted.
--- 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?
Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` 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) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.