diff --git a/docs/components/accordion.mdx b/docs/components/accordion.mdx index 48312ed91..5002e094d 100644 --- a/docs/components/accordion.mdx +++ b/docs/components/accordion.mdx @@ -33,6 +33,7 @@ Widget-state variants (`onHovered`, `onPressed`, `onFocused`, `onFocusVisible`, `onDisabled`) describe interaction with the **trigger**, but may style any part of the item, including the container. The expanded content is not a separate hover or press target — moving the pointer into it clears the hovered state. +Widgets you place in `child` do not inherit the trigger's widget states. The top-level Box shorthand on `AccordionStyler` (`.color()`, `.padding()`, diff --git a/docs/components/data_table.mdx b/docs/components/data_table.mdx index 6c301f26c..8f81dd6c3 100644 --- a/docs/components/data_table.mdx +++ b/docs/components/data_table.mdx @@ -410,6 +410,9 @@ Required. Builds this column's cell for one row. Exactly one is required. `label` renders with the table's header typography; `header` renders arbitrary content and additionally requires `semanticLabel`. +Content of a header that is not sortable resolves widget-state variants against +an empty state, so a plain Mix widget there does not react to its own hover or +press. Wrap it in its own `Pressable` if it should. #### `semanticLabel` → `String?` @@ -437,7 +440,9 @@ Optional. Whether activating the header emits a new sort descriptor. Sets the row chrome painted behind every cell of the header row, of a body row, and of the final body row. `bodyRow` variants such as `onHovered` and -`onSelected` resolve against that row's own state. +`onSelected` resolve against that row's own state. Header regions resolve +against a sortable header's own state; a header that is not sortable matches +no widget-state variant, even inside a focused or hovered ancestor. #### `headerCell(BoxStyler value)` / `bodyCell(BoxStyler value)` / `selectionCell(BoxStyler value)` diff --git a/docs/components/disclosure.mdx b/docs/components/disclosure.mdx index f772c658d..276fb6752 100644 --- a/docs/components/disclosure.mdx +++ b/docs/components/disclosure.mdx @@ -96,7 +96,8 @@ and `borderRadius` to the trigger. Style the other parts explicitly: - `containerEffects(...)` — shadows, strokes, backdrop blur - `trigger(...)` / `content(...)` - `onExpanded(...)` / `onCollapsed(...)` -- `onHovered`, `onPressed`, `onFocused`, `onFocusVisible`, `onDisabled` +- `onHovered`, `onPressed`, `onFocused`, `onFocusVisible`, `onDisabled`; + these describe the trigger, so widgets in `content` do not inherit them Default transition is a 200ms top-anchored fade and size. Use `AnimationStyle.noAnimation` to skip it. diff --git a/packages/remix/CHANGELOG.md b/packages/remix/CHANGELOG.md index 8103958b9..af31a83a1 100644 --- a/packages/remix/CHANGELOG.md +++ b/packages/remix/CHANGELOG.md @@ -1,3 +1,20 @@ +## Unreleased + +### Fixes + +- Data-table headers without `sortable`, including the selection header, no + longer read widget state from outside the table. A table inside a focused, + hovered, or pressed ancestor, such as a `Pressable`, no longer lights their + `onFocusVisible`, `onHovered`, or `onPressed` styles. Sortable headers still + resolve against their own state. Content of a non-sortable header's custom + `header` resolves widget-state variants against an empty state, so wrap a + widget in its own `Pressable` if it should react to its own hover or press. +- Accordion and disclosure content no longer resolves widget-state variants + against the trigger, so widgets in the panel no longer change with the + trigger's state. +- `RemixSidebar` header and footer content no longer resolves `onDisabled` + variants when the sidebar is disabled. The sidebar's own regions still do. + ## 1.0.0-beta.13 ### Breaking changes diff --git a/packages/remix/lib/src/components/accordion/accordion_widget.dart b/packages/remix/lib/src/components/accordion/accordion_widget.dart index b54ef43f1..978868aad 100644 --- a/packages/remix/lib/src/components/accordion/accordion_widget.dart +++ b/packages/remix/lib/src/components/accordion/accordion_widget.dart @@ -334,7 +334,9 @@ class _RemixAccordionBodyState extends State<_RemixAccordionBody> { // whether or not the caller asked to observe presses. onPressChange: _handlePressChange, semanticLabel: _config.semanticLabel ?? _config.title, - child: _config.child, + // The panel is mounted inside the trigger's state scope, which belongs + // to the trigger alone. + child: WidgetStateProvider(states: const {}, child: _config.child), builder: _config.builder ?? (context, state) => _buildDefaultTrigger(spec, state), diff --git a/packages/remix/lib/src/components/data_table/data_table_spec.dart b/packages/remix/lib/src/components/data_table/data_table_spec.dart index 90ab82497..3019b9f95 100644 --- a/packages/remix/lib/src/components/data_table/data_table_spec.dart +++ b/packages/remix/lib/src/components/data_table/data_table_spec.dart @@ -24,6 +24,10 @@ part of 'data_table.dart'; /// region: `bodyRow(BoxStyler().onSelected(...))` already expresses it, and it /// additionally composes with hover, focus, and press. /// +/// A header that is not sortable has no state of its own. Its regions and any +/// custom [RemixDataTableColumn.header] content resolve those variants against +/// an empty state, whatever state encloses the table. +/// /// ## Composed controls /// /// [selectionCheckbox], [pageButton], and [pageSizeSelect] deliberately hold diff --git a/packages/remix/lib/src/components/data_table/data_table_widget.dart b/packages/remix/lib/src/components/data_table/data_table_widget.dart index 1d5417554..481a34dce 100644 --- a/packages/remix/lib/src/components/data_table/data_table_widget.dart +++ b/packages/remix/lib/src/components/data_table/data_table_widget.dart @@ -1093,6 +1093,10 @@ class _DataTableHeaderCell extends StatelessWidget { onPress: onSort, child: cell, ); + } else { + // A static header has no state of its own. Without this scope its + // regions would resolve against whatever encloses the table. + cell = WidgetStateProvider(states: const {}, child: cell); } final owned = semanticLabel != null; diff --git a/packages/remix/lib/src/components/disclosure/disclosure_widget.dart b/packages/remix/lib/src/components/disclosure/disclosure_widget.dart index c809c9dbd..977043aa0 100644 --- a/packages/remix/lib/src/components/disclosure/disclosure_widget.dart +++ b/packages/remix/lib/src/components/disclosure/disclosure_widget.dart @@ -42,7 +42,9 @@ final class _RemixDisclosureContent extends StatelessWidget { Widget build(BuildContext context) { return Box( styleSpec: _RemixDisclosureStyleScope.of(context).content, - child: child, + // The panel is mounted inside the trigger's state scope, which belongs + // to the trigger alone. + child: WidgetStateProvider(states: const {}, child: child), ); } } diff --git a/packages/remix/lib/src/components/sidebar/sidebar_widget.dart b/packages/remix/lib/src/components/sidebar/sidebar_widget.dart index 7db42873c..0a232b7b7 100644 --- a/packages/remix/lib/src/components/sidebar/sidebar_widget.dart +++ b/packages/remix/lib/src/components/sidebar/sidebar_widget.dart @@ -460,7 +460,16 @@ class RemixSidebar extends StatefulWidget { Semantics( container: true, explicitChildNodes: true, - child: Box(styleSpec: spec.header, child: header), + child: Box( + styleSpec: spec.header, + // The scope above belongs to the sidebar's own + // regions; header and footer content is not disabled + // with the destinations. + child: WidgetStateProvider( + states: const {}, + child: header, + ), + ), ), if (hasBoundedHeight) Expanded(child: SingleChildScrollView(child: landmark)) @@ -470,7 +479,13 @@ class RemixSidebar extends StatefulWidget { Semantics( container: true, explicitChildNodes: true, - child: Box(styleSpec: spec.footer, child: footer), + child: Box( + styleSpec: spec.footer, + child: WidgetStateProvider( + states: const {}, + child: footer, + ), + ), ), ], ); diff --git a/packages/remix/test/components/accordion/accordion_widget_test.dart b/packages/remix/test/components/accordion/accordion_widget_test.dart index d0fb391b1..df6a174c1 100644 --- a/packages/remix/test/components/accordion/accordion_widget_test.dart +++ b/packages/remix/test/components/accordion/accordion_widget_test.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter/services.dart' show LogicalKeyboardKey; import 'package:flutter_test/flutter_test.dart'; import 'package:naked_ui/naked_ui.dart'; import 'package:remix/remix.dart'; @@ -7,8 +8,9 @@ import 'package:remix/src/utilities/remix_path_icon.dart'; import '../../helpers/test_helpers.dart'; import '../../helpers/test_methods.dart'; -/// The single coloured [Box] in the tree is the outer panel: every test that -/// uses this styles `container(...)` and nothing else that paints a fill. +/// The single [Box] with a resolved fill: every test that uses this colours one +/// region, usually `container(...)`. A [Box] built from a `style:` carries no +/// resolved spec, so probes placed in the content are not counted. Color? _panelColor(WidgetTester tester) { return tester .widgetList(find.byType(Box)) @@ -17,6 +19,20 @@ Color? _panelColor(WidgetTester tester) { .single; } +const _probeKey = ValueKey('expanded-content-probe'); + +/// The resolved fill of the [Box] a test places in the expanded content. +Color? _probeColor(WidgetTester tester) { + final container = tester.widget( + find.descendant( + of: find.byKey(_probeKey), + matching: find.byType(Container), + ), + ); + + return (container.decoration as BoxDecoration?)?.color; +} + void main() { group('RemixAccordionGroup', () { group('Basic Rendering', () { @@ -597,6 +613,63 @@ void main() { expect(_panelColor(tester), Colors.blue); }); + + final triggerActivations = Function(WidgetTester)>{ + 'hover': (tester) async { + final gesture = await tester.createGesture(kind: .mouse); + await gesture.addPointer(location: Offset.zero); + addTearDown(gesture.removePointer); + await gesture.moveTo(tester.getCenter(find.text('Test Title'))); + }, + 'press': (tester) async { + final gesture = await tester.startGesture( + tester.getCenter(find.text('Test Title')), + ); + addTearDown(gesture.up); + }, + 'keyboard focus': (tester) => + tester.sendKeyEvent(LogicalKeyboardKey.tab), + }; + + for (final MapEntry(key: name, value: activate) + in triggerActivations.entries) { + testWidgets('trigger $name styles the trigger, not widgets in the ' + 'expanded content', (tester) async { + await tester.pumpRemixApp( + RemixAccordionGroup( + controller: RemixAccordionController(), + initialExpandedValues: const ['item1'], + child: RemixAccordion( + value: 'item1', + title: 'Test Title', + style: AccordionStyler() + .container(.color(Colors.blue)) + .onHovered(.container(.color(Colors.green))) + .onPressed(.container(.color(Colors.green))) + .onFocusVisible(.container(.color(Colors.green))), + child: SizedBox.square( + dimension: 10, + child: Box( + key: _probeKey, + style: BoxStyler() + .color(Colors.grey) + .onHovered(.color(Colors.red)) + .onPressed(.color(Colors.red)) + .onFocusVisible(.color(Colors.red)), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + await activate(tester); + await tester.pumpAndSettle(); + + expect(_panelColor(tester), Colors.green); + expect(_probeColor(tester), Colors.grey); + }); + } }); group('Disabled State', () { @@ -625,6 +698,37 @@ void main() { // Content should still not be visible expect(find.text('Content'), findsNothing); }); + + testWidgets('expanded content takes onDisabled from its own region, not ' + 'from the disabled item', (tester) async { + await tester.pumpRemixApp( + RemixAccordionGroup( + controller: RemixAccordionController(), + initialExpandedValues: const ['item1'], + child: RemixAccordion( + value: 'item1', + title: 'Disabled Item', + enabled: false, + style: AccordionStyler() + .content(.color(Colors.blue)) + .onDisabled(.content(.color(Colors.green))), + child: SizedBox.square( + dimension: 10, + child: Box( + key: _probeKey, + style: BoxStyler() + .color(Colors.grey) + .onDisabled(.color(Colors.red)), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(_panelColor(tester), Colors.green); + expect(_probeColor(tester), Colors.grey); + }); }); group('Custom Builder', () { diff --git a/packages/remix/test/components/data_table/data_table_style_test.dart b/packages/remix/test/components/data_table/data_table_style_test.dart index fb32b9657..1a4638aa1 100644 --- a/packages/remix/test/components/data_table/data_table_style_test.dart +++ b/packages/remix/test/components/data_table/data_table_style_test.dart @@ -1,5 +1,6 @@ import 'package:flutter/gestures.dart' show PointerDeviceKind; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart' show LogicalKeyboardKey; import 'package:flutter_test/flutter_test.dart'; import 'package:remix/remix.dart'; import 'package:remix/src/utilities/remix_path_icon.dart'; @@ -57,11 +58,13 @@ Widget _table({ Set selected = const {}, double minimumWidth = 0, List>? columns, + ValueChanged? onSortChanged, }) { return RemixDataTable<_Record>( rows: rows, columns: columns ?? _columns(), minimumWidth: minimumWidth, + onSortChanged: onSortChanged, rowId: selectable ? (row) => row.id : null, selectedRowIds: selected, onSelectionChanged: selectable ? (_) {} : null, @@ -462,6 +465,124 @@ void main() { }); }); + group('outside widget state', () { + const idle = Color(0xFF111111); + const lit = Color(0xFFEE2222); + const probeKey = ValueKey('outside-state-probe'); + + BoxStyler boxStyle({required bool hover}) { + final base = BoxStyler().color(idle); + final reacting = BoxStyler().color(lit); + + return hover ? base.onHovered(reacting) : base.onFocusVisible(reacting); + } + + DataTableStyler tableStyle({required bool hover}) { + final label = TextStyler().color(idle); + final reacting = TextStyler().color(lit); + + return DataTableStyler() + .headerRow(boxStyle(hover: hover)) + .headerCell(boxStyle(hover: hover)) + .selectionCell(boxStyle(hover: hover)) + .bodyRow(boxStyle(hover: hover)) + .bodyCell(boxStyle(hover: hover)) + .headerLabel( + hover ? label.onHovered(reacting) : label.onFocusVisible(reacting), + ); + } + + Color? probeColor(WidgetTester tester) { + final container = tester.widget( + find.descendant( + of: find.byKey(probeKey), + matching: find.byType(Container), + ), + ); + + return (container.decoration as BoxDecoration?)?.color; + } + + for (final hover in [false, true]) { + final state = hover ? 'hover' : 'keyboard focus'; + + testWidgets('static headers ignore an enclosing $state scope', ( + tester, + ) async { + await tester.pumpRemixApp( + Pressable( + onPress: () {}, + child: SizedBox( + width: 400, + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + SizedBox.square( + dimension: 10, + child: Box( + key: probeKey, + style: boxStyle(hover: hover), + ), + ), + _table(selectable: true, style: tableStyle(hover: hover)), + ], + ), + ), + ), + ); + + if (hover) { + final pointer = TestPointer(1, PointerDeviceKind.mouse); + await tester.sendEventToBinding( + pointer.hover(tester.getCenter(find.text('Name'))), + ); + } else { + await tester.sendKeyEvent(LogicalKeyboardKey.tab); + } + await tester.pump(); + + expect(probeColor(tester), lit); + final colors = _boxColors(tester); + expect(colors, contains(idle)); + expect(colors, isNot(contains(lit))); + expect(tester.widget(find.text('Name')).style?.color, idle); + expect(tester.widget(find.text('Id')).style?.color, idle); + }); + } + + testWidgets('a sortable header resolves its own focus', (tester) async { + await tester.pumpRemixApp( + SizedBox( + width: 400, + child: _table( + columns: [ + RemixDataTableColumn<_Record>( + id: 'name', + label: 'Name', + sortable: true, + cellBuilder: (context, row) => Text(row.name), + ), + RemixDataTableColumn<_Record>( + id: 'id', + label: 'Id', + cellBuilder: (context, row) => Text(row.id), + ), + ], + onSortChanged: (_) {}, + style: tableStyle(hover: false), + ), + ), + ); + + await tester.sendKeyEvent(LogicalKeyboardKey.tab); + await tester.pump(); + + expect(_boxColors(tester).where((c) => c == lit), hasLength(2)); + expect(tester.widget(find.text('Name')).style?.color, lit); + expect(tester.widget(find.text('Id')).style?.color, idle); + }); + }); + group('metrics', () { testWidgets('rowMinHeight and headerMinHeight set the row floors', ( tester, diff --git a/packages/remix/test/components/disclosure/disclosure_widget_test.dart b/packages/remix/test/components/disclosure/disclosure_widget_test.dart index 90df72c81..193d81737 100644 --- a/packages/remix/test/components/disclosure/disclosure_widget_test.dart +++ b/packages/remix/test/components/disclosure/disclosure_widget_test.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter/services.dart' show LogicalKeyboardKey; import 'package:flutter_test/flutter_test.dart'; import 'package:remix/remix.dart'; @@ -259,6 +260,102 @@ void main() { expect(disclosure.style, same(style)); }); + + testWidgets('content takes onDisabled from its own region, not from the ' + 'disabled trigger', (tester) async { + const probeKey = ValueKey('content-probe'); + await tester.pumpRemixApp( + RemixDisclosure( + enabled: false, + defaultExpanded: true, + trigger: const Text('Trigger'), + content: SizedBox.square( + dimension: 10, + child: Box( + key: probeKey, + style: BoxStyler() + .color(Colors.grey) + .onDisabled(.color(Colors.red)), + ), + ), + style: DisclosureStyler() + .content(.color(Colors.blue)) + .onDisabled(.content(.color(Colors.green))), + ), + ); + await tester.pumpAndSettle(); + + expect(_boxColors(tester, find.byKey(probeKey)), contains(Colors.green)); + final probe = tester.widget( + find.descendant( + of: find.byKey(probeKey), + matching: find.byType(Container), + ), + ); + expect((probe.decoration as BoxDecoration?)?.color, Colors.grey); + }); + + final triggerActivations = Function(WidgetTester)>{ + 'hover': (tester) async { + final gesture = await tester.createGesture(kind: .mouse); + await gesture.addPointer(location: Offset.zero); + addTearDown(gesture.removePointer); + await gesture.moveTo(tester.getCenter(find.text('Trigger'))); + }, + 'press': (tester) async { + final gesture = await tester.startGesture( + tester.getCenter(find.text('Trigger')), + ); + addTearDown(gesture.up); + }, + 'keyboard focus': (tester) => tester.sendKeyEvent(LogicalKeyboardKey.tab), + }; + + for (final MapEntry(key: name, value: activate) + in triggerActivations.entries) { + testWidgets('trigger $name styles the trigger, not widgets in the ' + 'content', (tester) async { + const probeKey = ValueKey('content-probe'); + await tester.pumpRemixApp( + RemixDisclosure( + defaultExpanded: true, + trigger: const Text('Trigger'), + content: SizedBox.square( + dimension: 10, + child: Box( + key: probeKey, + style: BoxStyler() + .color(Colors.grey) + .onHovered(.color(Colors.red)) + .onPressed(.color(Colors.red)) + .onFocusVisible(.color(Colors.red)), + ), + ), + style: DisclosureStyler() + .trigger(.color(Colors.blue)) + .onHovered(.trigger(.color(Colors.green))) + .onPressed(.trigger(.color(Colors.green))) + .onFocusVisible(.trigger(.color(Colors.green))), + ), + ); + await tester.pumpAndSettle(); + + await activate(tester); + await tester.pumpAndSettle(); + + expect( + _boxColors(tester, find.text('Trigger')), + contains(Colors.green), + ); + final probe = tester.widget( + find.descendant( + of: find.byKey(probeKey), + matching: find.byType(Container), + ), + ); + expect((probe.decoration as BoxDecoration?)?.color, Colors.grey); + }); + } }); } diff --git a/packages/remix/test/components/sidebar/sidebar_style_test.dart b/packages/remix/test/components/sidebar/sidebar_style_test.dart index 61f5e8a38..bfae464d7 100644 --- a/packages/remix/test/components/sidebar/sidebar_style_test.dart +++ b/packages/remix/test/components/sidebar/sidebar_style_test.dart @@ -258,5 +258,61 @@ void main() { }, ); } + + for (final disabledBy in ['enabled false', 'null callback']) { + testWidgets('$disabledBy disables the header region, not the header ' + 'content', (tester) async { + const headerProbe = ValueKey('header-probe'); + const footerProbe = ValueKey('footer-probe'); + Widget probe(Key key) => SizedBox.square( + dimension: 10, + child: Box( + key: key, + style: BoxStyler() + .color(Colors.grey) + .onDisabled(.color(Colors.red)), + ), + ); + + await tester.pumpRemixApp( + SizedBox( + height: 400, + child: RemixSidebar( + header: probe(headerProbe), + footer: probe(footerProbe), + sections: _sections, + selectedValue: 'overview', + enabled: disabledBy != 'enabled false', + onSelected: disabledBy == 'null callback' ? null : (_) {}, + style: SidebarStyler().onDisabled( + SidebarStyler() + .header(.color(Colors.green)) + .footer(.color(Colors.green)), + ), + ), + ), + ); + + for (final key in [headerProbe, footerProbe]) { + final regions = tester.widgetList( + find.ancestor(of: find.byKey(key), matching: find.byType(Box)), + ); + expect( + regions.map( + (box) => + (box.styleSpec?.spec.decoration as BoxDecoration?)?.color, + ), + contains(Colors.green), + ); + final content = tester.widget( + find.descendant( + of: find.byKey(key), + matching: find.byType(Container), + ), + ); + expect((content.decoration as BoxDecoration?)?.color, Colors.grey); + } + }); + } }); }