Skip to content

fix(web): anchor popup animations to their aligned edge - #22812

Closed
brandon-julio-t wants to merge 6 commits into
pingdotgg:mainfrom
brandon-julio-t:fix/thread-details-animation-origin
Closed

brandon-julio-t wants to merge 6 commits into
pingdotgg:mainfrom
brandon-julio-t:fix/thread-details-animation-origin

Conversation

@brandon-julio-t

@brandon-julio-t brandon-julio-t commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Thread Details scales sideways when opened with the right panel visible: Base UI 1.5.0 derives the origin from the wide chat header’s center, although the popup is end-aligned. This adopts Base UI’s existing fix #5015, first released in 1.8.0.

The popup now scales from its aligned edge. Centered, collision-shifted, and arrow-bearing popups retain their anchor-aware origin. The app’s animation duration and scale are unchanged.

Reproduction and motion evidence

At 1280 × 800, open a thread, open the right panel, and toggle Thread Details. The right panel’s empty launcher is enough to reproduce it. Watch the card’s right edge while it opens and closes; the settled layout is intentionally unchanged.

Before — Base UI 1.5.0, normal speed

before.mp4

After — Base UI 1.8.0, normal speed

after.mp4
Enlarged motion detail — 0.25× playback

Crops of the same recordings, slowed in playback only. The application’s timings were not changed. The host label is blurred in all recordings.

Before

before-detail.mp4

After

after-detail.mp4

Frame measurements of the same 296px popup: origin changes from 58px 0px to 296px 0px; its right edge changes from 731.24–736px during the transition to a fixed 736px. These are geometry observations, not a frame-rate benchmark.

Scope and compatibility

This uses the small, focused bug-fix exception: an existing animation defect is reproduced above and the correction already exists upstream. Related PRs #22434 and #22614 address timing and layout switching, respectively; neither fixes the origin.

This does upgrade shared UI behavior, including the web renderer used by Electron. Reviewed the cumulative 1.6–1.8 release notes and T3’s imports. The documented OTP Field rename does not apply; T3 does not import it. The lockfile updates only Base UI and its required Floating UI dependencies, preserving other consumers’ resolutions. Native React Native does not consume this package.

The PR changes only the web dependency manifest and lockfile. The current sidebar uses upstream’s controlled popover state; its release-notes lifecycle is covered by the focused browser tests below.

Verification

  • vp install --frozen-lockfile passes.
  • vp run check:changed passes formatting, lint, knip, and 34 affected package typechecks on the final candidate.
  • vp run --filter @t3tools/web build passed during the original verification. It was not rerun for this dependency-manifest/lockfile conflict resolution.
  • After merging current main, 29 focused tests across 11 files pass, covering menus/popovers, settings color and writing controls, Thread Details PR rows, update controls, and upstream’s controlled sidebar release-notes popover. See the command below. The earlier broader audit also passed 51 tests across 24 files.
  • Live Chromium: Thread Details at 1280 × 800 and 390 × 844; Escape/focus return; model search and keyboard selection; permissions-select keyboard navigation; advanced-typography switch; color popover nested inside the theme dialog, including Escape returning focus without closing the parent.
  • Independent Codex review found no actionable P0–P2 findings in the dependency diff. This supplements the execution evidence; it is not comprehensive application coverage.

Limits: native Electron and Safari/Firefox were not exercised. Native iOS/Android and the full repository CI suites were not run locally. The first browser-test attempt lacked Chromium; installing the required Playwright browser resolved that setup failure. These checks provide evidence for the tested paths, not a guarantee that every shared primitive is regression-free.

Focused test commands
vp test run apps/web/src/components/chat/OpenInPicker.browser.test.tsx apps/web/src/components/chat/ComposerStashMenu.browser.test.tsx apps/web/src/components/chat/AssistantCitationChip.browser.test.tsx apps/web/src/components/GitActionsControl.browser.test.tsx apps/web/src/components/settings/colorPickers.browser.test.tsx apps/web/src/components/settings/SourceControlWritingSettings.browser.test.tsx apps/web/src/components/chat/ThreadDetailsPrRow.browser.test.tsx apps/web/src/components/ServerUpdateAction.browser.test.tsx apps/web/src/components/desktopUpdate.logic.test.ts apps/web/src/components/sidebar/updateEverything.logic.test.ts apps/web/src/components/sidebar/SidebarUpdatePill.browser.test.tsx

Implemented and verified with GPT-6-Astra and GPT-6.1-Sol through the Codex harness in T3 Code; independently reviewed with GPT-6.1-Sol through Codex.

Toggling Thread Details with the right panel open makes the card scale
sideways: its header-wide anchor puts the animation origin near the
opposite edge. In the browser reproduction, the 296px popup uses an
origin of 58px and its right edge moves 4.76px during the transition.

Adopt Base UI 1.8.0, the first release containing mui/base-ui#5015:
mui/base-ui#5015
The released fix uses the aligned edge for arrowless, unshifted popups
and retains anchor-aware positioning for centered or shifted popups.
The same T3 workflow now uses a 296px origin with a stationary right
edge. Keep the lockfile update limited to Base UI and its required
Floating UI dependencies, preserving other consumers' resolutions.

Verified the actual web UI at 1280x800 and 390x844, repeated open/close,
Escape focus return, and the model picker. check:changed passes format,
knip, and 32 package typechecks; 7 focused browser suites pass 8 tests.
Frozen installation passes. Native Electron and mobile were not run.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 21, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 1d6e561

Macroscope's review found this PR approvable — The PR is confined to ignored dependency metadata and lockfile paths, with no non-ignored source, schema, security, billing, deployment, product-default, or static-analysis changes. Its scope is consistent with a focused dependency-based UI bug fix.

Notes:

  • No code objects were reviewed. Approvability was decided on eligibility alone.

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

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Files ignored due to path filters: 1
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
Run configuration
Commits

Reviewing files that changed from the base of the PR and between a07e912 and a11da55.

Files ignored due to path filters: 1
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between e74e43a and a07e912.

Files ignored due to path filters: 1
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
Files selected for processing: 1
  • apps/web/package.json

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

Walkthrough

The web app updates the @base-ui/react dependency range from ^1.4.1 to ^1.8.0.

Changes: Web app dependency

Layer / File(s) Summary
Update Base UI version range
apps/web/package.json
The @base-ui/react dependency range changes from ^1.4.1 to ^1.8.0.

Priority: Normal

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Suggested labels: bugfix

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · previous review a07e9

The dependency update has no identified merge-blocking risk.

Architecture risk: Low · up to 1d6e5

The change affects 1 system.

Changed systems: apps/web

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/web (ui) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/web/package.json: The @base-ui/react dependency constraint changed from ^1.4.1 to ^1.8.0.
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main fix: anchoring popup animations to the aligned edge.
Description check Passed The description covers the problem, change, scope and approval rationale, verification evidence, visual motion evidence, compatibility impact, and known testing limits.
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.
Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

brandon-julio-t and others added 4 commits October 21, 2026 17:38
Base UI 1.8 warns when a detached handle is closed. SidebarUpdateControl
runs its closing effect even when the web/no-update path renders no
Popover.Root, producing two warnings during a StrictMode mount.

Close only an open handle. A detached handle reports isOpen=false;
attached open release notes still close when update visibility changes.
This is a compatibility adjustment for the Base UI upgrade in this PR.

Verified the warning disappears on a fresh dev-page load, Thread Details
still opens with its right-edge origin, and 18 focused update tests pass.
check:changed passes formatting, lint, knip, and 32 package typechecks.
Resolve the lockfile conflict between the popup-origin fix's Floating UI
utils entry and main's new Inter font entry by retaining both package and
snapshot records. Preserve main's dependency updates and the Base UI
1.8.0 upgrade with its existing popover lifecycle guard.

The frozen-lockfile install and focused formatting checks pass. The diff
against main remains confined to the original three PR files.
Keep main's new document-preview dependencies alongside the Base UI
1.8.0 upgrade. Adopt main's controlled sidebar release-notes popover,
which removes the effect previously protected by this branch's guard.
The PR now changes only the web dependency manifest and lockfile.

The merged frozen lockfile installs successfully and focused formatting
passes. Required checks and popup regression tests run before pushing.

maria-rcks commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

@brandon-julio-t, your upstream fix preserves the collision-shift and arrow cases that a local CSS override would lose. i kept your implementation and commits, and merged current main (dd5b5401e1) without rebasing. the final diff is still only apps/web/package.json and pnpm-lock.yaml: base ui 1.8 and its required floating ui dependencies. nothing was dropped or added beyond the main merge.

verification at 3607a88cd8d51b9259f57ea417d209513f9aec25:

  • blacksmith: frozen install, vp run check:changed (format, knip, unused-patch validation, 34 package typechecks), 44 existing tests across 19 focused files, and the production web build passed. no source files changed, so the changed-file lint stage had no applicable inputs.
  • real chrome client: main reproduces a 58px origin on the 296px thread details popup; this head uses 296px. repeated toggles and escape passed at 1280×800 and 390×844, including light/dark narrow layouts. model-menu keyboard navigation/escape and settings opening passed.
  • focused browser coverage includes menus, color pickers, typography, nested dialogs, sidebar release notes, toast, image dialogs, ssh prompts, provider setup and linking pull requests. two independent read-only reviews approved this exact head with no findings.
  • unverified: native electron, native mobile, safari/firefox, and the full ci suites. github's CI run 40401311537 ended action_required before creating jobs; fork workflow approval is still needed. other preview workflows have the same gate. gitcafe's status is currently stale, so i checked the pushed head and actions directly on github. no labels were changed.

media note: the uploads load correctly and the video plays and seeks in chrome. gitcafe currently renders these external embeds as links; its native attachment upload returned 409 stale-operation twice after refreshing status, so inline media on gitcafe remains unavailable.

before, current main:

main thread details with right panel open

after, same data and layout:

upgraded thread details with right panel open

opening/closing detail, main on the left and this pr on the right, normal speed:

popup opening and closing comparison

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified as-is at 3607a88cd8d51b9259f57ea417d209513f9aec25 on a disposable linux boat vm. no source changes or pushes were needed.

  • installed and ran github/main (0c80751701) in a separate worktree, then stopped its captured process group before starting the exact pr head. drove both real dev web clients with the repo's playwright and /usr/bin/google-chrome.
  • added a project pointing at the real checkout through the app, selected codex gpt-6.1-sol, and sent a prompt. the main turn returned popup verification ready.. copied that app-created state for the comparison; a further turn on the pr returned pr provider path verified.. no mocks, replay provider, fixtures, or demo data were used for these runtime checks.
  • at 1280×800, opened the right panel and toggled thread details three times. main reproduced the sideways motion; the pr kept the right edge fixed. these are frame geometry observations, not a performance benchmark:
                         main               pr head
popup width              296px              296px
transform origin         58px 0px           296px 0px
right edge during motion 731.24–736px        736–736px
  • escape dismissed thread details and returned focus to its trigger. the model picker opened, accepted keyboard navigation, and dismissed. keyboard selection changed permissions to supervised; selecting full access restored them. a nested theme accent-color popup opened and escape retained the parent theme editor. thread details also opened and dismissed at 390×844.
  • blacksmith passed vp run check:changed dd5b5401e1f240f4314af29edf431369d182f386: format, unused patches, both knip passes, and 34 package typechecks. fetching two levels of the pr's history resolved the worker's initial missing-merge-base setup failure.
  • blacksmith passed 7 existing tests in ThreadDetailsPrRow.browser.test.tsx, colorPickers.browser.test.tsx, and ComposerStashMenu.browser.test.tsx.

before and after use the same conversation snapshot and viewport; the checked-out branch and its git metadata differ. inspected both full-size captures and the exported recording. uploaded images load at 1280×800; the uploaded video plays through and seeks in t3's browser.

main thread details with right panel open

pr thread details with the same conversation and right panel open

main and pr popup opening and closing at normal speed

Not verified

native electron, ios/android, windows/macos, safari/firefox, and remote/relay/tunnel connections were not exercised. the 390×844 check is the web client, not a native mobile client. other provider adapters and every other shared base ui consumer were not exhaustively exercised.

github's full ci and preview workflows remain action_required at this head, including ci run 40401311537. the remote checks above passed independently; this does not mean github ci is green.

@t3dotgg

t3dotgg commented Oct 31, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Closing: this is covered by #27796, which landed. Reopen if a needed part is still missing.

@t3dotgg t3dotgg closed this Oct 31, 2026
@brandon-julio-t
brandon-julio-t deleted the fix/thread-details-animation-origin branch November 1, 2026 03:27
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:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants