Skip to content

Conversation

@BryanValverdeU
Copy link
Contributor

Sometimes the onBlur and onFocus is not triggered from different logical roots.
To fix, when a logical root changes, re-attach the handlers to the new logical root so we can cache the selection in the onBlur event and re-create the selection on focus.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds support for re-attaching selection-related event handlers whenever the logical root changes, ensuring proper cleanup and re-initialization.

  • Introduces a logicalRootDisposer field and extends dispose() to clean it up.
  • Handles the logicalRootChanged event by re-attaching focus, blur (non-Safari), and drop handlers.
  • Adds unit tests for non-Safari, Safari, and repeated logical root change scenarios.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts Added tests covering logicalRootChanged handling.
packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts Implemented logicalRootChanged event handling.
Comments suppressed due to low confidence (2)

packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts:3169

  • DEFAULT_DARK_COLOR_SUFFIX_COLOR is not imported or defined in this test file, causing a ReferenceError. Consider importing the constant from its module or replacing it with a literal mock value.
getDarkColor: (color: string) => `${DEFAULT_DARK_COLOR_SUFFIX_COLOR}${color}`

packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts:3209

  • DEFAULT_DARK_COLOR_SUFFIX_COLOR is referenced here but not defined. Add the necessary import or use a mock string literal to avoid test failures.
getDarkColor: (color: string) => `${DEFAULT_DARK_COLOR_SUFFIX_COLOR}${color}`

@BryanValverdeU BryanValverdeU merged commit e976c67 into master Jun 5, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants