Repository navigation
Add a scrollbar for horizontal scrolling of source files in the debugger - #3262
Conversation
Merge with master
|
Not sure which test I should modify for this change. I was thinking maybe |
| controller: textController, | ||
| // Only listen for vertical scroll notifications (ignore those | ||
| // from the nested horizontal SingleChildScrollView): | ||
| notificationPredicate: (ScrollNotification notification) => |
There was a problem hiding this comment.
We should use notification.depth to predicate, otherwise, all the descendants of Scrollables with vertical axis will interfere.
There was a problem hiding this comment.
Thanks, changed to use notificationDepth.
There was a problem hiding this comment.
are we sure this notificationPredicate is needed? If it is needed, there are likely multiple other places in the UI where we need notificationPredicates. I thought it wasn't needed as we explicitly specify the scrollController.
There was a problem hiding this comment.
The notificationPredicate is needed, otherwise the vertical scrollbar responds to horizontal scroll events (and shows up at the bottom along with the horizontal scrollbar, see image). I'm guessing scrollbars in other parts of the UI would be similarly broken if this was needed for them?
There was a problem hiding this comment.
Hi @xu-baolin! I see you added the notificationPredicate 😄 Could you clarify what the purpose of the notificationPredicate is vs. the scrollController? Is the scrollController no longer used? Thanks!
There was a problem hiding this comment.
Oh this may already be fixed, we noticed this recently in flutter/flutter#87697.
The scrollbar uses the scroll controller but also listens and responds to scroll events. This is useful like in this 2D case where the scrollbars might want to know about both axes and respond when either one scrolls. They just aren't behaving correctly here, should be fixed by flutter/flutter#87698.
|
|
|
Nice job! |
5 participants
Demo: