Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Inspector V2] Prevent inspector tree scroll from bouncing #8367

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

elliette
Copy link
Member

@elliette elliette commented Sep 27, 2024

Fixes #8356

Previously we were determining whether or not to trigger a scroll based on whether the top-left and bottom-right of an item in the tree was visible.

However, for items with long descriptions, we wouldn't actually horizontally scroll to their rightmost corner, which would cause the scroll controller to trigger a scroll that didn't actually do anything.

Now we decide whether to scroll based on whether the "center left half" (red dot below) of the item is in view.

image

scrolling_fix

@elliette elliette requested a review from a team as a code owner September 27, 2024 23:18
@elliette elliette requested review from bkonyi and kenzieschmoll and removed request for a team September 27, 2024 23:18
@elliette elliette changed the title [Inspector V2] Prevent scroll from bouncing [Inspector V2] Prevent inspector tree scroll from bouncing Sep 27, 2024
Copy link
Contributor

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

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

LGTM with one suggestion to add a comment.

@@ -990,9 +990,12 @@ class _InspectorTreeState extends State<InspectorTree>
safeViewportHeight,
);

final centerLeftHalf = Offset(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth leaving some documentation here explaining why we do this and referencing the original issue (or this PR)? ASCII art could work too ;-)

Copy link
Member

Choose a reason for hiding this comment

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

Plus one to a comment and issue link 👍

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

Successfully merging this pull request may close these issues.

Inspector scroll bounces
3 participants