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.

Sign up for GitHub

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

Add animationDuration property to useTabController hook #446

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

Add animationDuration property to useTabController hook #446

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
83f146a
Add animationDuration property to useTabController hook
d-polikhranidi Sep 26, 2024
ffc0d2a
Update use_tab_controller_test.dart
d-polikhranidi Oct 2, 2024
34a47fd
Update packages/flutter_hooks/test/use_tab_controller_test.dart
rrousselGit Oct 2, 2024
85156a5
Update packages/flutter_hooks/test/use_tab_controller_test.dart
rrousselGit Oct 2, 2024
cbd4e66
Update use_tab_controller_test.dart
d-polikhranidi Oct 2, 2024
a92e2fe
dart format and update import in hooks.dart
Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions packages/flutter_hooks/lib/src/tab_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ part of 'hooks.dart';
/// - [TabController]
TabController useTabController({
required int initialLength,
Duration? animationDuration = kTabScrollDuration,
TickerProvider? vsync,
int initialIndex = 0,
List<Object?>? keys,
Expand All @@ -17,6 +18,7 @@ TabController useTabController({
vsync: vsync,
length: initialLength,
initialIndex: initialIndex,
animationDuration: animationDuration,
keys: keys,
),
);
Expand All @@ -27,23 +29,24 @@ class _TabControllerHook extends Hook<TabController> {
required this.length,
required this.vsync,
required this.initialIndex,
List<Object?>? keys,
}) : super(keys: keys);
required this.animationDuration,
super.keys,
});

final int length;
final TickerProvider vsync;
final int initialIndex;
final Duration? animationDuration;

@override
HookState<TabController, Hook<TabController>> createState() =>
_TabControllerHookState();
HookState<TabController, Hook<TabController>> createState() => _TabControllerHookState();
}

class _TabControllerHookState
extends HookState<TabController, _TabControllerHook> {
class _TabControllerHookState extends HookState<TabController, _TabControllerHook> {
late final controller = TabController(
length: hook.length,
initialIndex: hook.initialIndex,
animationDuration: hook.animationDuration,
vsync: hook.vsync,
);

Expand Down
21 changes: 21 additions & 0 deletions packages/flutter_hooks/test/use_tab_controller_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,27 @@ void main() {
verifyNoMoreInteractions(vsync);
ticker.dispose();
});

testWidgets('initial animationDuration matches with real constructor', (tester) async {
late TabController controller;
late TabController controller2;

final vsync = TickerProviderMock();
final ticker = Ticker((_) {});
when(vsync.createTicker((_) {})).thenReturn(ticker);

await tester.pumpWidget(
HookBuilder(
builder: (context) {
controller = useTabController(initialLength: 4);
controller2 = TabController(length: 4, vsync: vsync);
return Container();
},
),
);

expect(controller.animationDuration, controller2.animationDuration);
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve test implementation and description

The test case correctly verifies that the animationDuration matches between controllers created with useTabController and the TabController constructor. However, there are a few improvements we can make:

  1. Use useSingleTickerProvider() instead of TickerProviderMock as suggested in the past review comments.
  2. Make the test description more specific about what it's testing.

Here's a suggested implementation:

testWidgets('useTabController creates TabController with matching animationDuration', (tester) async {
  late TabController hookController;
  late TabController constructorController;

  await tester.pumpWidget(
    HookBuilder(
      builder: (context) {
        final vsync = useSingleTickerProvider();
        hookController = useTabController(initialLength: 4);
        constructorController = TabController(length: 4, vsync: vsync);
        return Container();
      },
    ),
  );

  expect(hookController.animationDuration, constructorController.animationDuration);
});

This implementation addresses the previous comments and improves the test's clarity and consistency with the rest of the test suite.

});
}

Expand Down
Loading