diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index dd0d731..ea6ecb3 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,5 +1,5 @@ { "name": "layrz_theme", "description": "Claude Code skills for layrz_theme Flutter widget library", - "version": "7.5.21" + "version": "7.5.22" } diff --git a/.claude-plugin/skills/themed-table2/SKILL.md b/.claude-plugin/skills/themed-table2/SKILL.md new file mode 100644 index 0000000..a3fe9ce --- /dev/null +++ b/.claude-plugin/skills/themed-table2/SKILL.md @@ -0,0 +1,433 @@ +--- +name: themed-table2 +description: ThemedTable2 and ThemedColumn2 — large-dataset virtualized table with sort, search, multiselect and actions +--- + +## Overview + +`ThemedTable2` is the layrz_theme table widget designed for **large datasets** (tested up to 55,000+ rows). It uses Flutter's `ListView.builder` with a fixed `itemExtent` for virtualized rendering, and offloads sort operations to a background isolate via `compute()`. + +| Feature | Details | +|---|---| +| Rendering | Virtualized — only visible rows are built | +| Sort | Background isolate via `compute()`, non-blocking | +| Search | Debounced 600ms, searches all column values | +| Multiselect | Optional checkbox column with bulk actions | +| Actions | Per-row action buttons (icon-only on desktop, menu on mobile) | +| Scroll sync | Header + content + multiselect + actions scroll in sync | + +**When to use:** Any time you display a list of domain objects in a table format. Handles both small and large datasets — it's the standard table for all modules. + +--- + +## ThemedColumn2\ + +Defines a single column: its header, how to extract the display value from `T`, and optional rich rendering. + +### Key parameters + +| Parameter | Type | Default | Notes | +|---|---|---|---| +| `headerText` | `String` | required | Column header label | +| `valueBuilder` | `String Function(T)` | required | Extracts the plain string value — used for sort, search, and cell display. **Must be isolate-safe** (see Isolate Safety below) | +| `richTextBuilder` | `List Function(T)?` | `null` | Custom rich rendering for the cell. Does NOT affect sort — sort always uses `valueBuilder` | +| `width` | `double?` | `null` | Fixed width in pixels. If `null`, column takes a flexible share of available space | +| `alignment` | `Alignment` | `.centerLeft` | Cell content alignment | +| `isSortable` | `bool` | `true` | Whether clicking the header triggers sort | +| `onTap` | `void Function(T)?` | `null` | Per-cell tap handler. Overrides `onTapDefaultBehavior` | +| `customSort` | `int Function(T a, T b, bool ascending)?` | `null` | Custom comparator. **Must be isolate-safe** (see Isolate Safety below) | + +### Minimal column + +```dart +ThemedColumn2( + headerText: 'Name', + valueBuilder: (item) => item.name, +) +``` + +### Fixed-width column + +```dart +ThemedColumn2( + headerText: 'ID', + width: 80, + valueBuilder: (item) => item.id ?? 'N/A', +) +``` + +### Column with rich cell rendering + +```dart +// State — precompute labels map outside the build method +final Map statusLabels = { + 'active': i18n.t('status.active'), + 'inactive': i18n.t('status.inactive'), +}; + +// Widget +ThemedColumn2( + headerText: i18n.t('asset.status'), + valueBuilder: (item) => statusLabels[item.status] ?? item.status ?? 'N/A', + richTextBuilder: (item) => [ + WidgetSpan( + alignment: PlaceholderAlignment.middle, + child: StatusChip(status: item.status), + ), + ], +) +``` + +### Column with custom sort + +```dart +ThemedColumn2( + headerText: 'Plate', + valueBuilder: (item) => item.plate ?? 'N/A', + customSort: (a, b, ascending) { + // ascending == true means A→Z + final result = (a.plate ?? '').compareTo(b.plate ?? ''); + return ascending ? result : -result; + }, +) +``` + +--- + +## ThemedTable2\ + +### Key parameters + +| Parameter | Type | Default | Notes | +|---|---|---|---| +| `items` | `List` | required | Full dataset. Pass all items — filtering and sorting happen internally | +| `columns` | `List>` | required | At least one column required | +| `actionsCount` | `int` | `0` | Max number of actions per row. Set to `0` to hide the actions column. Must match the actual number returned by `actionsBuilder` | +| `actionsBuilder` | `List Function(T)?` | `null` | Required when `actionsCount > 0` | +| `canSearch` | `bool` | `true` | Shows the search input above the table | +| `hasMultiselect` | `bool` | `true` | Shows the checkbox column. Requires `multiselectActions` | +| `multiselectActions` | `List` | `[]` | Bulk action buttons shown when rows are selected. Required when `hasMultiselect: true` | +| `multiselectValue` | `ValueNotifier>?` | `null` | External notifier to read/control the selection from outside | +| `onTapDefaultBehavior` | `ThemedTable2OnTapBehavior` | `.copyToClipboard` | What happens when a cell without `onTap` is tapped | +| `controller` | `ThemedTable2Controller?` | `null` | Programmatic control: trigger sort or refresh from outside | +| `populateDelay` | `Duration` | `150ms` | Delay before showing data (gives the loading spinner time to appear) | +| `minColumnWidth` | `double` | `250` | Minimum width for flex columns | +| `headerHeight` | `double` | `40` | Header row height | +| `reloadOnDidUpdate` | `bool` | `false` | Forces reload on hot reload (debug only) | + +--- + +## Minimal usage + +```dart +// State +final List _assets = store.assets; + +// Widget +ThemedTable2( + items: _assets, + actionsCount: 0, + hasMultiselect: false, + columns: [ + ThemedColumn2( + headerText: 'Name', + valueBuilder: (item) => item.name, + ), + ThemedColumn2( + headerText: 'Plate', + valueBuilder: (item) => item.plate ?? 'N/A', + ), + ], +) +``` + +--- + +## Common patterns + +### With search + actions + +```dart +// State +final List _items = store.assets; +bool _isLoading = false; + +// Widget +ThemedTable2( + items: _items, + canSearch: true, + actionsCount: 2, + hasMultiselect: false, + columns: [ + ThemedColumn2( + headerText: i18n.t('asset.name'), + valueBuilder: (item) => item.name, + ), + ThemedColumn2( + headerText: i18n.t('asset.plate'), + width: 150, + valueBuilder: (item) => item.plate ?? 'N/A', + ), + ], + actionsBuilder: (item) => [ + ThemedActionButton.edit( + labelText: i18n.t('actions.edit'), + isLoading: _isLoading, + onTap: () => _onEdit(item), + ), + ThemedActionButton.delete( + labelText: i18n.t('actions.delete'), + isLoading: _isLoading, + onTap: () => _onDelete(item), + ), + ], +) +``` + +### With multiselect + +```dart +// State +final ValueNotifier> _selected = ValueNotifier([]); + +// Widget +ThemedTable2( + items: _items, + hasMultiselect: true, + actionsCount: 0, + multiselectValue: _selected, + multiselectActions: [ + ThemedActionButton( + icon: LayrzIcons.solarOutlineTrashBin, + labelText: i18n.t('actions.deleteSelected'), + color: Colors.red, + onTap: () => _onDeleteSelected(_selected.value), + ), + ], + columns: [ /* ... */ ], +) +``` + +### With richTextBuilder + +```dart +// State — precompute labels to avoid capturing i18n in valueBuilder +final Map _fuelLabels = { + for (final type in AtsFuelSubType.values) + if (type != AtsFuelSubType.unknown) + type.toJson(): i18n.t(type.getLocaleKey()), +}; + +// Widget +ThemedColumn2( + headerText: i18n.t('cacl.product'), + // valueBuilder must return a plain string — used for sort and search + valueBuilder: (item) => _fuelLabels[item.product] ?? 'N/A', + // richTextBuilder only affects visual rendering — not sort + richTextBuilder: (item) { + final type = AtsFuelSubType.fromJson(item.product ?? ''); + if (type == AtsFuelSubType.unknown) return [const TextSpan(text: 'N/A')]; + return [WidgetSpan(child: type.chip(i18n: i18n))]; + }, +) +``` + +### With programmatic controller + +```dart +// State +final _controller = ThemedTable2Controller(); + +@override +void dispose() { + _controller.dispose(); + super.dispose(); +} + +// Widget +ThemedTable2( + items: _items, + controller: _controller, + columns: [ /* ... */ ], + actionsCount: 0, + hasMultiselect: false, +) + +// Programmatic sort (column index 0, ascending) +_controller.sort(columnIndex: 0, ascending: true); + +// Programmatic refresh (re-runs filter + sort with current state) +_controller.refresh(); +``` + +### Disable copy-to-clipboard on cell tap + +```dart +ThemedTable2( + items: _items, + onTapDefaultBehavior: ThemedTable2OnTapBehavior.none, + columns: [ /* ... */ ], + actionsCount: 0, + hasMultiselect: false, +) +``` + +--- + +## ⚠️ Isolate Safety — CRITICAL + +`valueBuilder` and `customSort` are executed inside a background isolate via `compute()`. **They cannot capture any Flutter objects from the widget tree.** If they do, the app will crash at runtime with: + +``` +Invalid argument(s): Illegal argument in isolate message: object is unsendable +``` + +### What you CANNOT capture in valueBuilder or customSort + +- `BuildContext` (directly or indirectly) +- `LayrzAppLocalizations` / `i18n` — it's an `InheritedWidget` that holds a reference to `BuildContext` +- `State` objects +- Streams, `AnimationController`, `ValueNotifier` +- Any Flutter widget or element + +### Safe patterns + +```dart +// ❌ CRASH — i18n captures BuildContext +ThemedColumn2( + headerText: i18n.t('asset.status'), + valueBuilder: (item) => i18n.t('status.${item.status}'), +) + +// ✅ SAFE — precompute a simple Map of strings outside the column +final Map statusLabels = { + 'active': i18n.t('status.active'), + 'inactive': i18n.t('status.inactive'), + 'archived': i18n.t('status.archived'), +}; + +ThemedColumn2( + headerText: i18n.t('asset.status'), + valueBuilder: (item) => statusLabels[item.status] ?? item.status ?? 'N/A', +) + +// ✅ SAFE — pure function, no captured Flutter objects +ThemedColumn2( + headerText: 'Name', + valueBuilder: (item) => item.name.toUpperCase(), +) + +// ✅ SAFE — customSort with no captured state +ThemedColumn2( + headerText: 'Created', + valueBuilder: (item) => item.createdAt?.toIso8601String() ?? '', + customSort: (a, b, ascending) { + final dtA = a.createdAt ?? DateTime(0); + final dtB = b.createdAt ?? DateTime(0); + return ascending ? dtA.compareTo(dtB) : dtB.compareTo(dtA); + }, +) +``` + +### Rule of thumb + +> If you need translated labels in a column, build a `Map` from `i18n` **before** the `ThemedTable2(...)` call and reference that map inside `valueBuilder`. The map is a plain Dart object — sendable. `i18n` itself is not. + +--- + +## Anti-patterns + +| Anti-pattern | Why it's wrong | Fix | +|---|---|---| +| `valueBuilder: (item) => i18n.t(item.key)` | Captures `i18n` → `BuildContext` → non-sendable, runtime crash | Precompute a `Map` from `i18n` outside the column | +| `customSort` capturing `context` or `store` | Same crash as above | Use only item fields inside `customSort` | +| `actionsCount: 2` without `actionsBuilder` | Assert failure at construction | Always provide `actionsBuilder` when `actionsCount > 0` | +| `hasMultiselect: true` with empty `multiselectActions` | Assert failure at construction | Always provide at least one `multiselectAction` | +| Two columns with the same `headerText` | Header tooltip and sort icon display are ambiguous | Use distinct `headerText` values, or add `width` to differentiate | +| Calling `ThemedTable2` without wrapping in `Expanded` inside a `Column` | Table needs bounded height to render — will overflow or crash layout | Always wrap in `Expanded` or give it a fixed `height` | +| Using `item.hashCode` as a key in external Maps | `hashCode` can collide; use `identityHashCode(item)` for identity-based keys | Use `identityHashCode(item)` or a unique field like `item.id` | + +--- + +## Controller API + +```dart +final controller = ThemedTable2Controller(); + +// Sort by column index +controller.sort(columnIndex: 0, ascending: true); + +// Force re-filter and re-sort (useful when items changed externally) +controller.refresh(); + +// Always dispose +controller.dispose(); +``` + +The controller communicates via an event bus (`ThemedTable2SortEvent`, `ThemedTable2RefreshEvent`). Wire it up via the `controller` parameter on `ThemedTable2`. + +--- + +## Integration in a module view + +```dart +class _AssetsViewState extends State { + // State — precompute translated labels for isolate-safe valueBuilders + late final Map _categoryLabels = { + for (final cat in store.categories) + cat.id: cat.name, + }; + + @override + Widget build(BuildContext context) { + final i18n = LayrzAppLocalizations.of(context); + + return Expanded( + child: ThemedTable2( + items: store.assets, + canSearch: true, + actionsCount: 3, + hasMultiselect: true, + multiselectActions: [ + ThemedActionButton( + icon: LayrzIcons.solarOutlineTrashBin, + labelText: i18n.t('actions.deleteSelected'), + color: Colors.red, + onTap: _onDeleteSelected, + ), + ], + columns: [ + ThemedColumn2( + headerText: 'ID', + width: 80, + valueBuilder: (item) => item.id ?? 'N/A', + ), + ThemedColumn2( + headerText: i18n.t('asset.name'), + valueBuilder: (item) => item.name, + ), + ThemedColumn2( + headerText: i18n.t('asset.category'), + // Safe: _categoryLabels is a plain Map, not a Flutter object + valueBuilder: (item) => _categoryLabels[item.categoryId] ?? 'N/A', + ), + ], + actionsBuilder: (item) => [ + ThemedActionButton.show( + labelText: i18n.t('actions.show'), + onTap: () => _onShow(item), + ), + ThemedActionButton.edit( + labelText: i18n.t('actions.edit'), + onTap: () => _onEdit(item), + ), + ThemedActionButton.delete( + labelText: i18n.t('actions.delete'), + onTap: () => _onDelete(item), + ), + ], + ), + ); + } +} +``` diff --git a/CHANGELOG.md b/CHANGELOG.md index 4655d5f..4ac0a48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## 7.5.22 + +- Fixed `ThemedTable2` column key collision in `_itemsStrings`: inner key now uses column index instead of `col.hashCode`, preventing silent data corruption when two columns share the same `headerText`. +- Fixed `ThemedTable2` silent update loss: updates arriving while loading are now queued via `_pendingUpdate` flag and processed once the current operation completes. +- Fixed `ThemedTable2` unsendable closure crash: sort keys are now precomputed on the main thread before `compute()`, ensuring `valueBuilder` closures that capture `BuildContext` or `i18n` objects never cross the isolate boundary. +- Improved `ThemedTable2` multiselect performance: introduced an internal `Set` mirroring `_selectedItems` for O(1) `contains()` lookups instead of O(n) on the backing `List`. +- Improved `ThemedTable2` `didUpdateWidget` performance: replaced `DeepCollectionEquality` (O(n)) with a fast O(1) heuristic using `identical` + length check. +- Removed all `debugPrint` statements from `ThemedTable2._filterAndSort` (startup, precompute, sort, finish, queue logs). +- Added widget tests for `ThemedTable2` covering: renders item values after compute completes, shows loading indicator while computing, renders without errors on empty dataset, column key collision regression (two columns with identical `headerText` show distinct data), search filters by matching query, search matches values from all columns, search restores all items when cleared, `controller.sort` ascending (A→Z), `controller.sort` descending (Z→A), numeric sort as numbers not strings, `controller.refresh` preserves sort order, `didUpdateWidget` reflects new items on rebuild, `didUpdateWidget` handles growing list without losing existing rows. +- Add `ThemedTable2` skill. + ## 7.5.21 - Add claudio `Skills` diff --git a/example/pubspec.lock b/example/pubspec.lock index 33b7c71..9a06dd5 100644 --- a/example/pubspec.lock +++ b/example/pubspec.lock @@ -390,7 +390,7 @@ packages: path: ".." relative: true source: path - version: "7.5.18" + version: "7.5.22" leak_tracker: dependency: transitive description: diff --git a/lib/src/table2/src/table.dart b/lib/src/table2/src/table.dart index 4472202..b125763 100644 --- a/lib/src/table2/src/table.dart +++ b/lib/src/table2/src/table.dart @@ -164,7 +164,10 @@ class _ThemedTable2State extends State> { final ValueNotifier> _filteredData = ValueNotifier([]); /// [_itemsStrings] holds a precomputed list of string representations of the items for efficient searching. - Map> _itemsStrings = {}; + /// + /// Key: item.hashCode — assumes items have a stable, value-based hashCode (typical for domain objects). + /// Value: `List` indexed by column position — avoids column hashCode collisions. + Map> _itemsStrings = {}; /// [_colSelected] is the currently selected column used for sorting. late ThemedColumn2 _colSelected; @@ -178,9 +181,16 @@ class _ThemedTable2State extends State> { /// [_isLoading] indicates whether the table is currently loading or computing data. final ValueNotifier _isLoading = .new(false); + /// [_pendingUpdate] tracks whether a _filterAndSort call arrived while loading was in progress. + /// If true, _filterAndSort will re-run once the current operation finishes. + bool _pendingUpdate = false; + /// [_selectedItems] holds the list of currently selected items in multi-select mode. late ValueNotifier> _selectedItems; + /// [_selectedSet] mirrors [_selectedItems] as a Set for O(1) contains() lookups. + final Set _selectedSet = {}; + @override void initState() { super.initState(); @@ -196,6 +206,7 @@ class _ThemedTable2State extends State> { _actionsController = _verticalScrollControllerGroup.addAndGet(); _selectedItems = widget.multiselectValue ?? .new([]); + _selectedItems.addListener(_syncSelectedSet); widget.controller?.addListener(_onControllerEvent); @@ -206,9 +217,15 @@ class _ThemedTable2State extends State> { @override void didUpdateWidget(covariant ThemedTable2 oldWidget) { - final eq = const DeepCollectionEquality().equals; - final bool c1 = !eq(oldWidget.items, widget.items); - final bool c2 = !eq(oldWidget.columns, widget.columns); + // Use a fast heuristic instead of O(n) DeepCollectionEquality for large lists. + // Checks referential identity first, then length, then the first element. + final bool c1 = + !identical(oldWidget.items, widget.items) && + (oldWidget.items.length != widget.items.length || + (widget.items.isNotEmpty && !identical(oldWidget.items.first, widget.items.first))); + final bool c2 = + oldWidget.columns.length != widget.columns.length || + (widget.columns.isNotEmpty && oldWidget.columns.first != widget.columns.first); final bool c3 = oldWidget.actionsCount != widget.actionsCount; final bool c4 = oldWidget.canSearch != widget.canSearch; bool c5 = false; @@ -219,9 +236,10 @@ class _ThemedTable2State extends State> { } Future _filterAndSort(String source) async { - debugPrint("layrz_theme/ThemedTable2: Starting _filterAndSortAsync from $source..."); if (_isLoading.value) { - debugPrint('layrz_theme/ThemedTable2: Skipping _filterAndSortAsync from $source because is already loading'); + // Queue the update instead of silently dropping it. + // _filterAndSort will re-run once the current operation finishes. + _pendingUpdate = true; return; } @@ -230,52 +248,55 @@ class _ThemedTable2State extends State> { try { List items = .from(widget.items, growable: true); if (items.isEmpty) { - debugPrint("layrz_theme/ThemedTable2: No items to filter and sort from $source."); _filteredData.value = items; return; } - debugPrint("layrz_theme/ThemedTable2: Precomputing data from $source..."); + // Key: item.hashCode (stable for value-based domain objects, consistent across isolate boundaries). + // Value: List indexed by column position — avoids col.hashCode collisions + // (e.g. two columns with the same headerText). _itemsStrings = {}; for (final item in widget.items) { - int rowHashCode = item.hashCode; - _itemsStrings[rowHashCode] = {}; - - for (final col in widget.columns) { - final colHashCode = col.hashCode; - _itemsStrings[rowHashCode]![colHashCode] = col.valueBuilder(item); - } + _itemsStrings[item.hashCode] = [ + for (final col in widget.columns) col.valueBuilder(item), + ]; } if (_searchController.text.isNotEmpty) { - debugPrint("layrz_theme/ThemedTable2: Filtering data from $source..."); final searchLower = _searchController.text.toLowerCase(); items = items.where((row) { - final rowHashCode = row.hashCode; - final cols = _itemsStrings[rowHashCode]; + final cols = _itemsStrings[row.hashCode]; if (cols == null) return false; - for (final entry in cols.entries) { - if (entry.value.toLowerCase().contains(searchLower)) return true; + for (final value in cols) { + if (value.toLowerCase().contains(searchLower)) return true; } return false; }).toList(); } - debugPrint("layrz_theme/ThemedTable2: Sorting data..."); - + // Precompute sort keys on the main thread so that valueBuilder closures + // (which may capture BuildContext or i18n objects) never cross the isolate boundary. + final columnIndex = widget.columns.indexOf(_colSelected); + final sortKeys = [ + for (final item in items) _itemsStrings[item.hashCode]?[columnIndex] ?? _colSelected.valueBuilder(item), + ]; _filteredData.value = await compute( _sort, _SortParams( items: items, - column: _colSelected.isolateSafety, + sortKeys: sortKeys, isReversed: _isReversed, - itemsStrings: _itemsStrings, + customSort: _colSelected.customSort, ), ); } finally { - debugPrint("layrz_theme/ThemedTable2: Finished filtering and sorting from $source."); _isLoading.value = false; if (mounted) WidgetsBinding.instance.addPostFrameCallback((_) => setState(() {})); + // Process a queued update that arrived while we were loading. + if (_pendingUpdate && mounted) { + _pendingUpdate = false; + _filterAndSort('PENDING_UPDATE'); + } } } @@ -294,18 +315,25 @@ class _ThemedTable2State extends State> { _horizontalHeaderController.dispose(); _horizontalContentController.dispose(); + _selectedItems.removeListener(_syncSelectedSet); widget.controller?.removeListener(_onControllerEvent); super.dispose(); } + /// Keeps [_selectedSet] in sync with [_selectedItems] for O(1) contains() lookups. + void _syncSelectedSet() { + _selectedSet + ..clear() + ..addAll(_selectedItems.value); + } + void _onControllerEvent(ThemedTable2Event event) { if (event is ThemedTable2SortEvent) { final columnIndex = event.columnIndex; final ascending = event.ascending; if (columnIndex < 0 || columnIndex >= widget.columns.length) { - debugPrint('layrz_theme/ThemedTable2: Invalid column index $columnIndex for sorting'); return; } @@ -320,8 +348,6 @@ class _ThemedTable2State extends State> { _filterAndSort('CONTROLLER_REFRESH'); return; } - - debugPrint('layrz_theme/ThemedTable2: Unknown controller event type: ${event.runtimeType}'); } @override @@ -602,15 +628,20 @@ class _ThemedTable2State extends State> { color: index % 2 == 0 ? null : _stripColor, child: ValueListenableBuilder( valueListenable: _selectedItems, - builder: (context, value, child) { + builder: (context, selectedList, child) { + // O(1) lookup via the mirrored Set instead of O(n) List.contains() return Checkbox( - value: value.contains(item), + value: _selectedSet.contains(item), onChanged: (val) { if (val == true) { - if (!value.contains(item)) _selectedItems.value = [...value, item]; + if (!_selectedSet.contains(item)) { + _selectedItems.value = [...selectedList, item]; + } } else { - if (value.contains(item)) { - _selectedItems.value = value.where((i) => i != item).toList(); + if (_selectedSet.contains(item)) { + _selectedItems.value = selectedList + .where((i) => i != item) + .toList(); } } }, @@ -657,9 +688,10 @@ class _ThemedTable2State extends State> { final header = entry.value; final colIndex = entry.key; + // Use column index as key (avoids col.hashCode collisions + // when two columns share the same headerText). String text = - _itemsStrings[data.hashCode]?[header.hashCode] ?? - header.valueBuilder(data); + _itemsStrings[data.hashCode]?[colIndex] ?? header.valueBuilder(data); Widget child; if (header.richTextBuilder != null) { @@ -871,15 +903,23 @@ class _ThemedTable2State extends State> { class _SortParams { final List items; - final ThemedColumn2 column; + + /// [sortKeys] are the precomputed string values for the sort column, computed on the + /// main thread. This ensures that [valueBuilder] closures (which may capture [BuildContext] + /// or localization objects) never cross the isolate boundary. + final List sortKeys; + final bool isReversed; - final Map> itemsStrings; + + /// [customSort] is an optional custom comparator. If provided, it must not capture + /// any non-sendable objects (e.g. BuildContext, i18n, streams). + final int Function(T a, T b, bool ascending)? customSort; _SortParams({ required this.items, - required this.column, + required this.sortKeys, required this.isReversed, - this.itemsStrings = const {}, + this.customSort, }); } @@ -887,44 +927,29 @@ class _SortParams { /// /// Requires the [_SortParams] containing: /// - [items]: The list of items to sort. -/// - [column]: The column to sort by. +/// - [sortKeys]: Precomputed sort key strings (one per item), built on the main thread. /// - [isReversed]: Whether to sort in descending order. -/// - [itemsStrings]: A precomputed map of string representations of items for efficient sorting. +/// - [customSort]: Optional custom comparator (must not capture non-sendable objects). /// -/// This function runs on an Isolated thread to ensure non-blocking UI performance, the only issue with this -/// is, you cannot use complexes objects or functions that are not sendable between isolates. +/// Sort keys are precomputed on the main thread before entering the isolate, so that +/// valueBuilder closures (which may capture BuildContext, i18n, etc.) never cross +/// the isolate boundary. List _sort(_SortParams params) { - if (params.column.customSort != null) { - params.items.sort((a, b) => params.column.customSort!.call(a, b, !params.isReversed)); - } else { - params.items.sort( - (a, b) => _defaultSort( - a, - b, - colSelected: params.column, - isReversed: params.isReversed, - itemsStrings: params.itemsStrings, - ), - ); + if (params.customSort != null) { + params.items.sort((a, b) => params.customSort!.call(a, b, !params.isReversed)); + return params.items; } - return params.items; -} -/// [_defaultSort] is the default sorting function used when no custom sort is provided. -int _defaultSort( - T a, - T b, { - required ThemedColumn2 colSelected, - required bool isReversed, - required Map> itemsStrings, -}) { - final colHashCode = colSelected.hashCode; - final rowAHashCode = a.hashCode; - final rowBHashCode = b.hashCode; - - final valueA = itemsStrings[rowAHashCode]?[colHashCode] ?? colSelected.valueBuilder.call(a); - final valueB = itemsStrings[rowBHashCode]?[colHashCode] ?? colSelected.valueBuilder.call(b); + // Sort a parallel index array using the precomputed keys, then reorder items. + final indices = List.generate(params.items.length, (i) => i); + indices.sort((a, b) => _defaultSort(params.sortKeys[a], params.sortKeys[b], isReversed: params.isReversed)); + + return [for (final i in indices) params.items[i]]; +} +/// [_defaultSort] compares two precomputed string values, attempting numeric, duration, +/// datetime, and finally lexicographic comparison in that order. +int _defaultSort(String valueA, String valueB, {required bool isReversed}) { final numA = num.tryParse(valueA); final numB = num.tryParse(valueB); if (numA != null && numB != null) { diff --git a/pubspec.yaml b/pubspec.yaml index f48d6c1..f6d8b8a 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,6 +1,6 @@ name: layrz_theme description: Layrz standard styling library for Flutter. Widget library following the Material Design 3 guidelines, with a focus on reliavility and functionality. -version: "7.5.21" +version: "7.5.22" homepage: https://theme.layrz.com repository: https://github.com/goldenm-software/layrz_theme diff --git a/test/widgets/table2_test.dart b/test/widgets/table2_test.dart new file mode 100644 index 0000000..ef672a0 --- /dev/null +++ b/test/widgets/table2_test.dart @@ -0,0 +1,468 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:layrz_theme/layrz_theme.dart'; + +// Minimal domain object for tests. +// Value-based hashCode ensures item.hashCode is stable across isolate boundaries. +class _Item { + final String id; + final String name; + final String secondary; + + const _Item({required this.id, required this.name, required this.secondary}); + + @override + bool operator ==(Object other) => + identical(this, other) || other is _Item && id == other.id; + + @override + int get hashCode => id.hashCode; +} + +// Waits for a _filterAndSort cycle to complete. +// +// compute() spawns a REAL isolate even in widget tests. pumpAndSettle() cannot +// be used because: +// 1. CircularProgressIndicator animates while loading → frames never settle. +// 2. pump(duration) advances the FAKE scheduler clock, not real wall-clock time, +// so the isolate may not finish within that fake window. +// +// Strategy: +// 1. pump(1ms) — advances the fake clock by 1ms, which fires Future.delayed(Duration.zero) +// (populateDelay). pump(Duration.zero) does NOT fire zero-duration timers because +// FakeAsync uses strict less-than comparison (elapsed > fireTime). Advancing by ≥1μs +// makes the timer due and _filterAndSort continues, kicking off compute(). +// 2. runAsync(500ms) — exits fake-async so the real isolate gets CPU time to finish. +// 3. Three pump() calls — flush _filteredData ValueNotifier, _isLoading = false, +// addPostFrameCallback setState, and the final tree rebuild. +Future _waitForCompute(WidgetTester tester) async { + // Advance the fake clock by 1ms so Future.delayed(Duration.zero) fires. + await tester.pump(const Duration(milliseconds: 1)); + await tester.runAsync(() async { + // 500ms of REAL time is more than enough for a small-dataset isolate. + await Future.delayed(const Duration(milliseconds: 500)); + }); + await tester.pump(); // flush _filteredData update + _isLoading = false + await tester.pump(); // flush addPostFrameCallback → setState + await tester.pump(); // final rebuild +} + +// Builds a bounded, minimal table for testing. +Widget _buildTable( + List<_Item> items, { + List>? columns, + bool canSearch = false, + ThemedTable2Controller<_Item>? controller, +}) { + return MaterialApp( + home: Scaffold( + body: SizedBox( + height: 600, + width: 800, + child: ThemedTable2<_Item>( + items: items, + actionsCount: 0, + hasMultiselect: false, + canSearch: canSearch, + populateDelay: Duration.zero, + controller: controller, + columns: columns ?? + [ + ThemedColumn2<_Item>( + headerText: 'Name', + valueBuilder: (item) => item.name, + ), + ], + ), + ), + ), + ); +} + +void main() { + group('ThemedTable2', () { + // ───────────────────────────────────────────── + // Rendering + // ───────────────────────────────────────────── + group('rendering', () { + testWidgets('renders item values after compute completes', (tester) async { + final items = [ + const _Item(id: '1', name: 'Alpha', secondary: 'X'), + const _Item(id: '2', name: 'Beta', secondary: 'Y'), + const _Item(id: '3', name: 'Gamma', secondary: 'Z'), + ]; + + await tester.pumpWidget(_buildTable(items)); + await tester.pump(); // trigger postFrameCallback → _filterAndSort + await _waitForCompute(tester); + + expect(find.text('Alpha'), findsOneWidget); + expect(find.text('Beta'), findsOneWidget); + expect(find.text('Gamma'), findsOneWidget); + }); + + testWidgets('shows CircularProgressIndicator while loading', (tester) async { + final items = [ + const _Item(id: '1', name: 'Item 1', secondary: ''), + const _Item(id: '2', name: 'Item 2', secondary: ''), + ]; + + await tester.pumpWidget(_buildTable(items)); + + // After first pump, postFrameCallback fires and _isLoading becomes true. + await tester.pump(); + expect(find.byType(CircularProgressIndicator), findsOneWidget); + + // After compute completes, loading disappears. + await _waitForCompute(tester); + expect(find.byType(CircularProgressIndicator), findsNothing); + expect(find.text('Item 1'), findsOneWidget); + }); + + testWidgets('renders without errors when items is empty', (tester) async { + await tester.pumpWidget(_buildTable([])); + await tester.pump(); + await _waitForCompute(tester); + + expect(tester.takeException(), isNull); + expect(find.byType(ThemedTable2<_Item>), findsOneWidget); + }); + }); + + // ───────────────────────────────────────────── + // Column key collision regression + // ───────────────────────────────────────────── + group('column key collision regression', () { + testWidgets( + 'two columns with identical headerText show distinct data', + (tester) async { + // Before the fix, the inner key of _itemsStrings was col.hashCode. + // hashCode == headerText.hashCode, so two columns named 'Label' + // would collide — the second overwrites the first and BOTH cells + // display the same value. + // + // After the fix, column INDEX is used as the inner key, so each + // column is independent regardless of shared headerText. + final items = [ + const _Item(id: '1', name: 'Alice', secondary: 'Engineer'), + ]; + + await tester.pumpWidget( + _buildTable( + items, + columns: [ + ThemedColumn2<_Item>( + headerText: 'Label', + valueBuilder: (item) => item.name, + ), + ThemedColumn2<_Item>( + headerText: 'Label', + valueBuilder: (item) => item.secondary, + ), + ], + ), + ); + await tester.pump(); + await _waitForCompute(tester); + + // Both distinct values must be visible. + // If only one appears, column collision has returned. + expect(find.text('Alice'), findsOneWidget); + expect(find.text('Engineer'), findsOneWidget); + }, + ); + }); + + // ───────────────────────────────────────────── + // Search + // ───────────────────────────────────────────── + group('search', () { + testWidgets('filters items matching the query', (tester) async { + final items = [ + const _Item(id: '1', name: 'Apple', secondary: 'Fruit'), + const _Item(id: '2', name: 'Banana', secondary: 'Fruit'), + const _Item(id: '3', name: 'Carrot', secondary: 'Veggie'), + ]; + + await tester.pumpWidget(_buildTable(items, canSearch: true)); + await tester.pump(); + await _waitForCompute(tester); + + expect(find.text('Apple'), findsOneWidget); + expect(find.text('Banana'), findsOneWidget); + expect(find.text('Carrot'), findsOneWidget); + + await tester.enterText(find.byType(TextField), 'Apple'); + await tester.pump(const Duration(milliseconds: 650)); // past 600ms debounce + await _waitForCompute(tester); + + // 'Apple' may appear in both the search input and the table row. + // The key invariant is that non-matching items are gone. + expect(find.text('Apple'), findsAtLeastNWidgets(1)); + expect(find.text('Banana'), findsNothing); + expect(find.text('Carrot'), findsNothing); + }); + + testWidgets('search matches values from all columns', (tester) async { + final items = [ + const _Item(id: '1', name: 'Alpha', secondary: 'Engineer'), + const _Item(id: '2', name: 'Beta', secondary: 'Designer'), + ]; + + await tester.pumpWidget( + _buildTable( + items, + canSearch: true, + columns: [ + ThemedColumn2<_Item>( + headerText: 'Name', + valueBuilder: (item) => item.name, + ), + ThemedColumn2<_Item>( + headerText: 'Role', + valueBuilder: (item) => item.secondary, + ), + ], + ), + ); + await tester.pump(); + await _waitForCompute(tester); + + // Search by secondary column value — 'Designer' only belongs to Beta. + await tester.enterText(find.byType(TextField), 'Designer'); + await tester.pump(const Duration(milliseconds: 650)); + await _waitForCompute(tester); + + expect(find.text('Beta'), findsOneWidget); + expect(find.text('Alpha'), findsNothing); + }); + + testWidgets('shows all items when search is cleared', (tester) async { + final items = [ + const _Item(id: '1', name: 'Apple', secondary: ''), + const _Item(id: '2', name: 'Banana', secondary: ''), + ]; + + await tester.pumpWidget(_buildTable(items, canSearch: true)); + await tester.pump(); + await _waitForCompute(tester); + + await tester.enterText(find.byType(TextField), 'Apple'); + await tester.pump(const Duration(milliseconds: 650)); + await _waitForCompute(tester); + expect(find.text('Banana'), findsNothing); + + await tester.enterText(find.byType(TextField), ''); + await tester.pump(const Duration(milliseconds: 650)); + await _waitForCompute(tester); + + expect(find.text('Apple'), findsOneWidget); + expect(find.text('Banana'), findsOneWidget); + }); + }); + + // ───────────────────────────────────────────── + // Sort (via controller — avoids RichText header finders) + // ───────────────────────────────────────────── + group('sort', () { + testWidgets('controller.sort ascending orders items A→Z', (tester) async { + final controller = ThemedTable2Controller<_Item>(); + addTearDown(controller.dispose); + + final items = [ + const _Item(id: '1', name: 'Zebra', secondary: ''), + const _Item(id: '2', name: 'Apple', secondary: ''), + const _Item(id: '3', name: 'Mango', secondary: ''), + ]; + + await tester.pumpWidget(_buildTable(items, controller: controller)); + await tester.pump(); + await _waitForCompute(tester); + + controller.sort(columnIndex: 0, ascending: true); + await _waitForCompute(tester); + + final appleY = tester.getTopLeft(find.text('Apple')).dy; + final mangoY = tester.getTopLeft(find.text('Mango')).dy; + final zebraY = tester.getTopLeft(find.text('Zebra')).dy; + + expect(appleY, lessThan(mangoY), reason: 'Apple must appear before Mango'); + expect(mangoY, lessThan(zebraY), reason: 'Mango must appear before Zebra'); + }); + + testWidgets('controller.sort descending orders items Z→A', (tester) async { + final controller = ThemedTable2Controller<_Item>(); + addTearDown(controller.dispose); + + final items = [ + const _Item(id: '1', name: 'Apple', secondary: ''), + const _Item(id: '2', name: 'Mango', secondary: ''), + const _Item(id: '3', name: 'Zebra', secondary: ''), + ]; + + await tester.pumpWidget(_buildTable(items, controller: controller)); + await tester.pump(); + await _waitForCompute(tester); + + controller.sort(columnIndex: 0, ascending: false); + await _waitForCompute(tester); + + final appleY = tester.getTopLeft(find.text('Apple')).dy; + final zebraY = tester.getTopLeft(find.text('Zebra')).dy; + + expect(zebraY, lessThan(appleY), reason: 'Zebra must appear before Apple in descending'); + }); + + testWidgets('numeric values sort as numbers not as strings', (tester) async { + final controller = ThemedTable2Controller<_Item>(); + addTearDown(controller.dispose); + + // '10' > '9' numerically, but '10' < '9' lexicographically. + final items = [ + const _Item(id: '1', name: '9', secondary: ''), + const _Item(id: '2', name: '10', secondary: ''), + const _Item(id: '3', name: '2', secondary: ''), + ]; + + await tester.pumpWidget(_buildTable(items, controller: controller)); + await tester.pump(); + await _waitForCompute(tester); + + controller.sort(columnIndex: 0, ascending: true); + await _waitForCompute(tester); + + final y2 = tester.getTopLeft(find.text('2')).dy; + final y9 = tester.getTopLeft(find.text('9')).dy; + final y10 = tester.getTopLeft(find.text('10')).dy; + + // Numeric order: 2 < 9 < 10 + expect(y2, lessThan(y9), reason: '2 must appear before 9'); + expect(y9, lessThan(y10), reason: '9 must appear before 10'); + }); + + testWidgets('controller.refresh re-runs sort without changing order', (tester) async { + final controller = ThemedTable2Controller<_Item>(); + addTearDown(controller.dispose); + + final items = [ + const _Item(id: '1', name: 'Zebra', secondary: ''), + const _Item(id: '2', name: 'Apple', secondary: ''), + ]; + + await tester.pumpWidget(_buildTable(items, controller: controller)); + await tester.pump(); + await _waitForCompute(tester); + + controller.sort(columnIndex: 0, ascending: true); + await _waitForCompute(tester); + + controller.refresh(); + await _waitForCompute(tester); + + final appleY = tester.getTopLeft(find.text('Apple')).dy; + final zebraY = tester.getTopLeft(find.text('Zebra')).dy; + expect(appleY, lessThan(zebraY), reason: 'Sort order must be preserved after refresh'); + }); + }); + + // ───────────────────────────────────────────── + // didUpdateWidget + // ───────────────────────────────────────────── + group('didUpdateWidget', () { + testWidgets('reflects new items when widget rebuilds with a different list', (tester) async { + List<_Item> items = [const _Item(id: '1', name: 'Original', secondary: '')]; + late StateSetter rebuildState; + + await tester.pumpWidget( + StatefulBuilder( + builder: (context, setState) { + rebuildState = setState; + return MaterialApp( + home: Scaffold( + body: SizedBox( + height: 600, + width: 800, + child: ThemedTable2<_Item>( + items: items, + actionsCount: 0, + hasMultiselect: false, + canSearch: false, + populateDelay: Duration.zero, + columns: [ + ThemedColumn2<_Item>( + headerText: 'Name', + valueBuilder: (item) => item.name, + ), + ], + ), + ), + ), + ); + }, + ), + ); + await tester.pump(); + await _waitForCompute(tester); + + expect(find.text('Original'), findsOneWidget); + + rebuildState(() { + items = [const _Item(id: '2', name: 'Updated', secondary: '')]; + }); + await tester.pump(); + await _waitForCompute(tester); + + expect(find.text('Original'), findsNothing); + expect(find.text('Updated'), findsOneWidget); + }); + + testWidgets('handles growing list without losing existing rows', (tester) async { + List<_Item> items = [const _Item(id: '1', name: 'First', secondary: '')]; + late StateSetter rebuildState; + + await tester.pumpWidget( + StatefulBuilder( + builder: (context, setState) { + rebuildState = setState; + return MaterialApp( + home: Scaffold( + body: SizedBox( + height: 600, + width: 800, + child: ThemedTable2<_Item>( + items: items, + actionsCount: 0, + hasMultiselect: false, + canSearch: false, + populateDelay: Duration.zero, + columns: [ + ThemedColumn2<_Item>( + headerText: 'Name', + valueBuilder: (item) => item.name, + ), + ], + ), + ), + ), + ); + }, + ), + ); + await tester.pump(); + await _waitForCompute(tester); + + rebuildState(() { + items = [ + const _Item(id: '1', name: 'First', secondary: ''), + const _Item(id: '2', name: 'Second', secondary: ''), + ]; + }); + await tester.pump(); + await _waitForCompute(tester); + + expect(find.text('First'), findsOneWidget); + expect(find.text('Second'), findsOneWidget); + }); + }); + }); +}