Skip to content

Use passive listener for link editor scrolling - #10610

Open
sanjulaonline wants to merge 2 commits into
wso2:masterfrom
sanjulaonline:fix/27880-passive-scroll-listener
Open

sanjulaonline wants to merge 2 commits into
wso2:masterfrom
sanjulaonline:fix/27880-passive-scroll-listener

Conversation

@sanjulaonline

@sanjulaonline sanjulaonline commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Purpose

Fix the React Doctor client-passive-event-listeners finding for the rich text link editor scroll listener.

Root cause: features/admin.flow-builder-core.v1/components/resource-property-panel/rich-text/helper-plugins/link-plugin.tsx registered the body scroll listener without { passive: true }:

scrollerElem.addEventListener(scroll, update);

The update callback only reads the Lexical editor state and repositions the floating link editor. It does not accept the scroll event, does not call preventDefault(), and does not need to block scrolling, so passive mode is safe.

This PR adds a typed AddEventListenerOptions constant with passive: true and uses it for both addEventListener and removeEventListener. Cleanup remains correct because listener removal matches by event type, callback, and capture value; using the same options object keeps the intent clear. The resize listener is unchanged.

Patch changesets are included for @wso2is/admin.flow-builder-core.v1 and @wso2is/console per the repository changeset policy for features changes.

Related Issues

Related PRs

  • N/A

Verification

  • pnpm dlx pnpm@10.33.0 --version - Passed, confirmed 10.33.0.
  • pnpm dlx pnpm@10.33.0 install - Passed.
  • pnpm dlx pnpm@10.33.0 run build:modules - Passed.
  • .\node_modules\.bin\eslint.cmd features/admin.flow-builder-core.v1/components/resource-property-panel/rich-text/helper-plugins/link-plugin.tsx - Passed.
  • git diff --check - Passed.
  • Static red/green check:
    • origin/master exact non-passive listener count: 1.
    • Working tree exact non-passive listener count: 0.
    • Working tree passive listener count: 1.
  • Confirmed update does not accept the scroll event and does not call preventDefault(). Existing preventDefault() calls in the file are unrelated keyboard/click handlers.
  • CodeRabbit changeset policy finding was addressed with a normal follow-up commit adding the @wso2is/console changeset entry.

Checks not completed locally

  • Unit test not added: this package does not expose an obvious focused test seam for this Lexical plugin, and adding a browser/Lexical test harness for a listener option change would be disproportionate.
  • React Doctor local red/green is UNPROVEN: the repository exposes React Doctor through .github/workflows/react-doctor.yml, but no supported local command/script was found.
  • ..\..\node_modules\.bin\tsc.cmd --noEmit --project tsconfig.json --pretty false from features/admin.flow-builder-core.v1 was attempted after dependency install and shared module build, but it fails on existing workspace package/type resolution errors such as Cannot find module '@wso2is/react-components' and Cannot find module '@wso2is/forms/legacy' outside this change.
  • .\node_modules\.bin\rollup.cmd --config rollup.config.cjs from features/admin.flow-builder-core.v1 was attempted after dependency install and shared module build. It emitted the first bundle but failed while bundling declarations on existing generated declaration/style resolution, e.g. Could not resolve ./visual-flow.scss from dist/esm/types/components/visual-flow/visual-flow.d.ts.

Checklist

  • e2e cypress tests locally verified. (for internal contributers)
  • Manual test round performed and verified.
  • UX/UI review done on the final implementation.
  • Documentation provided. (Add links if there are any)
  • Relevant backend changes deployed and verified
  • Unit tests provided. (Add links if there are any)
  • Integration tests provided. (Add links if there are any)

Security checks

Developer Checklist (Mandatory)

  • Complete the Developer Checklist in the related product-is issue to track any behavioral change or migration impact.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: d17fd9c8-2e22-49e3-b4a5-a61c5a44ca6c

📥 Commits

Reviewing files that changed from the base of the PR and between a7adb10 and 1adf08d.

📒 Files selected for processing (2)
  • .changeset/passive-link-editor-scroll.md
  • features/admin.flow-builder-core.v1/components/resource-property-panel/rich-text/helper-plugins/link-plugin.tsx

📝 Walkthrough

Walkthrough

The link editor now registers and removes its scroll listener with { passive: true }. A patch Changeset records the update for @wso2is/admin.flow-builder-core.v1.

Changes

Link editor scroll handling

Layer / File(s) Summary
Passive scroll listener handling
features/admin.flow-builder-core.v1/components/resource-property-panel/rich-text/helper-plugins/link-plugin.tsx, .changeset/passive-link-editor-scroll.md
The link editor uses passive options for scroll listener registration and cleanup. A patch Changeset documents the change.

Suggested reviewers: mahima-sanketh-git


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Changeset Required ❌ Error A new changeset exists, but the changed source is under features and its header lists only admin.flow-builder-core.v1; the required @wso2is/console entry is missing. Update .changeset/passive-link-editor-scroll.md to include "@wso2is/console": patch alongside the feature package.
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code makes the affected scroll listener passive after confirming it does not call preventDefault(), satisfying issue #27880.
Out of Scope Changes check ✅ Passed The changes are limited to the targeted scroll listener and its patch changeset; the resize listener remains unchanged.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly and concisely identifies the main change: using a passive listener for link editor scrolling.
Description check ✅ Passed The description covers the purpose, related issue, implementation details, verification, limitations, checklist, security checks, and developer checklist.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ create changeset

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sanjulaonline

Copy link
Copy Markdown
Contributor Author

Hi @Mahima-Sanketh-Git, could you take a look at this one when you have a moment?

This PR has no workflow runs — the only statuses are the CLA check and CodeRabbit, so the build and tests have never actually executed. Could you approve the workflow runs for it?

It addresses wso2/product-is#27880 by making the link editor's scroll listener passive. The update handler does not call preventDefault(), so this is safe; the resize listener is intentionally left as is.

Happy to make any changes you'd like.

@Mahima-Sanketh-Git

Copy link
Copy Markdown
Contributor

Looks good to me! Thanks for the fix.
@pavinduLakshan , could you approve the workflow runs when you get a chance?

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.

[React Doctor] client-passive-event-listeners: "scroll" listener without { passive: true } — blocks scrolling performance (1 occurrence)

2 participants