Repository navigation
Allow detection of taps on TabBar - #23919
Conversation
|
This will need a test. |
|
Generally speaking I'd like to keep callbacks such that not setting one and setting one to a no-op callback (as in Not really sure what the best way to do that here would be, though... cc @HansMuller |
|
I have opened an issue so there can be proper discussion (should have done this before) #24224. A potential solution would be to handle this in the |
| /// is null then the text style of the theme's body2 definition is used. | ||
| final TextStyle unselectedLabelStyle; | ||
|
|
||
| /// An option callback when the [TabBar] is tapped. |
There was a problem hiding this comment.
An optional callback that's called when the [TabBar] is tapped.
The callback is applied to the index of the tab where the tap occurred.
This callback has no effect on the default handling of taps. It's for applications that want to do a little extra work when a tab is tapped, even if the tap doesn't change the TabController's index. TabBar [onTap] callbacks should not make changes to the TabController since that would interfere with the default tap handler.
| child: TabBar( | ||
| controller: controller, | ||
| tabs: tabs.map<Widget>((String tab) => Tab(text: tab)).toList(), | ||
| onTap: (int index) { |
There was a problem hiding this comment.
need to fix the indent here
| controller: controller, | ||
| tabs: tabs.map<Widget>((String tab) => Tab(text: tab)).toList(), | ||
| onTap: (int index) { | ||
| if (controller.index != index) { |
There was a problem hiding this comment.
This is what we don't want developers to do; onTap is a "me too" notification.
Maybe this is leftover from a previous version of the PR?
| semantics.dispose(); | ||
| }); | ||
|
|
||
| testWidgets('can be notified of TabBar onTap behavior', (WidgetTester tester) async { |
There was a problem hiding this comment.
This test looks like it needs to be revised. It should confirm that the onTap() call is applied to the correct index and that it happens even when the default tap handler doesn't have any effect on the TabController.
|
@HansMuller Thanks for the review, I believe these have now been resolved. |
HansMuller
left a comment
There was a problem hiding this comment.
This looks good, just needs a few trivial changes.
| /// | ||
| /// This callback has no effect on the default handling of taps. It's for | ||
| /// applications that want to do a little extra work when a tab is tapped, | ||
| /// even if the tap doesn't change the TabController's index. TabBar [onTap] |
There was a problem hiding this comment.
extra leading space on this line and the next
| await tester.pumpWidget(buildFrame(tabs: tabs, controller: controller)); | ||
| await tester.tap(find.text('C')); | ||
| await tester.pump(); | ||
| await tester.pump(const Duration(seconds: 1)); |
There was a problem hiding this comment.
tester.pumpAndSetttle() here and below
|
|
||
| testWidgets('can be notified of TabBar onTap behavior', (WidgetTester tester) async { | ||
| int tabIndex = -1; | ||
| bool hasBeenTapped = false; |
There was a problem hiding this comment.
This appears to be redundant with tabIndex != -1 and existing expects already check that.
HansMuller
left a comment
There was a problem hiding this comment.
LGTM
I will land this as soon as the repo is open again.
…#25342) * Revert "Add imports section to sample code templates, and more docs. (#25184)" This reverts commit 95b0124. * Revert "Use stderr instead of stdout to contain errors in flutter attach test (#25305)" This reverts commit 2b819dd. * Revert "Allow detection of taps on TabBar (#23919)" This reverts commit 01694ab. * Revert "a549981da Roll src/third_party/skia a69b10312977..5eb29448dfbd (1 commits) (flutter-team-archive/engine#7211) (#25333)" This reverts commit a37099f. * Revert "Revert "obscureText and enableInteractiveSelection defaults (#24527)" (#25335)" This reverts commit c545706. * Password fields are no longer selectable nor copiable
Fixes #24224
I have added a callback to the
TabBarthat is called if not null. This would fix all 3 of these examples.