CarouselView children are not clickable #154701
Comments
|
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) {
//
}, |
@erickjtorres Do you expect CarouselView to have other GestureDetector actions like onDoubleTap, onLongPress, etc? |
huycozy
added
the
waiting for customer response
|
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),
),
],
),
],
),
);
}
}
|
github-actions
bot
removed
the
waiting for customer response
|
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
),
],
)
|
|
LGTM, also we can lift up this null checker. |
|
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. |
huycozy
added
c: new feature
Piinks
added
triaged-design
|
In this duplicate issue #155199, I propose that we refrain from directly using the
Therefore, I suggest introducing a new parameter called /// 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 Here are a few options to consider: Match the button widgetsThere have been 4 pull requests opened over the past couple of months adding the option to remove the
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 Use a boolean flag, and change every button to matchI think the discussion in dart-lang/language#2232 has several good points, notably:
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 More childrenInstead of removing the 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. |
I agree with you that the name is very bad in this case.
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 When I was typing this comment, I have a new idea. What if we replace both /// Somewhere in CarouselView
Widget builder(int index, Widget foreground); |
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
I'd be in favor of this, though instead of replacing |
I was worried about the cased that we already have a loop in the
Thank you for this suggestion. I learned a lot. |
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 What I mentioned the possible two loops case is when we are using 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 |
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
Screenshots or Video
No response
Logs
No response
Flutter Doctor output
Doctor output
The text was updated successfully, but these errors were encountered: