Skip to content

Chore: Mobile: Fixes #14834: Fix JSDOM scrollIntoView error in tests#14870

Open
jellyfrostt wants to merge 3 commits intolaurent22:devfrom
jellyfrostt:fix/scrollIntoView-polyfill
Open

Chore: Mobile: Fixes #14834: Fix JSDOM scrollIntoView error in tests#14870
jellyfrostt wants to merge 3 commits intolaurent22:devfrom
jellyfrostt:fix/scrollIntoView-polyfill

Conversation

@jellyfrostt
Copy link

AI Assistance Disclosure

This PR was developed with AI assistance (Claude). AI was used for:

  • Drafting the polyfill implementation
  • Grammar check the wording and phrases used in drafting PR message
  • Reviewing adherence to Joplin coding style

All generated code was reviewed, understood, and validated with local test runs.

Problem

After Note.test.tsx tests complete, a deferred setTimeout in afterFullPageRender.ts calls element.scrollIntoView(). JSDOM does not implement Element.prototype.scrollIntoView, so this throws a TypeError that surfaces as a "Cannot log after tests are done" warning. The existing JSDOM mock (ExtendedWebView/index.jest.tsx) already polyfilled window.scrollBy but not scrollIntoView.

Solution

Extracted the inline scrollBy polyfill from index.jest.tsx into a shared polyfill string (polyfillScrollFunctions.ts) and added a guarded no-op for Element.prototype.scrollIntoView. The polyfill runs inside the isolated JSDOM context via dom.window.eval(), so it does not affect Jest's global jsdom environment or other test files.

This is a low-risk change as it only adds a no-op function to an isolated test mock. No production code is modified.

Test plan

  • yarn test Note.test - 21/21 pass
  • yarn test handleAnchorClick - 5/5 pass (file not modified, unaffected by change)
  • 'yarn test-ci' - 36/37 suites pass (1 pre-existing flaky failure in NoteEditor.test.tsx, unrelated to the change in the PR)
Screenshot 2026-03-23 132418 Screenshot 2026-03-23 135537

Additional things to note: we recognized one failing test (NoteEditor.test.tsx) in the full suite and identified that the failing test is a pre-existing flaky test that is unrelated to the changes in this PR. It fails intermittently due to timing issues in the editor component tests and was already failing prior to the changes made in this PR.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2026

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 23, 2026

📝 Walkthrough

Walkthrough

This change refactors scroll-related polyfill logic in the mobile app's extended WebView component. A new utility module is created containing reusable JavaScript snippet strings for polyfilling window.scrollBy and Element.prototype.scrollIntoView, which replaces the previously inline polyfill code in the test setup configuration.

Changes

Cohort / File(s) Summary
Ignore files
.eslintignore, .gitignore
Added exclusion patterns for the new polyfill utility file to prevent linting and version control tracking.
Polyfill module
packages/app-mobile/components/ExtendedWebView/utils/polyfillScrollFunctions.ts
New utility module exporting a JavaScript snippet string that defines no-op polyfills for window.scrollBy and Element.prototype.scrollIntoView when not natively available.
Test configuration
packages/app-mobile/components/ExtendedWebView/index.jest.tsx
Refactored to import and use the new polyfillScrollFunctions module instead of hardcoding inline window.scrollBy polyfill logic during JSDOM setup.

Suggested labels

mobile, bug

Suggested reviewers

  • laurent22
  • personalizedrefrigerator
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description is comprehensive and directly related to the changeset, detailing the problem, solution, and test results with appropriate context.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Pr Description Must Follow Guidelines ✅ Passed The PR description fulfils all requirements: clear problem statement, comprehensive solution explanation, and concrete test plan results. It avoids restating implementation details and provides purposeful context including scope clarification and code provenance disclosure.
Title check ✅ Passed The title clearly and specifically summarizes the main change: extracting and enhancing a polyfill for scrollIntoView errors in JSDOM tests, which aligns with the core purpose of this PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@coderabbitai coderabbitai bot added bug It's a bug mobile All mobile platforms labels Mar 23, 2026
@jellyfrostt
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Mar 23, 2026
@personalizedrefrigerator personalizedrefrigerator changed the title Mobile: Fixes #14834: Fix JSDOM scrollIntoView error in tests Chore: Mobile: Fixes #14834: Fix JSDOM scrollIntoView error in tests Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug It's a bug mobile All mobile platforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants