flutter
CarouselView children are not clickable
Steps to reproduce
Create any CarouselView with children that are clickable.
Expected results
I expect to be able to trigger the onTap of the children passed into the CarouselView.
Actual results
Nothing happens when I click on the items. I am forced to use CarouseView's onTap. This is limiting and restricts further customization. What if I want each item to have its own onTap behavior but want to avoid writing conditional logic on the onTap. Perhaps I might want to trigger different actions depending on where a user might click on a single item.
If you run the following code in DartPad you can see that the print statement never gets called really
Code sample
Code sample
import 'package:flutter/material.dart';
/// Flutter code sample for [Carousel].
void main() => runApp(const CarouselExampleApp());
class CarouselExampleApp extends StatelessWidget {
const CarouselExampleApp({super.key});
@override
Widget build(BuildContext context) {
return MaterialApp(
debugShowCheckedModeBanner: false,
home: Scaffold(
appBar: AppBar(
title: const Text('Carousel Sample'),
),
body: const CarouselExample(),
),
);
}
}
class CarouselExample extends StatelessWidget {
const CarouselExample({super.key});
@override
Widget build(BuildContext context) {
return Center(
child: ConstrainedBox(
constraints: const BoxConstraints(maxHeight: 200),
child: CarouselView(
itemExtent: 330,
shrinkExtent: 200,
// Reduced the number of items for simplicity
children: List<Widget>.generate(5, (int index) {
return GestureDetector(
onTap: () {
print('Clicked on Item $index');
},
child: UncontainedLayoutCard(index: index, label: 'Item $index'),
);
}),
),
),
);
}
}
class UncontainedLayoutCard extends StatelessWidget {
const UncontainedLayoutCard({
super.key,
required this.index,
required this.label,
});
final int index;
final String label;
@override
Widget build(BuildContext context) {
return ColoredBox(
color: Colors.primaries[index % Colors.primaries.length].withOpacity(0.5),
child: Center(
child: Text(
label,
style: const TextStyle(color: Colors.white, fontSize: 20),
overflow: TextOverflow.clip,
softWrap: false,
),
),
);
}
}
Screenshots or Video
No response
Logs
No response
Flutter Doctor output
Doctor output
[✓] Flutter (Channel stable, 3.24.2, on macOS 15.1 24B5035e darwin-arm64, locale en-US)
• Flutter version 3.24.2 on channel stable at /Users/ericktorres-moreno/development/flutter
• Upstream repository https://github.com/flutter/flutter.git
• Framework revision 4cf269e36d (2 days ago), 2024-09-03 14:30:00 -0700
• Engine revision a6bd3f1de1
• Dart version 3.5.2
• DevTools version 2.37.2
[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
• Android SDK at /Users/ericktorres-moreno/Library/Android/sdk
• Platform android-34, build-tools 34.0.0
• Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
• Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)
• All Android licenses accepted.
[✓] Xcode - develop for iOS and macOS (Xcode 16.1)
• Xcode at /Applications/Xcode-beta.app/Contents/Developer
• Build 16B5001e
• CocoaPods version 1.15.2
[✓] Chrome - develop for the web
• Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome
[✓] Android Studio (version 2022.3)
• Android Studio at /Applications/Android Studio.app/Contents
• Flutter plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/9212-flutter
• Dart plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/6351-dart
• Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)
[✓] VS Code (version 1.92.2)
• VS Code at /Applications/Visual Studio Code.app/Contents
• Flutter extension version 3.96.0
[✓] Connected device (2 available)
• iPhone 15 Pro (mobile) • ios • com.apple.CoreSimulator.SimRuntime.iOS-18-1 (simulator)
• Chrome (web) • chrome • web-javascript • Google Chrome 128.0.6613.120
[✓] Network resources
• All expected network resources are available.
• No issues found!
CarouselView has built-int onTap which is a Inkwell and the GestureDetector tap event get skipped by this.
return CarouselView(
itemExtent: 325,
itemSnapping: true,
onTap: (int index) {
//
},
What if I want each item to have its own onTap behavior but want to avoid writing conditional logic on the onTap. Perhaps I might want to trigger different actions depending on where a user might click on a single item.
@erickjtorres Do you expect CarouselView to have other GestureDetector actions like onDoubleTap, onLongPress, etc?
I would like to be able to interact with the children passed into the CarouselView. The biggest reason is to allow for a user to click on different parts of an image or item and trigger different behavior.
Perhaps this other example makes it more clear. If I wanted to create temp or placeholder image where a user can click on an icon to either add a photo or take a picture. There is not an easy way to do this with just the onTap that CarouselView provides:
import 'package:flutter/material.dart';
/// Flutter code sample for [Carousel].
void main() => runApp(const CarouselExampleApp());
class CarouselExampleApp extends StatelessWidget {
const CarouselExampleApp({super.key});
@override
Widget build(BuildContext context) {
return MaterialApp(
debugShowCheckedModeBanner: false,
home: Scaffold(
appBar: AppBar(
title: const Text('Carousel Sample'),
),
body: const CarouselExample(),
),
);
}
}
class CarouselExample extends StatelessWidget {
const CarouselExample({super.key});
@override
Widget build(BuildContext context) {
return Center(
child: ConstrainedBox(
constraints: const BoxConstraints(maxHeight: 200),
child: CarouselView(
itemExtent: 330,
shrinkExtent: 200,
// Reduced the number of items for simplicity
children: List<Widget>.generate(5, (int index) {
return UncontainedLayoutCard(index: index, label: 'Item $index');
}),
),
),
);
}
}
class UncontainedLayoutCard extends StatelessWidget {
const UncontainedLayoutCard({
super.key,
required this.index,
required this.label,
});
final int index;
final String label;
@override
Widget build(BuildContext context) {
return ColoredBox(
color: Colors.primaries[index % Colors.primaries.length].withOpacity(0.5),
child: Column(
mainAxisAlignment: MainAxisAlignment.center,
children: [
Row(
mainAxisAlignment: MainAxisAlignment.center,
children: [
GestureDetector(
onTap: () {
print('Camera icon clicked on Item $index');
},
child: const Icon(Icons.camera_alt, color: Colors.white, size: 30),
),
const SizedBox(width: 20),
GestureDetector(
onTap: () {
print('Photo icon clicked on Item $index');
},
child: const Icon(Icons.photo, color: Colors.white, size: 30),
),
],
),
],
),
);
}
}
From the implementation I see that we use a Stack to layout the Inkwell and the child. Perhaps we can make the Inkwell conditional to whether onTap is null or not? This way it does not absorb the pointer if the dev wants to allow for a user to interact with the child items. Seems weird to me anyway that a ripple effect happens even if no onTap function is passed in.
This will perhaps solve the problem I am having and also prevent the end user from thinking an item/image in a CarouselView is actionable when in fact it is not.
/// ...flutter/lib/src/material/carousel.dart
Stack(
fit: StackFit.expand,
children: <Widget>[
widget.children.elementAt(index),
Material(
color: Colors.transparent,
child: widget.onTap != null
? InkWell(
onTap: () {
widget.onTap?.call(index);
},
overlayColor: widget.overlayColor ??
MaterialStateProperty.resolveWith((Set<MaterialState> states) {
if (states.contains(MaterialState.pressed)) {
return theme.colorScheme.onSurface.withOpacity(0.1);
}
if (states.contains(MaterialState.hovered)) {
return theme.colorScheme.onSurface.withOpacity(0.08);
}
if (states.contains(MaterialState.focused)) {
return theme.colorScheme.onSurface.withOpacity(0.1);
}
return null;
}),
)
: null, // No InkWell if onTap is null
),
],
)
Same issue here. The solution @erickjtorres proposed would solve it for us I believe.
It really limited implementation of a video player nested inside a carousel view.
We are also currently unable to use CarouselView for wizard type behaviour (no way to do go / no-go buttons).
I agree that deferring to children when onTap is null is a good solution.
I wonder if a mechanism that allows taps to pass to children (or a whole container using Colors.transparent, similar to GestureDetector) would be useful. That way we can use the onTap in the CarouselView if we want to, but if we define children the events will pass through to them instead (like a GestureDetector).
Thank you all for sharing the case. I haven't seen this feature on its M3 specs, so I will mark this as a feature request. If anyone sees one, please share it here.
In this duplicate issue #155199, I propose that we refrain from directly using the onTap property. There are two reasons behind this suggestion.
-
it may cause some test cases to fail and alter the default behavior, which could be considered a breaking change.
-
if we intend to retain the entire
CarouselViewonTapparameter and its child items’onTapbehavior, it would result in another breaking change.
Therefore, I suggest introducing a new parameter called disabledChildrenInteraction to address this issue in this PR and provide greater flexibility in handling the aforementioned scenarios.
/// Whether the default tap interaction for children is disabled.
///
/// If true, tap events for all children are disabled by covering them with an [InkWell].
/// This prevents direct interaction with child widgets.
///
/// If false, tap events are passed through to the child widgets, allowing them
/// to handle interactions directly.
///
/// Defaults to true.
///
/// Note: Setting this to false while also providing an [onTap] callback will
/// throw an assertion error, as these options are mutually exclusive.
final bool disabledChildrenInteraction;
Actually if the second case work, it could be further like that.
if (disabledChildrenInteraction)
return Stack(children[children[index], InkWell])
else
return GestureDetector(onTap, child: children[index]) // It can help to solve the second case.
What do you think?
I think it's important to have consistency with other widgets that have optional splash effects, and the button widgets' behavior is determined based on whether or not the onPressed field is null.
Here are a few options to consider:
Match the button widgets
There have been 4 pull requests opened over the past couple of months adding the option to remove the Material / InkWell widgets shown on top of the Carousel item by default.
- #153162
- #154745
- #155214
- #155218
They all started out with something like the following:
children: <Widget>[
widget.children[index],
+ if (widget.onTap != null)
Material(
color: Colors.transparent,
child: InkWell(
onTap: () => widget.onTap?.call(index),
overlayColor: effectiveOverlayColor,
),
),
],
I think it'd be possible to do this, if we also make an update to the constructor:
class CarouselView extends StatefulWidget {
const CarouselView({
// ...
this.onTap = createSplash,
});
final ValueChanged<int>? onTap;
/// Creates an ink splash inside the Carousel item
/// without any additional effects.
static void createSplash(int index) {}
}
By setting up the default argument this way, the widget would still have a splash effect when no onTap argument is passed; doing CarouselView(onTap: null) would remove it.
Use a boolean flag, and change every button to match
I think the discussion in https://github.com/dart-lang/language/issues/2232 has several good points, notably:
If passing
nulldoes not mean the same as passing nothing, I believe that the API is already confusing to users.
I'd love to change this:
ElevatedButton(
onPressed: someCondition
? () {
// ...
}
: null,
)
to this:
ElevatedButton(
enabled: someCondition,
onPressed: () {
// ...
},
)
Perhaps we could merge #155214 with the boolean flag, in hopes that someday the buttons will be reworked to follow suit.
(The flag should probably be something like enableSplash, since putting "disabled" in the name goes against the avoid double negatives guideline.)
More children
Instead of removing the InkWell, why not just show things on top?
class CarouselView extends StatefulWidget {
final List<Widget> children;
final List<Widget?>? foregroundChildren;
}
Though I'm guessing that even if it doesn't block a button from working, a developer would still want to have the choice of whether to make a splash when the area outside the button is tapped.
(The flag should probably be something like
enableSplash, since putting "disabled" in the name goes against the avoid double negatives guideline.)
I agree with you that the name is very bad in this case.
More children
Instead of removing the
InkWell, why not just show things on top?class CarouselView extends StatefulWidget { final List<Widget> children; final List<Widget?>? foregroundChildren; }Though I'm guessing that even if it doesn't block a button from working, a developer would still want to have the choice of whether to make a splash when the area outside the button is tapped.
That is pretty good idea. I would vote for it.
( Only one small problem is that Dart does not support union type, so we cannot define the children with both Widget? and List<Widget?>?. Developer have to call the builder method even if every foregroundChildren are the same. Beside that, it it the best proposal.
When I was typing this comment, I have a new idea. What if we replace both children and foregroundChildren with a builder function that has a default foreground widget, similar to other builder methods? This way, the developer can choose whether to use the foreground widget or not.
/// Somewhere in CarouselView
Widget builder(int index, Widget foreground);
Only one small problem is that Dart does not support union type, so we cannot define the children with both
Widget?andList<Widget?>?.
I too am very much hoping for union types, but fortunately I don't think they're necessary here.
I was thinking something along the lines of
CarouselView(
children: [image1, image2, image3],
foregroundChildren: [buttons, null, null],
);
The buttons would be shown on top of image1, and the other children wouldn't have anything on top.
I have a new idea. What if we replace both
childrenandforegroundChildrenwith a builder function that has a default foreground widget, similar to other builder methods? This way, the developer can choose whether to use the foreground widget or not./// Somewhere in CarouselView Widget builder(int index, Widget foreground);
I'd be in favor of this, though instead of replacing children, it would ideally be implemented as a separate CarouselView.builder() constructor in order to maintain backward compatibility.
I too am very much hoping for union types, but fortunately I don't think they're necessary here.
I was thinking something along the lines of
CarouselView( children: [image1, image2, image3], foregroundChildren: [buttons, null, null], );The
buttonswould be shown on top ofimage1, and the other children wouldn't have anything on top.
I was worried about the cased that we already have a loop in the CarouselView widget, and we may need to define another two loops when passing the parameter. If we are passing const List of widget, that will not be a problem.
I'd be in favor of this, though instead of replacing
children, it would ideally be implemented as a separateCarouselView.builder()constructor in order to maintain backward compatibility.
Thank you for this suggestion. I learned a lot.
we already have a loop in the
CarouselViewwidget, and we may need to define another two loops when passing the parameter.
I don't think any additional loops would be necessary :)
children: <Widget>[
widget.children[index],
Material(
color: Colors.transparent,
child: InkWell(
onTap: () => widget.onTap?.call(index),
overlayColor: effectiveOverlayColor,
),
),
if (widget.foregroundChildren?[index] case final Widget foregroundChild)
foregroundChild,
]
we already have a loop in the
CarouselViewwidget, and we may need to define another two loops when passing the parameter.I don't think any additional loops would be necessary :)
children: <Widget>[ widget.children[index], Material( color: Colors.transparent, child: InkWell( onTap: () => widget.onTap?.call(index), overlayColor: effectiveOverlayColor, ), ), if (widget.foregroundChildren?[index] case final Widget foregroundChild) foregroundChild, ]
Sorry, I didn't say clearly. That was I mentioned that the one loop in the CarouselView.
What I mentioned the possible two loops case is when we are using CarouselView. ( When we are rendering the children foreground widgets and children widgets dynamically
const someList = [
{title: 'Title 1', showForeground: true},
{title: 'Title 2', showForeground: false},
]
CarouselView(
children: someList.map(item => Text(item.title)).toList(), // These two loops
foregroundChildren: someList.map(item => showForeground ? Container(child: Text('showForeground')) : null).toList(),
);
Ah, I see—if someone's using loops to construct their widget lists, they'd have to use 2 loops here instead of 1.
I agree that this would be a downside, but I personally don't think it's a huge deal, since there are a variety of ways to make the lists:
const children = [
Text('Title 1'),
Text('Title 2'),
];
final foregroundChildren = List<Widget?>.filled(children.length, null);
foregroundChildren.last = Container(child: Text('showForeground');
return CarouselView(children: children, foregroundChildren: foregroundChildren);
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}