Add SliverClipRect and SliverClipRRect#179003
Conversation
ee0ecb6 to
bb892ee
Compare
a166658 to
e97348b
Compare
e97348b to
c9db9e0
Compare
|
The push (c9db9e0) is for migration from @manu-sncf to @zemanux |
c9db9e0 to
5e8ef2b
Compare
f259a25 to
a2ed28f
Compare
|
Last commit for rebase and synchronization of
I added a new example demonstrating the key capabilities of this new widget: sliver_clip_rect.mp4Code sampleimport 'package:flutter/material.dart';
void main() {
runApp(const MaterialApp(home: MyApp()));
}
class MyApp extends StatelessWidget {
const MyApp({super.key});
@override
Widget build(BuildContext context) => Scaffold(
body: DecoratedBox(
decoration: BoxDecoration(
gradient: LinearGradient(
begin: Alignment.topCenter,
end: Alignment.bottomCenter,
colors: <Color>[Colors.blue.shade200, Colors.blue.shade800],
),
),
child: Padding(
padding: const EdgeInsets.symmetric(horizontal: 32),
child: CustomScrollView(
physics: const BouncingScrollPhysics(),
reverse: false,
slivers: <Widget>[
SliverClipRRect(
borderRadius: BorderRadius.circular(12),
sliver: const SliverAppBar(
pinned: true,
stretch: true,
title: Text('Group Clipping'),
backgroundColor: Colors.transparent,
),
),
SliverClipRRect(
clipOverlap: false,
borderRadius: BorderRadius.circular(12),
sliver: const PinnedHeaderSliver(
child: Material(
color: Colors.amber,
child: ListTile(
title: Text('This is a PinnedHeaderSliver', style: TextStyle(color: Colors.white)),
),
),
),
),
...List.generate(
2,
(_) => SliverPadding(
padding: const EdgeInsets.all(16),
sliver: SliverClipRRect(
// clipOverlap: false,
borderRadius: BorderRadius.circular(12),
sliver: DecoratedSliver(
decoration: const BoxDecoration(color: Colors.purple),
sliver: SliverMainAxisGroup(
slivers: <Widget>[
const PinnedHeaderSliver(
child: ListTile(
title: Text('Pinned Header', style: TextStyle(color: Colors.white)),
),
),
SliverClipRect(
sliver: SliverList.builder(
itemCount: 20,
itemBuilder: (BuildContext context, int index) => Material(
type: MaterialType.transparency,
child: ListTile(
title: Text('Item $index', style: const TextStyle(color: Colors.white)),
onTap: () {},
),
),
),
),
],
),
),
),
),
),
],
),
),
),
);
}
// class _DecoratedSliver extends StatelessWidget {
// const _DecoratedSliver({this.isEnabled = true, required this.sliver});
// final bool isEnabled;
// final Widget sliver;
// @override
// Widget build(BuildContext context) => isEnabled
// ? DecoratedSliver(
// decoration: BoxDecoration(
// border: Border.all(),
// borderRadius: BorderRadius.circular(8),
// gradient: const LinearGradient(
// begin: Alignment.topCenter,
// end: Alignment.bottomCenter,
// colors: [Colors.blue, Colors.purple],
// ),
// ),
// sliver: sliver,
// )
// : sliver;
// } |
a2ed28f to
0683f26
Compare
24855d6 to
38ce420
Compare
38ce420 to
aa21e2c
Compare
Renzo-Olivares
left a comment
There was a problem hiding this comment.
@zemanux thanks again for the quick changes, these should be the last of it.
| /// Whether to clip starting from the overlap area. | ||
| ClipOverlapBehavior get clipOverlap => _clipOverlap; | ||
| ClipOverlapBehavior _clipOverlap; | ||
| set clipOverlap(ClipOverlapBehavior value) { |
There was a problem hiding this comment.
[Bug] The setter for clipOverlap in _RenderSliverCustomClip does not invalidate the cached clip (_clip = null), leading to a state-machine caching bug. When clipOverlap is dynamically updated, the render object continues using the cached clip from the previous overlap behavior.
To resolve this, update the setter to call _markNeedsClip() instead of markNeedsPaint():
set clipOverlap(ClipOverlapBehavior value) {
if (_clipOverlap == value) {
return;
}
_clipOverlap = value;
_markNeedsClip();
}Runnable Reproduction Application:
import 'package:flutter/material.dart';
void main() {
runApp(const MaterialApp(home: SliverClipOverlapRepro()));
}
class SliverClipOverlapRepro extends StatefulWidget {
const SliverClipOverlapRepro({super.key});
@override
State<SliverClipOverlapRepro> createState() => _SliverClipOverlapReproState();
}
class _SliverClipOverlapReproState extends State<SliverClipOverlapRepro> {
bool _clipOverlap = true;
@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(
title: const Text('SliverClipRect Repro'),
),
body: CustomScrollView(
slivers: <Widget>[
// Pinned header with semi-transparent background to visualize overlap
SliverPersistentHeader(
pinned: true,
delegate: _PinnedHeaderDelegate(),
),
// Spacer
const SliverToBoxAdapter(
child: SizedBox(height: 50.0),
),
// SliverClipRect whose clipOverlap is toggled
SliverClipRect(
clipOverlap: _clipOverlap,
sliver: SliverToBoxAdapter(
child: Container(
height: 150.0,
color: Colors.blue.withOpacity(0.5),
child: const Center(
child: Text(
'Content inside SliverClipRect',
style: TextStyle(color: Colors.white, fontSize: 20.0),
),
),
),
),
),
const SliverToBoxAdapter(
child: SizedBox(height: 1000.0),
),
],
),
floatingActionButton: FloatingActionButton.extended(
onPressed: () {
setState(() {
_clipOverlap = !_clipOverlap;
});
print('Toggled clipOverlap to: $_clipOverlap');
},
label: Text('Toggle clipOverlap: $_clipOverlap'),
),
);
}
}
class _PinnedHeaderDelegate extends SliverPersistentHeaderDelegate {
@override
Widget build(BuildContext context, double shrinkOffset, bool overlapsContent) {
return Container(
color: Colors.red.withOpacity(0.5),
alignment: Alignment.center,
child: const Text(
'Pinned Header (Overlap Creator)',
style: TextStyle(color: Colors.white, fontSize: 18.0),
),
);
}
@override
double get maxExtent => 100.0;
@override
double get minExtent => 100.0;
@override
bool shouldRebuild(covariant SliverPersistentHeaderDelegate oldDelegate) => false;
}| AxisDirection.down => copyNewClipWith(top: clipOrigin), | ||
| AxisDirection.up => copyNewClipWith(bottom: geometry!.paintExtent - clipOrigin), | ||
| AxisDirection.right => copyNewClipWith(left: clipOrigin), | ||
| AxisDirection.left => copyNewClipWith(right: geometry!.paintExtent - clipOrigin), |
There was a problem hiding this comment.
[Bug] When a custom clipper is provided, the overlap adjustment logic in buildClip will incorrectly expand the clip boundary (unclipping content that should be clipped) if the overlap is less than the custom clip's initial inset. For example, in AxisDirection.down, if the custom clipper sets a top inset of 100.0 and the overlap is 50.0, the code sets the clip top to clipOrigin (50.0), which unclips the region between 50.0 and 100.0.
The same issue is present in RenderSliverClipRRect.buildClip (around line 498).
To fix this, ensure the new clip boundaries are bounded by the custom clipper's initial offsets:
AxisDirection.down:math.max(newClip.top, clipOrigin)AxisDirection.up:math.min(newClip.bottom, geometry!.paintExtent - clipOrigin)AxisDirection.right:math.max(newClip.left, clipOrigin)AxisDirection.left:math.min(newClip.right, geometry!.paintExtent - clipOrigin)
Runnable Reproduction Application:
import 'package:flutter/material.dart';
void main() {
runApp(const MaterialApp(home: SliverCustomClipperRepro()));
}
class SliverCustomClipperRepro extends StatefulWidget {
const SliverCustomClipperRepro({super.key});
@override
State<SliverCustomClipperRepro> createState() => _SliverCustomClipperReproState();
}
class _SliverCustomClipperReproState extends State<SliverCustomClipperRepro> {
@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(
title: const Text('SliverCustomClipper Overlap Repro'),
),
body: CustomScrollView(
slivers: <Widget>[
// Pinned header of 50px
const SliverPersistentHeader(
pinned: true,
delegate: _HeaderDelegate(50.0),
),
// Spacer of 50px
const SliverToBoxAdapter(
child: SizedBox(height: 50.0),
),
// SliverClipRect with custom clipper that clips top 100px
SliverClipRect(
clipper: const _CustomClipper100(),
sliver: SliverToBoxAdapter(
child: Container(
height: 200.0,
color: Colors.blue.withOpacity(0.5),
child: const Center(
child: Text(
'Content (Top 100px should be clipped)',
style: TextStyle(color: Colors.white, fontSize: 18.0),
),
),
),
),
),
const SliverToBoxAdapter(
child: SizedBox(height: 1000.0),
),
],
),
);
}
}
class _CustomClipper100 extends CustomClipper<Rect> {
const _CustomClipper100();
@override
Rect getClip(Size size) => Rect.fromLTRB(0.0, 100.0, size.width, size.height);
@override
bool shouldReclip(covariant CustomClipper<Rect> oldClipper) => false;
}
class _HeaderDelegate extends SliverPersistentHeaderDelegate {
const _HeaderDelegate(this.height);
final double height;
@override
Widget build(BuildContext context, double shrinkOffset, bool overlapsContent) {
return Container(
color: Colors.red.withOpacity(0.5),
alignment: Alignment.center,
child: Text(
'Header (Height: $height)',
style: const TextStyle(color: Colors.white, fontSize: 18.0),
),
);
}
@override
double get maxExtent => height;
@override
double get minExtent => height;
@override
bool shouldRebuild(covariant _HeaderDelegate oldDelegate) => oldDelegate.height != height;
}|
Heads up there are some linux analyzer failures + test leak failures that will need to be resolved before landing this. |
|
|
||
| /// The entire shape of the clip shifts inwards to preserve its form. | ||
| /// | ||
| /// Instead of simply slicing off the overlapped portion, the clip |
There was a problem hiding this comment.
Linux analyzer is complaining about the word "simply" https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8679348322693150353/+/u/run_test.dart_for_analyze_shard_and_subshard_None/stdout .
| testWidgets('updates clip when overlap changes even if geometry is same', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| final controller = ScrollController(); |
There was a problem hiding this comment.
Should dispose controller, with addTearDown. Here and elsewhere in this file.
| testWidgets('updates clip when overlap changes even if geometry is same', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| final controller = ScrollController(); |
There was a problem hiding this comment.
Should dispose controller with addTearDown to prevent memory leaks. Here and elsewhere in this file.
Renzo-Olivares
left a comment
There was a problem hiding this comment.
LGTM w/ small nits.
| bottom ?? newClip.bottom, | ||
| ); | ||
|
|
||
| if (clipOverlap != ClipOverlapBehavior.none && constraints.overlap > 0) { |
There was a problem hiding this comment.
Please use a double literal for this comparison since constraints.overlap is a double (i.e. 0.0 instead of 0).
This also applies to the overlap check in RenderSliverClipRRect.buildClip.
See Flutter Style Guide: Use double literals for double constants.
| testWidgets('updates properties', (WidgetTester tester) async { | ||
| await tester.pumpWidget( | ||
| const Directionality( | ||
| textDirection: TextDirection.ltr, |
There was a problem hiding this comment.
Please use dot shorthands to omit the obvious type for this named argument (e.g. textDirection: .ltr instead of textDirection: TextDirection.ltr).
This also applies to other named argument values inside this test file, such as textDirection, clipBehavior, and scrollDirection calls where the expected type is explicit.
See Flutter Style Guide: Prefer using dot shorthands to omit obvious types for named argument values.
Fix #179002.
Pre-launch Checklist
///).