You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(web): keep settled pagination focused while loading - #35522
A desktop user reported that clicking "Show 25 more" in the unfiltered Settled sidebar jumps the list back to the open thread. The loading state disables the focused pagination button, dropping focus and allowing focus recovery to choose another sidebar row.
Change
Keep the loading button focusable with aria-disabled and ignore activation while a page is loading. This preserves focus without allowing repeated pagination requests. Web and desktop share this sidebar; native mobile is unchanged.
Scope
This is a small fix for a reported pagination bug and needs no separate issue. Searches found no matching open PR. Related #34475 changes pagination length after project filter changes, rather than loading focus.
Verification
The new browser regression fails before the fix because focus moves from the loading button to the Settled header. With the fix it retains focus and the existing scroll offset, and another activation during loading reveals no additional rows.
All 8 sidebar browser tests pass.
vp run check:changed passes formatting, lint, both Knip checks, affected typechecks, 7 test files, and the React compiler check.
The reporter supplied a desktop recording showing the scroll jump. The desktop app itself has not been exercised with the fix, and no before/after desktop captures are attached.
Macroscope's review found this PR approvable — The settled-thread pagination button retains keyboard focus during loading while explicitly blocking repeated activation. This is a narrowly scoped interaction fix with no changes to pagination defaults or data handling.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
The settled-thread “Show more” button now ignores clicks while the older-thread shelf is loading. A browser test checks the loading state, viewport position, and rendered thread-row count.
Changes: Settled-thread pagination
Layer / File(s)
Summary
Guard pagination while loading apps/web/src/components/Sidebar.tsx, apps/web/src/components/Sidebar.browser.test.tsx
The button uses aria-disabled and a loading guard to prevent retrying pagination or increasing the visible thread count while loading. The browser test checks that loading preserves viewport position and a second click adds no rows.
The settled-thread "Show more" button stays focused while loading and ignores repeat clicks. No merge-blocking risk was found; the desktop app was not exercised with the fix.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title 'fix(web): keep settled pagination focused while loading' is a concise conventional commit title that names the main change: keeping focus on the 'Show 25 more' button during loading. The ra…
Description check
The description covers the template's main sections: Problem (focus drops when the disabled button is used, letting focus recovery jump the list), Change (aria-disabled button with activation ignored …
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title 'fix(web): keep settled pagination focused while loading' is a concise conventional commit title that names the main change: keeping focus on the 'Show 25 more' button during loading. The raw summary confirms the Sidebar.tsx change (aria-disabled instead of disabled, ignored activation while loading) and a regression test for focus and scroll position.
Full details: Description check
Explanation
The description covers the template's main sections: Problem (focus drops when the disabled button is used, letting focus recovery jump the list), Change (aria-disabled button with activation ignored while loading), Scope (small bug fix, no separate issue needed, related #34475 noted), and Verification (regression test fails before the fix and passes after, 8 sidebar browser tests pass, check:changed passes, and the desktop app was not exercised). The description does not link a bug issue, and the Scope section says none is needed. It also lacks before/after screenshots, though the template asks for them only for UI changes and the author notes that no desktop captures are attached. The check passes because the description is mostly complete. The template's trailing agent-attribution line is present as 'GPT 6.1 Sol through the Codex harness.'
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Reviewing files that changed from the base of the PR and between fc2b5de and 8e0ee71.
Files selected for processing: 2
apps/web/src/components/Sidebar.browser.test.tsx
apps/web/src/components/Sidebar.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
…6 (Running browser tests in apps/web)
Failing check: Running browser tests in apps/web
Browser tests in apps/web are passing again. Two test files had failed, each for a different reason, and both came from changes upstream made to the code around fork code.
**`src/routes/-chat.index.browser.test.tsx`** (fork-owned, 3 failing tests): upstream pingdotgg#35442 added a `useSettings` import to `components/ui/sidebar.tsx`. Through that import, the route under test now loads `themePalette` → `~/state/server` and `~/state/environments`. The fork test replaces both modules with mocks that only export what the route itself used, so loading the page failed with "does not provide an export named …" (`themeServerEnvironmentThemesAtom`, and after that `usePrimaryEnvironment`). Adding the real exports back to those mocks would only have exposed the next missing one, so I changed the test to replace `../components/ui/sidebar` with a plain `<main>` instead. `SidebarInset` is the only thing the route uses from that module, and this puts the test back to the dependencies it had before the merge.
**`src/components/Sidebar.browser.test.tsx`** (upstream test, 1 failing): upstream's new settled-pagination test (pingdotgg#35522) gives the sidebar a stand-in server config that has no `environment` field. The fork's `CheckoutStatus` hook in `apps/web/src/fork/project-lifecycle/CheckoutStatus.tsx` read `config.environment.capabilities` and crashed on it. I left upstream's test alone and changed the fork hook to read `environment?.capabilities`, with a one-line comment saying why. Real configs always include `environment`, so the app behaves the same as before.
Only fork-owned files changed, and no seams or `docs/fork/seams.tsv` rows were affected. I reran both files with `pnpm exec vp test run --project browser`: 11 of 11 tests pass. Lint and the format check are clean on the two edited files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size:XS0-9 changed lines (additions + deletions).vouch:trustedAuthor is trusted by repo permissions or the VOUCHED list.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Problem
A desktop user reported that clicking "Show 25 more" in the unfiltered Settled sidebar jumps the list back to the open thread. The loading state disables the focused pagination button, dropping focus and allowing focus recovery to choose another sidebar row.
Change
Keep the loading button focusable with
aria-disabledand ignore activation while a page is loading. This preserves focus without allowing repeated pagination requests. Web and desktop share this sidebar; native mobile is unchanged.Scope
This is a small fix for a reported pagination bug and needs no separate issue. Searches found no matching open PR. Related #34475 changes pagination length after project filter changes, rather than loading focus.
Verification
vp run check:changedpasses formatting, lint, both Knip checks, affected typechecks, 7 test files, and the React compiler check.GPT 6.1 Sol through the Codex harness.