Skip to content

fix: respect reduced motion in programmatic scrolling - #12871

Closed
saphid wants to merge 3 commits into
pingdotgg:mainfrom
saphid:fix/consistency-reduced-motion
Closed

saphid wants to merge 3 commits into
pingdotgg:mainfrom
saphid:fix/consistency-reduced-motion

Conversation

@saphid

@saphid saphid commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

What Changed

Programmatic scrolling now reads the current reduced-motion preference when the action runs. Theme token reveal, timeline minimap and send anchors, scroll-to-bottom, keyboard page steps, tabs, settings navigation and citation reveal share the web behavior; native message-to-end actions query the device preference. Missing or unavailable preference APIs select instant movement.

Scroll destinations, focus, selection, direct manipulation, held scrolling and keyboard dismissal retain their existing behavior. The contribution includes the cross-entry-point guidance in #12660, whose documentation commit is still part of this branch.

Verification

  • 43 focused web tests passed across ThemeEditorPanel, settingsLayout, pageScrollController and AssistantCitationSource. The unavailable-preference regressions fail five cases with the original helper and pass after the repair.
  • Web and mobile package typechecks and scoped lint/format passed. Independent GPT-6.1 Sol review found no actionable source findings and independently reran the 43 web tests.
  • Checks used the integrated 38969148 toolchain. A prospective merge onto main 35be904f is clean and preserves the exact contribution; that newer Vite toolchain was not tested here.
  • Current integrated web/Electron and native before-and-after motion proof remains missing, including live preference changes and mobile keyboard/scroll timing. Source and unit checks do not prove native animation behavior. This is not yet a merge-ready claim.

Historical evidence from September 20 below predates the current repair and integration. It used an isolated proof app with matchMedia stubbed to reduced motion, comparing base d6f291303 with the then-current branch. It demonstrates that earlier PageDown scenario only; it does not verify the current revision or a native client.

Implementation and readiness repair: GPT-6 Astra using Codex in T3 Code. Independent review: GPT-6.1 Sol using Codex in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 21, 2026
@saphid
saphid marked this pull request as ready for review September 21, 2026 10:24
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 21, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d92242b

Macroscope's review found this PR approvable — This is a focused reduced-motion accessibility fix that preserves existing destinations and interaction behavior while removing unnecessary animated travel across established web and mobile scroll paths. The production logic is small and supported by targeted preference and fallback tests, with the larger diff primarily consisting of tests and documentation.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 43d71bf0-198d-4c65-9c7c-a65633ec2ae2

📥 Commits

Reviewing files that changed from the base of the PR and between d71e51a and d92242b.

📒 Files selected for processing (11)
  • apps/mobile/src/features/threads/ThreadDetailScreen.tsx
  • apps/web/src/components/ChatView.tsx
  • apps/web/src/components/RightPanelTabs.tsx
  • apps/web/src/components/chat/AssistantCitationSource.test.ts
  • apps/web/src/components/chat/MessagesTimeline.tsx
  • apps/web/src/components/chat/pageScrollController.test.ts
  • apps/web/src/components/settings/ThemeEditorPanel.test.tsx
  • apps/web/src/components/settings/settingsLayout.test.tsx
  • apps/web/src/components/settings/settingsLayout.tsx
  • apps/web/src/lib/reducedMotion.ts
  • docs/internals/consistency-reduced-motion.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Web and mobile scrolling and reveal actions now account for reduced-motion preferences. The web changes use a shared preference helper, including a fallback for unavailable browser APIs. Mobile thread scrolling checks the device accessibility preference.

Changes

Reduced-motion behavior

Layer / File(s) Summary
Preference helper and page scrolling
apps/web/src/lib/reducedMotion.ts, apps/web/src/components/chat/pageScrollController.ts, apps/web/src/components/chat/pageScrollController.test.ts, docs/internals/consistency-reduced-motion.md
Adds shared reduced-motion detection and immediate page scrolling when enabled. Tests cover unavailable browser APIs and preference changes. The documentation describes reduced-motion constraints across clients.
Chat and panel scrolling
apps/web/src/components/ChatView.tsx, apps/web/src/components/RightPanelTabs.tsx, apps/web/src/components/chat/AssistantCitationSource.tsx, apps/web/src/components/chat/AssistantCitationSource.test.ts, apps/web/src/components/chat/MessagesTimeline.tsx
Uses the shared preference reader to select animated or immediate scrolling in chat and panel actions. Citation tests cover reduced motion and unavailable browser preference APIs.
Settings reveal scrolling
apps/web/src/components/settings/ThemeEditorPanel.tsx, apps/web/src/components/settings/ThemeEditorPanel.test.tsx, apps/web/src/components/settings/settingsLayout.tsx, apps/web/src/components/settings/settingsLayout.test.tsx
Uses the reduced-motion preference for theme and settings target reveals. Tests cover preference changes and missing matchMedia.
Mobile thread scrolling
apps/mobile/src/features/threads/ThreadDetailScreen.tsx
Checks the device accessibility preference for submitted-message and manual scroll-to-end actions. Guards asynchronous scrolling against thread changes.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge, yash-singh1

Merge Risk: ⚪ Minimal · up to d9224

Programmatic scrolling now follows the reduced-motion preference on web and mobile, with instant movement when the preference cannot be read. No merge-blocking issue was found. Native mobile behavior was verified only by tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 13 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: respecting reduced-motion preferences during programmatic scrolling.
Description check ✅ Passed The description is detailed and directly explains the changes, rationale, scope, validation, and known verification limits. It does not reproduce the template’s explicit Why, UI Changes, or Checklist …
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 13 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

@macroscopeapp
macroscopeapp Bot dismissed their stale review September 30, 2026 18:59

Dismissing prior approval to re-evaluate d92242b

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

Closing for missing current motion verification. At d92242b, the change affects web/Electron reveal and scrolling paths plus native mobile scroll-to-end and keyboard timing. I inspected the linked historical desktop recordings and context image. As the description explains, they predate this repair, use a stubbed preference in a proof app, and do not verify the current integrated clients or native behavior.

The focused preference/fallback tests are useful. The verification requirement also calls for before/after UI evidence and a short recording for motion or timing changes. Please provide current web/Electron and native before/after proof covering a live preference change, the affected scroll/reveal actions and mobile keyboard/scroll timing, while showing preserved destinations and focus. Identify the tested commit and results, then request reconsideration.

@saphid

saphid commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Following up on the closure for missing current motion verification: the bug is now filed as #15953, with live before/after proof on main a1d9d72aef. The fix is split into two focused replacement PRs:

This branch's fallback-only changes (tabs, settings search, citations) are dropped, because those paths already honor the preference on main. Android and the macOS desktop app with the real system setting are not verified; both PRs say so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants