fix: left handle scroll direction on iOS/Android (#1185)
* fix: correct auto-scroll direction for left selection handle drag on iOS When dragging the left selection handle upward, the proxy rect used for auto-scroll detection was built with offset & Size(1, edgeOffset), which extends 220px downward from the selection start. This caused the bottom of the rect to exceed the viewport bottom, triggering a downward scroll (showing content below) — the opposite of the intended direction. Fix by using Rect.fromLTWH(offset.dx, offset.dy - edgeOffset, 1, edgeOffset) so the proxy rect extends upward, only triggering scroll-up when the handle approaches the top edge of the viewport. * chore: update iOS minimum deployment target to 13.0 * test: add auto_scroller proxy rect direction tests Adds regression tests for the left selection handle auto-scroll bug where the proxy rect incorrectly extended downward, triggering scroll in the wrong direction. Verifies the rect extends upward from the handle offset when direction is AxisDirection.up. * fix: remove unused import in auto_scroller_test * chore: code review
This commit is contained in:
parent
301d0fa579
commit
9bb1ca1fc5
6 changed files with 130 additions and 31 deletions
|
|
@ -21,6 +21,6 @@
|
||||||
<key>CFBundleVersion</key>
|
<key>CFBundleVersion</key>
|
||||||
<string>1.0</string>
|
<string>1.0</string>
|
||||||
<key>MinimumOSVersion</key>
|
<key>MinimumOSVersion</key>
|
||||||
<string>12.0</string>
|
<string>13.0</string>
|
||||||
</dict>
|
</dict>
|
||||||
</plist>
|
</plist>
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
# Uncomment this line to define a global platform for your project
|
# Uncomment this line to define a global platform for your project
|
||||||
platform :ios, '12.0'
|
platform :ios, '13.0'
|
||||||
|
|
||||||
# CocoaPods analytics sends network stats synchronously affecting flutter build latency.
|
# CocoaPods analytics sends network stats synchronously affecting flutter build latency.
|
||||||
ENV['COCOAPODS_DISABLE_STATS'] = 'true'
|
ENV['COCOAPODS_DISABLE_STATS'] = 'true'
|
||||||
|
|
|
||||||
|
|
@ -342,7 +342,7 @@
|
||||||
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
|
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
|
||||||
GCC_WARN_UNUSED_FUNCTION = YES;
|
GCC_WARN_UNUSED_FUNCTION = YES;
|
||||||
GCC_WARN_UNUSED_VARIABLE = YES;
|
GCC_WARN_UNUSED_VARIABLE = YES;
|
||||||
IPHONEOS_DEPLOYMENT_TARGET = 12.0;
|
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
|
||||||
MTL_ENABLE_DEBUG_INFO = NO;
|
MTL_ENABLE_DEBUG_INFO = NO;
|
||||||
SDKROOT = iphoneos;
|
SDKROOT = iphoneos;
|
||||||
SUPPORTED_PLATFORMS = iphoneos;
|
SUPPORTED_PLATFORMS = iphoneos;
|
||||||
|
|
@ -420,7 +420,7 @@
|
||||||
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
|
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
|
||||||
GCC_WARN_UNUSED_FUNCTION = YES;
|
GCC_WARN_UNUSED_FUNCTION = YES;
|
||||||
GCC_WARN_UNUSED_VARIABLE = YES;
|
GCC_WARN_UNUSED_VARIABLE = YES;
|
||||||
IPHONEOS_DEPLOYMENT_TARGET = 12.0;
|
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
|
||||||
MTL_ENABLE_DEBUG_INFO = YES;
|
MTL_ENABLE_DEBUG_INFO = YES;
|
||||||
ONLY_ACTIVE_ARCH = YES;
|
ONLY_ACTIVE_ARCH = YES;
|
||||||
SDKROOT = iphoneos;
|
SDKROOT = iphoneos;
|
||||||
|
|
@ -469,7 +469,7 @@
|
||||||
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
|
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
|
||||||
GCC_WARN_UNUSED_FUNCTION = YES;
|
GCC_WARN_UNUSED_FUNCTION = YES;
|
||||||
GCC_WARN_UNUSED_VARIABLE = YES;
|
GCC_WARN_UNUSED_VARIABLE = YES;
|
||||||
IPHONEOS_DEPLOYMENT_TARGET = 12.0;
|
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
|
||||||
MTL_ENABLE_DEBUG_INFO = NO;
|
MTL_ENABLE_DEBUG_INFO = NO;
|
||||||
SDKROOT = iphoneos;
|
SDKROOT = iphoneos;
|
||||||
SUPPORTED_PLATFORMS = iphoneos;
|
SUPPORTED_PLATFORMS = iphoneos;
|
||||||
|
|
|
||||||
|
|
@ -26,6 +26,7 @@
|
||||||
buildConfiguration = "Debug"
|
buildConfiguration = "Debug"
|
||||||
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
|
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
|
||||||
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
|
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
|
||||||
|
customLLDBInitFile = "$(SRCROOT)/Flutter/ephemeral/flutter_lldbinit"
|
||||||
shouldUseLaunchSchemeArgsEnv = "YES">
|
shouldUseLaunchSchemeArgsEnv = "YES">
|
||||||
<MacroExpansion>
|
<MacroExpansion>
|
||||||
<BuildableReference
|
<BuildableReference
|
||||||
|
|
@ -43,6 +44,7 @@
|
||||||
buildConfiguration = "Debug"
|
buildConfiguration = "Debug"
|
||||||
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
|
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
|
||||||
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
|
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
|
||||||
|
customLLDBInitFile = "$(SRCROOT)/Flutter/ephemeral/flutter_lldbinit"
|
||||||
launchStyle = "0"
|
launchStyle = "0"
|
||||||
useCustomWorkingDirectory = "NO"
|
useCustomWorkingDirectory = "NO"
|
||||||
ignoresPersistentStateOnLaunch = "NO"
|
ignoresPersistentStateOnLaunch = "NO"
|
||||||
|
|
|
||||||
|
|
@ -45,7 +45,7 @@ class AutoScroller extends EdgeDraggingAutoScroller
|
||||||
lastDirection = direction;
|
lastDirection = direction;
|
||||||
if (direction != null && direction == AxisDirection.up) {
|
if (direction != null && direction == AxisDirection.up) {
|
||||||
return startAutoScrollIfNecessary(
|
return startAutoScrollIfNecessary(
|
||||||
offset & Size(1, edgeOffset),
|
Rect.fromLTWH(offset.dx, offset.dy - edgeOffset, 1, edgeOffset),
|
||||||
duration: duration,
|
duration: duration,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,38 +1,135 @@
|
||||||
|
import 'package:appflowy_editor/src/editor/editor_component/service/scroll/auto_scroller.dart';
|
||||||
|
import 'package:flutter/material.dart';
|
||||||
import 'package:flutter_test/flutter_test.dart';
|
import 'package:flutter_test/flutter_test.dart';
|
||||||
|
|
||||||
// @LucasXu0 refactor this test
|
/// Captures the rect passed to [startAutoScrollIfNecessary] without triggering
|
||||||
|
/// real scrolling, so we can assert on the proxy rect computation.
|
||||||
|
class _CapturingAutoScroller extends AutoScroller {
|
||||||
|
_CapturingAutoScroller(super.scrollable);
|
||||||
|
|
||||||
|
Rect? capturedRect;
|
||||||
|
|
||||||
|
@override
|
||||||
|
void startAutoScrollIfNecessary(Rect dragTarget, {Duration? duration}) {
|
||||||
|
capturedRect = dragTarget;
|
||||||
|
// intentionally do not call super — we only want the rect
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
void main() {
|
void main() {
|
||||||
group('AutoScroller', () {
|
group('AutoScroller.startAutoScroll', () {
|
||||||
testWidgets('can render', (tester) async {
|
late ScrollableState scrollableState;
|
||||||
// late AutoScroller scroller;
|
|
||||||
|
|
||||||
// await tester.buildAndPump(
|
setUp(() async {});
|
||||||
// AutoScrollableWidget(
|
|
||||||
// scrollController: ScrollController(),
|
|
||||||
// builder: (context, autoScroller) {
|
|
||||||
// scroller = autoScroller;
|
|
||||||
|
|
||||||
// return const SizedBox(width: 600, height: 600);
|
/// Builds a minimal scrollable so we can obtain a real [ScrollableState].
|
||||||
// },
|
Future<void> buildScrollable(WidgetTester tester) async {
|
||||||
// ),
|
await tester.pumpWidget(
|
||||||
// );
|
const MaterialApp(
|
||||||
|
home: SingleChildScrollView(
|
||||||
|
child: SizedBox(height: 2000),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
scrollableState =
|
||||||
|
tester.state<ScrollableState>(find.byType(Scrollable).first);
|
||||||
|
}
|
||||||
|
|
||||||
// await tester.pumpAndSettle();
|
// Regression test for:
|
||||||
|
// When dragging the left selection handle upward on iOS/Android, the screen
|
||||||
|
// was scrolling DOWN (showing content below) instead of UP (showing content
|
||||||
|
// above). The root cause was that the proxy rect used for auto-scroll
|
||||||
|
// detection extended 220px DOWNWARD from the selection start, so when the
|
||||||
|
// handle was in the lower portion of the viewport the rect's bottom exceeded
|
||||||
|
// the viewport edge and triggered a downward scroll.
|
||||||
|
testWidgets(
|
||||||
|
'left handle drag (AxisDirection.up) — proxy rect extends upward from offset',
|
||||||
|
(tester) async {
|
||||||
|
await buildScrollable(tester);
|
||||||
|
|
||||||
// expect(scroller.scrolling, false);
|
final scroller = _CapturingAutoScroller(scrollableState);
|
||||||
// expect(scroller.lastOffset, null);
|
const offset = Offset(100, 500);
|
||||||
|
const edgeOffset = 200.0;
|
||||||
|
|
||||||
// scroller.startAutoScroll(const Offset(10, 0));
|
scroller.startAutoScroll(
|
||||||
// expect(scroller.lastOffset, const Offset(10, 0));
|
offset,
|
||||||
|
edgeOffset: edgeOffset,
|
||||||
|
direction: AxisDirection.up,
|
||||||
|
);
|
||||||
|
|
||||||
// scroller.stopAutoScroll();
|
expect(scroller.capturedRect, isNotNull);
|
||||||
// expect(scroller.lastOffset, null);
|
|
||||||
|
|
||||||
// scroller.startAutoScroll(
|
// Rect must extend UPWARD: top = offset.dy - edgeOffset, bottom = offset.dy
|
||||||
// const Offset(10, 0),
|
expect(
|
||||||
// direction: AxisDirection.up,
|
scroller.capturedRect!.top,
|
||||||
// );
|
offset.dy - edgeOffset,
|
||||||
});
|
reason: 'proxy rect should start edgeOffset pixels ABOVE the handle',
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
scroller.capturedRect!.bottom,
|
||||||
|
offset.dy,
|
||||||
|
reason:
|
||||||
|
'proxy rect bottom should be at the handle position, not below it',
|
||||||
|
);
|
||||||
|
|
||||||
|
// Guard: bottom must NOT be offset.dy + edgeOffset (the old buggy value
|
||||||
|
// that caused downward scrolling when the handle was near the bottom of
|
||||||
|
// the viewport).
|
||||||
|
expect(
|
||||||
|
scroller.capturedRect!.bottom,
|
||||||
|
isNot(offset.dy + edgeOffset),
|
||||||
|
reason:
|
||||||
|
'old buggy value extended the rect downward, triggering wrong-direction scroll',
|
||||||
|
);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
testWidgets(
|
||||||
|
'right handle drag (AxisDirection.down) — proxy rect is centered on offset',
|
||||||
|
(tester) async {
|
||||||
|
await buildScrollable(tester);
|
||||||
|
|
||||||
|
final scroller = _CapturingAutoScroller(scrollableState);
|
||||||
|
const offset = Offset(100, 300);
|
||||||
|
const edgeOffset = 200.0;
|
||||||
|
|
||||||
|
scroller.startAutoScroll(
|
||||||
|
offset,
|
||||||
|
edgeOffset: edgeOffset,
|
||||||
|
direction: AxisDirection.down,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(scroller.capturedRect, isNotNull);
|
||||||
|
|
||||||
|
// For AxisDirection.down the generic Rect.fromCenter path is used.
|
||||||
|
expect(
|
||||||
|
scroller.capturedRect!.center,
|
||||||
|
offset,
|
||||||
|
reason:
|
||||||
|
'right handle proxy rect should be centered on the handle offset',
|
||||||
|
);
|
||||||
|
expect(scroller.capturedRect!.width, edgeOffset);
|
||||||
|
expect(scroller.capturedRect!.height, edgeOffset);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
testWidgets(
|
||||||
|
'no direction — proxy rect is centered on offset',
|
||||||
|
(tester) async {
|
||||||
|
await buildScrollable(tester);
|
||||||
|
|
||||||
|
final scroller = _CapturingAutoScroller(scrollableState);
|
||||||
|
const offset = Offset(50, 200);
|
||||||
|
const edgeOffset = 100.0;
|
||||||
|
|
||||||
|
scroller.startAutoScroll(offset, edgeOffset: edgeOffset);
|
||||||
|
|
||||||
|
expect(scroller.capturedRect, isNotNull);
|
||||||
|
expect(scroller.capturedRect!.center, offset);
|
||||||
|
expect(scroller.capturedRect!.width, edgeOffset);
|
||||||
|
expect(scroller.capturedRect!.height, edgeOffset);
|
||||||
|
},
|
||||||
|
);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue