coderabbitai[bot]
commented
3 weeks ago Walkthrough
The updates involve enhancing the CKEditor5's handling of composition and selection events, particularly addressing issues with predictive text in Safari. Changes include importing new event types, refining event listeners in the typing module, and adjusting how events are fired in the observer to better manage text insertion and selection behaviors.
Changes
| File Path | Changes |
|---|---|
.../ckeditor5-engine/src/view/observer/selectionobserver.ts.../ckeditor5-typing/src/input.ts.../ckeditor5-typing/src/inserttextobserver.ts |
Imported new event types, adjusted event handling for specific types, and modified event firing method to improve text insertion and selection behaviors. |
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| Address predictive text "double typing" effect in Safari (Issue #16106) | ❓ | The changes seem relevant to improving event handling, which could potentially address the double typing effect. However, without specific details on how these changes directly impact the predictive text behavior in Safari, it's unclear if the issue is fully resolved. |
The assessment suggests that while the changes are pertinent to event handling, further details or testing may be required to confirm the resolution of the specific Safari predictive text issue.
Recent Review Details
**Configuration used: CodeRabbit UI**Commits
Files that changed from the base of the PR and between 1e6187d1da1616132f9ec89fb46b06ba2d8803f9 and 84e74039c002430bfcb497ada5f8d5b596c96652.Files selected for processing (4)
* packages/ckeditor5-engine/src/view/observer/selectionobserver.ts (5 hunks) * packages/ckeditor5-engine/tests/view/observer/selectionobserver.js (2 hunks) * packages/ckeditor5-typing/tests/input.js (3 hunks) * packages/ckeditor5-typing/tests/inserttextobserver.js (1 hunks)Files skipped from review as they are similar to previous changes (1)
* packages/ckeditor5-engine/src/view/observer/selectionobserver.tsAdditional comments not posted (6)
packages/ckeditor5-typing/tests/inserttextobserver.js (1)
`189-189`: Ensure the `selection` property is correctly set to `undefined` as expected. This change aligns with the new behavior where the `selection` property should not be part of the event data, simplifying the handling process.packages/ckeditor5-typing/tests/input.js (3)
`11-11`: Ensure the import of `DomEventData` is correctly utilized in the file. The addition of `DomEventData` is crucial for handling DOM events accurately within the tests. --- `131-151`: Verify the logic for using model document selection when the selection property is not passed. This change ensures that the model document selection is used as a fallback, which is a sensible default behavior. --- `195-227`: Check the synchronization of model and DOM selections during composition events. This addition is important for ensuring that the model and DOM selections are synchronized, which is critical during composition events to maintain consistency between the view and the model.packages/ckeditor5-engine/tests/view/observer/selectionobserver.js (2)
`21-21`: Ensure the import of `priorities` is correctly utilized in the file. The addition of `priorities` is crucial for handling event priorities accurately within the tests. --- `196-236`: Check the new test case for firing `selectionChange` synchronously on composition start. This test is important for ensuring that the `selectionChange` event behaves as expected during composition start, especially considering the priority handling.
niegowski
Suggested merge commit message (convention)
Fix (typing, engine): Tracking the composed range so content can be replaced while composing. Closes #16106.
Additional information
For example – encountered issues, assumptions you had to make, other affected tickets, etc.
Summary by CodeRabbit
selectionChangeevent on composition start events at the lowest priority level.