Skip to content

feat(autofix): Better autoscroll on autofix drawer#114869

Merged
Zylphrex merged 5 commits intomasterfrom
txiao/feat/better-autoscroll-on-autofix-drawer
May 6, 2026
Merged

feat(autofix): Better autoscroll on autofix drawer#114869
Zylphrex merged 5 commits intomasterfrom
txiao/feat/better-autoscroll-on-autofix-drawer

Conversation

@Zylphrex
Copy link
Copy Markdown
Member

@Zylphrex Zylphrex commented May 5, 2026

Make sure to scroll to the bottom when each section is finished loading. Also make sure to cancel the auto scroll behaviour if the user scrolled upwards.

Make sure to scroll to the bottom when each section is finished loading. Also
make sure to cancel the auto scroll behaviour if the user scrolled upwards.
@Zylphrex Zylphrex requested a review from a team as a code owner May 5, 2026 17:00
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 5, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.40%


return (
<Flex direction="column" gap="lg">
<Flex ref={containerRef} onScroll={onScrollHandler} direction="column" gap="lg">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The onScroll handler is attached to a non-scrollable <Flex> element instead of the scrollable parent, so it will never be triggered.
Severity: MEDIUM

Suggested Fix

Move the onScroll={onScrollHandler} prop from the inner <Flex> element in content.tsx to the StyledDrawerBody component in body.tsx. This will attach the event listener to the element that actually scrolls, allowing the onScrollHandler to be correctly triggered.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/components/events/autofix/v3/content.tsx#L70

Potential issue: The `onScroll` handler is attached to an inner `<Flex>` element which
is not scrollable. The actual scrollable container is the parent `StyledDrawerBody`.
Because `onScroll` events do not bubble in React 17+, the `onScrollHandler` will never
be called. This prevents the `canAutoScroll` state from ever being updated to `false`.
As a result, the feature that cancels auto-scrolling when a user scrolls up is
non-functional, and the user will always be force-scrolled to the bottom when the
autofix run completes.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment thread static/app/components/events/autofix/v3/content.tsx Outdated
Comment thread static/app/components/events/autofix/v3/artifactLoadingDetails.tsx Outdated
Comment thread static/app/components/events/autofix/v3/content.tsx Outdated
Comment thread static/app/utils/useAutoScroll.tsx
Comment thread static/app/utils/useAutoScroll.tsx
Comment thread static/app/utils/useAutoScroll.tsx Outdated
import {defined} from 'sentry/utils';

interface UseAutoScrollOptions {
key: any;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
key: any;
key: unknown;

Copy link
Copy Markdown
Member

@ryan953 ryan953 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better with de-duplicated util 👍

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit dc3751b. Configure here.

Comment thread static/app/utils/useAutoScroll.tsx
Comment thread static/app/utils/useAutoScroll.tsx Outdated
Comment thread static/app/utils/useAutoScroll.tsx Outdated
Comment on lines +9 to +11
export function useAutoScroll({key}: UseAutoScrollOptions) {
const canAutoScroll = useRef(true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The canAutoScroll ref is not reset on new autofix runs, permanently disabling auto-scrolling if the user scrolls up once.
Severity: MEDIUM

Suggested Fix

Reset the canAutoScroll ref to true when a new autofix run is initiated. This can be achieved by adding a dependency to the useEffect that triggers the scroll, or by exposing a reset function from the useAutoScroll hook that can be called when restarting the run.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/utils/useAutoScroll.tsx#L9-L11

Potential issue: The `canAutoScroll` ref in the `useAutoScroll` hook is initialized to
`true` but is not reset when a new autofix run begins. Since the `SeerDrawer` component
remains mounted across runs, if a user scrolls up during one run,
`canAutoScroll.current` is set to `false`. When a new run is started, this `false` value
persists, causing the `useEffect` responsible for scrolling to exit early. This
permanently disables the auto-scroll functionality for all subsequent runs until the
component is fully unmounted, defeating the feature's purpose.

@Zylphrex Zylphrex merged commit 8d0eaeb into master May 6, 2026
73 checks passed
@Zylphrex Zylphrex deleted the txiao/feat/better-autoscroll-on-autofix-drawer branch May 6, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants