diff --git a/packages/two_dimensional_scrollables/CHANGELOG.md b/packages/two_dimensional_scrollables/CHANGELOG.md index a201be420b1f..c9332dcb13ac 100644 --- a/packages/two_dimensional_scrollables/CHANGELOG.md +++ b/packages/two_dimensional_scrollables/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.3.9 + +* Fixes TableSpan borders being flipped when one or both axis directions are reversed. + ## 0.3.8 * Updates minimum supported SDK version to Flutter 3.35/Dart 3.9. diff --git a/packages/two_dimensional_scrollables/lib/src/common/span.dart b/packages/two_dimensional_scrollables/lib/src/common/span.dart index 27e40b4ae7e4..84d6f64da9ed 100644 --- a/packages/two_dimensional_scrollables/lib/src/common/span.dart +++ b/packages/two_dimensional_scrollables/lib/src/common/span.dart @@ -439,17 +439,24 @@ class SpanBorder { /// cells. void paint(SpanDecorationPaintDetails details, BorderRadius? borderRadius) { final AxisDirection axisDirection = details.axisDirection; + final AxisDirection? crossAxisDirection = details.crossAxisDirection; switch (axisDirectionToAxis(axisDirection)) { case Axis.horizontal: + final bool isLeadingTop = + crossAxisDirection == null || + crossAxisDirection == AxisDirection.down; final border = Border( - top: axisDirection == AxisDirection.right ? leading : trailing, - bottom: axisDirection == AxisDirection.right ? trailing : leading, + top: isLeadingTop ? leading : trailing, + bottom: isLeadingTop ? trailing : leading, ); border.paint(details.canvas, details.rect, borderRadius: borderRadius); case Axis.vertical: + final bool isLeadingLeft = + crossAxisDirection == null || + crossAxisDirection == AxisDirection.right; final border = Border( - left: axisDirection == AxisDirection.down ? leading : trailing, - right: axisDirection == AxisDirection.down ? trailing : leading, + left: isLeadingLeft ? leading : trailing, + right: isLeadingLeft ? trailing : leading, ); border.paint(details.canvas, details.rect, borderRadius: borderRadius); } @@ -468,6 +475,7 @@ class SpanDecorationPaintDetails { required this.canvas, required this.rect, required this.axisDirection, + this.crossAxisDirection, }); /// The [Canvas] that the [SpanDecoration] will be painted to. @@ -487,4 +495,10 @@ class SpanDecorationPaintDetails { /// [AxisDirection.right], which would be [Axis.horizontal], a row is being /// painted. final AxisDirection axisDirection; + + /// The [AxisDirection] of the [Axis] perpendicular to the [Span]. + /// + /// Used to determine the correct leading/trailing edge when deciding how to + /// paint borders or apply padding. + final AxisDirection? crossAxisDirection; } diff --git a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart index 3f26dd00dfd3..e0dd64e9e0b0 100644 --- a/packages/two_dimensional_scrollables/lib/src/table_view/table.dart +++ b/packages/two_dimensional_scrollables/lib/src/table_view/table.dart @@ -1336,7 +1336,6 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { final foregroundColumns = {}; final backgroundColumns = {}; - final TableSpan rowSpan = _rowMetrics[leadingVicinity.row]!.configuration; for ( int column = leadingVicinity.column; column <= trailingVicinity.column; @@ -1415,27 +1414,45 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { required RenderBox trailingCell, required bool consumePadding, }) { - final ({double leading, double trailing}) offsetCorrection = - axisDirectionIsReversed(verticalAxisDirection) - ? ( - leading: leadingCell.size.height, - trailing: trailingCell.size.height, - ) - : (leading: 0.0, trailing: 0.0); - return Rect.fromPoints( - parentDataOf(leadingCell).paintOffset! + - offset - - Offset( - consumePadding ? columnSpan.padding.leading : 0.0, - rowSpan.padding.leading - offsetCorrection.leading, - ), - parentDataOf(trailingCell).paintOffset! + - offset + - Offset(trailingCell.size.width, trailingCell.size.height) + - Offset( - consumePadding ? columnSpan.padding.trailing : 0.0, - rowSpan.padding.trailing - offsetCorrection.trailing, - ), + final bool reversedH = axisDirectionIsReversed( + horizontalAxisDirection, + ); + final bool reversedV = axisDirectionIsReversed(verticalAxisDirection); + final TableSpan leadingRowSpan = + _rowMetrics[parentDataOf(leadingCell).tableVicinity.row]! + .configuration; + final TableSpan trailingRowSpan = + _rowMetrics[parentDataOf(trailingCell).tableVicinity.row]! + .configuration; + + final double leftExpansion = consumePadding + ? (reversedH + ? columnSpan.padding.trailing + : columnSpan.padding.leading) + : 0.0; + final double rightExpansion = consumePadding + ? (reversedH + ? columnSpan.padding.leading + : columnSpan.padding.trailing) + : 0.0; + final double topExpansion = reversedV + ? trailingRowSpan.padding.trailing + : leadingRowSpan.padding.leading; + final double bottomExpansion = reversedV + ? leadingRowSpan.padding.leading + : trailingRowSpan.padding.trailing; + + final Offset p1 = parentDataOf(leadingCell).paintOffset! + offset; + final Offset p2 = + parentDataOf(trailingCell).paintOffset! + + offset + + Offset(trailingCell.size.width, trailingCell.size.height); + + return Rect.fromLTRB( + math.min(p1.dx, p2.dx - trailingCell.size.width) - leftExpansion, + math.min(p1.dy, p2.dy - trailingCell.size.height) - topExpansion, + math.max(p1.dx + leadingCell.size.width, p2.dx) + rightExpansion, + math.max(p1.dy + leadingCell.size.height, p2.dy) + bottomExpansion, ); } @@ -1471,8 +1488,6 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { // Row decorations final foregroundRows = {}; final backgroundRows = {}; - final TableSpan columnSpan = - _columnMetrics[leadingVicinity.column]!.configuration; for (int row = leadingVicinity.row; row <= trailingVicinity.row; row++) { TableSpan rowSpan = _rowMetrics[row]!.configuration; if (rowSpan.backgroundDecoration != null || @@ -1547,27 +1562,41 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { required RenderBox trailingCell, required bool consumePadding, }) { - final ({double leading, double trailing}) offsetCorrection = - axisDirectionIsReversed(horizontalAxisDirection) - ? ( - leading: leadingCell.size.width, - trailing: trailingCell.size.width, - ) - : (leading: 0.0, trailing: 0.0); - return Rect.fromPoints( - parentDataOf(leadingCell).paintOffset! + - offset - - Offset( - columnSpan.padding.leading - offsetCorrection.leading, - consumePadding ? rowSpan.padding.leading : 0.0, - ), - parentDataOf(trailingCell).paintOffset! + - offset + - Offset(trailingCell.size.width, trailingCell.size.height) + - Offset( - columnSpan.padding.leading - offsetCorrection.trailing, - consumePadding ? rowSpan.padding.trailing : 0.0, - ), + final bool reversedH = axisDirectionIsReversed( + horizontalAxisDirection, + ); + final bool reversedV = axisDirectionIsReversed(verticalAxisDirection); + final TableSpan leadingColSpan = + _columnMetrics[parentDataOf(leadingCell).tableVicinity.column]! + .configuration; + final TableSpan trailingColSpan = + _columnMetrics[parentDataOf(trailingCell).tableVicinity.column]! + .configuration; + + final double leftExpansion = reversedH + ? trailingColSpan.padding.trailing + : leadingColSpan.padding.leading; + final double rightExpansion = reversedH + ? leadingColSpan.padding.leading + : trailingColSpan.padding.trailing; + final double topExpansion = consumePadding + ? (reversedV ? rowSpan.padding.trailing : rowSpan.padding.leading) + : 0.0; + final double bottomExpansion = consumePadding + ? (reversedV ? rowSpan.padding.leading : rowSpan.padding.trailing) + : 0.0; + + final Offset p1 = parentDataOf(leadingCell).paintOffset! + offset; + final Offset p2 = + parentDataOf(trailingCell).paintOffset! + + offset + + Offset(trailingCell.size.width, trailingCell.size.height); + + return Rect.fromLTRB( + math.min(p1.dx, p2.dx - trailingCell.size.width) - leftExpansion, + math.min(p1.dy, p2.dy - trailingCell.size.height) - topExpansion, + math.max(p1.dx + leadingCell.size.width, p2.dx) + rightExpansion, + math.max(p1.dy + leadingCell.size.height, p2.dy) + bottomExpansion, ); } @@ -1612,6 +1641,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: horizontalAxisDirection, + crossAxisDirection: verticalAxisDirection, ); decoration.paint(paintingDetails); }); @@ -1620,6 +1650,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: verticalAxisDirection, + crossAxisDirection: horizontalAxisDirection, ); decoration.paint(paintingDetails); }); @@ -1630,6 +1661,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: verticalAxisDirection, + crossAxisDirection: horizontalAxisDirection, ); decoration.paint(paintingDetails); }); @@ -1638,6 +1670,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: horizontalAxisDirection, + crossAxisDirection: verticalAxisDirection, ); decoration.paint(paintingDetails); }); @@ -1682,6 +1715,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: horizontalAxisDirection, + crossAxisDirection: verticalAxisDirection, ); decoration.paint(paintingDetails); }); @@ -1690,6 +1724,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: verticalAxisDirection, + crossAxisDirection: horizontalAxisDirection, ); decoration.paint(paintingDetails); }); @@ -1700,6 +1735,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: verticalAxisDirection, + crossAxisDirection: horizontalAxisDirection, ); decoration.paint(paintingDetails); }); @@ -1708,6 +1744,7 @@ class RenderTableViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: horizontalAxisDirection, + crossAxisDirection: verticalAxisDirection, ); decoration.paint(paintingDetails); }); diff --git a/packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart b/packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart index 1c077c63f2a3..f10c147a50db 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart @@ -545,12 +545,14 @@ class RenderTreeViewport extends RenderTwoDimensionalViewport { ); // Decoration rects cover the whole row from the left and right // edge of the viewport. - return Rect.fromPoints( - Offset(0.0, parentData.layoutOffset!.dy), - Offset( - viewportDimension.width, - rowSpan.trailingOffset - verticalOffset.pixels, - ), + return Rect.fromLTRB( + 0.0, + parentData.paintOffset!.dy - + (consumePadding ? rowSpan.configuration.padding.leading : 0.0), + viewportDimension.width, + parentData.paintOffset!.dy + + child.size.height + + (consumePadding ? rowSpan.configuration.padding.trailing : 0.0), ); } @@ -577,6 +579,7 @@ class RenderTreeViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: horizontalAxisDirection, + crossAxisDirection: verticalAxisDirection, ); decoration.paint(paintingDetails); }); @@ -598,6 +601,7 @@ class RenderTreeViewport extends RenderTwoDimensionalViewport { canvas: context.canvas, rect: rect, axisDirection: horizontalAxisDirection, + crossAxisDirection: verticalAxisDirection, ); decoration.paint(paintingDetails); }); diff --git a/packages/two_dimensional_scrollables/lib/src/tree_view/tree_span.dart b/packages/two_dimensional_scrollables/lib/src/tree_view/tree_span.dart index 3c6f8dd65fec..d64f989f07fc 100644 --- a/packages/two_dimensional_scrollables/lib/src/tree_view/tree_span.dart +++ b/packages/two_dimensional_scrollables/lib/src/tree_view/tree_span.dart @@ -103,7 +103,15 @@ class TreeRowBorder extends SpanBorder { @override void paint(SpanDecorationPaintDetails details, BorderRadius? borderRadius) { - final border = Border(top: top, bottom: bottom, left: left, right: right); + final AxisDirection? crossAxisDirection = details.crossAxisDirection; + final bool isLeadingTop = + crossAxisDirection == null || crossAxisDirection == AxisDirection.down; + final border = Border( + top: isLeadingTop ? top : bottom, + bottom: isLeadingTop ? bottom : top, + left: left, + right: right, + ); border.paint(details.canvas, details.rect, borderRadius: borderRadius); } } diff --git a/packages/two_dimensional_scrollables/pubspec.yaml b/packages/two_dimensional_scrollables/pubspec.yaml index c17706a2af68..9d16a900d995 100644 --- a/packages/two_dimensional_scrollables/pubspec.yaml +++ b/packages/two_dimensional_scrollables/pubspec.yaml @@ -1,6 +1,6 @@ name: two_dimensional_scrollables description: Widgets that scroll using the two dimensional scrolling foundation. -version: 0.3.8 +version: 0.3.9 repository: https://github.com/flutter/packages/tree/main/packages/two_dimensional_scrollables issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+two_dimensional_scrollables%22+ diff --git a/packages/two_dimensional_scrollables/test/table_view/table_span_test.dart b/packages/two_dimensional_scrollables/test/table_view/table_span_test.dart index 4e546e8a9957..ad3a9b8dca44 100644 --- a/packages/two_dimensional_scrollables/test/table_view/table_span_test.dart +++ b/packages/two_dimensional_scrollables/test/table_view/table_span_test.dart @@ -3,7 +3,6 @@ // found in the LICENSE file. import 'package:flutter/material.dart'; -import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:two_dimensional_scrollables/two_dimensional_scrollables.dart'; @@ -850,6 +849,156 @@ void main() { ), ); }); + + testWidgets( + 'paints borders correctly when cross axis is reversed (TableView)', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/177117 + final tableView = TableView.builder( + horizontalDetails: const ScrollableDetails.horizontal(reverse: true), + rowCount: 1, + columnCount: 1, + columnBuilder: (int index) => const TableSpan( + extent: FixedTableSpanExtent(200.0), + foregroundDecoration: TableSpanDecoration( + border: TableSpanBorder( + leading: BorderSide(color: Colors.orange, width: 3), + ), + ), + ), + rowBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(200.0)), + cellBuilder: (_, TableVicinity vicinity) { + return TableViewCell( + child: Container( + height: 200, + width: 200, + color: Colors.grey.withValues(alpha: 0.5), + ), + ); + }, + ); + + tester.view.physicalSize = const Size(400, 400); + tester.view.devicePixelRatio = 1.0; + addTearDown(() { + tester.view.resetPhysicalSize(); + tester.view.resetDevicePixelRatio(); + }); + + await tester.pumpWidget(MaterialApp(home: Scaffold(body: tableView))); + await tester.pumpAndSettle(); + + expect( + find.byType(TableViewport), + paints..path( + includes: [ + const Offset(400.0, 0.0), + const Offset(400.0, 200.0), + ], + color: const Color(0xffff9800), + ), + ); + }, + ); + + testWidgets( + 'paints borders correctly when vertical scrolling is reversed (TableView)', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/177117 + final tableView = TableView.builder( + verticalDetails: const ScrollableDetails.vertical(reverse: true), + rowCount: 1, + columnCount: 1, + columnBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(200.0)), + rowBuilder: (int index) => const TableSpan( + extent: FixedTableSpanExtent(200.0), + foregroundDecoration: TableSpanDecoration( + border: TableSpanBorder( + leading: BorderSide(color: Colors.orange, width: 3), + ), + ), + ), + cellBuilder: (_, TableVicinity vicinity) { + return TableViewCell( + child: Container( + height: 200, + width: 200, + color: Colors.grey.withValues(alpha: 0.5), + ), + ); + }, + ); + + tester.view.physicalSize = const Size(400, 400); + tester.view.devicePixelRatio = 1.0; + addTearDown(() { + tester.view.resetPhysicalSize(); + tester.view.resetDevicePixelRatio(); + }); + + await tester.pumpWidget(MaterialApp(home: Scaffold(body: tableView))); + await tester.pumpAndSettle(); + + expect( + find.byType(TableViewport), + paints..path( + includes: [ + const Offset(0.0, 400.0), + const Offset(200.0, 400.0), + ], + color: const Color(0xffff9800), + ), + ); + }, + ); + + testWidgets( + 'TableView row decoration rect is correct when vertical axis is reversed and padding is used', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/issues/177117 + final tableView = TableView.builder( + verticalDetails: const ScrollableDetails.vertical(reverse: true), + rowCount: 1, + columnCount: 1, + columnBuilder: (int index) => + const TableSpan(extent: FixedTableSpanExtent(200.0)), + rowBuilder: (int index) => const TableSpan( + extent: FixedTableSpanExtent(200.0), + padding: TableSpanPadding(leading: 10.0, trailing: 20.0), + backgroundDecoration: TableSpanDecoration(color: Colors.red), + ), + cellBuilder: (_, TableVicinity vicinity) { + return TableViewCell( + child: Container(width: 200, height: 200, color: Colors.blue), + ); + }, + ); + + tester.view.physicalSize = const Size(400, 400); + tester.view.devicePixelRatio = 1.0; + addTearDown(() { + tester.view.resetPhysicalSize(); + tester.view.resetDevicePixelRatio(); + }); + + await tester.pumpWidget(MaterialApp(home: Scaffold(body: tableView))); + await tester.pumpAndSettle(); + + // Since vertical is reversed, row 0 is at the bottom (y=400). + // Leading padding covers from y=390 to y=400. + // Trailing padding covers from y=170 to y=190. + // Content covers from y=190 to y=390. + expect( + find.byType(TableViewport), + paints..rect( + rect: const Rect.fromLTRB(0.0, 170.0, 200.0, 400.0), + color: Colors.red, + ), + ); + }, + ); }); group('merged cell decorations', () {