Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class DevConnectMMKV {
* @returns A proxied MMKV-like object that auto-reports operations
*/
static wrap(mmkv: any, label: string = 'mmkv'): any {
const storageType = 'mmkv';
const storageType = `mmkv:${label}`;

// Resolve delete/remove — v4: .remove(), v3: .delete()
const deleteFn: ((key: string) => any) | undefined =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
# State Page — Tree/Pretty as Nested Tab Bar

## Problem

Switching the state-detail panel between Tree and JSON views is currently a
small chip toggle in the header row (one chip for both Before & After).
Per the user, this is awkward and inconsistent with how other pages in the
app let the user pick a viewer mode.

Goal: replace the chip with a proper segmented-control tab bar (matching the
existing `DetailTabBar` pattern used by Network Inspector, All Events,
Console), placed inside the Before and After tabs so the two views can be
chosen independently.

## Approach

Nested tab bar (Option A from the brainstorming):

```
┌─ Detail header ────────────────────────────┐
│ actionName [screenshot] [×] │
├────────────────────────────────────────────┤
│ [ Diff | Before | After ] │ ← outer tab bar (existing)
├────────────────────────────────────────────┤
│ ┌─ Before ─────────────────────────────┐ │
│ │ [ Tree | Pretty ] │ ← NEW inner tab bar
│ │ ────────────────────────────────────│ │
│ │ { … JSON tree … } │ │
│ └────────────────────────────────────────┘ │
│ ┌─ After ──────────────────────────────┐ │
│ │ [ Tree | Pretty ] │ ← NEW inner tab bar
│ │ ────────────────────────────────────│ │
│ │ { … JSON tree … } │ │
│ └────────────────────────────────────────┘ │
└────────────────────────────────────────────┘
```

Before and After each get their own inner tab controller, so the user can
view the Before tree and the After pretty (or any combination) at the same
time.

## Architecture

### New widget

`_StateJsonTabView` (private to `state_inspector_page.dart`, replacing
`_StateJsonToggleView`):

- `final dynamic data` — the state map to render.
- Stateful — owns its own `TabController` (length 2) so Before and After
have independent selection.
- Uses `DefaultTabController` so the inner `TabBarView` and `TabBar` wire
up without explicit plumbing.
- Renders:
- `_DetailTabBar(tabs: const ['Tree', 'Pretty'])` at the top.
- `TabBarView` children:
- Tree: `JsonViewer(data: widget.data, initiallyExpanded: true)`
(same as current Tree mode).
- Pretty: `JsonPrettyViewer(data: widget.data)`
(same as current JSON mode).
- Default tab: Tree (index 0) — matches current default behaviour.

### State changes

In `_StateInspectorPageState`:

- **Remove** `bool _jsonPrettyMode = false;` field (line 521).
- **Remove** the chip toggle `GestureDetector` + `Container` block in the
header (lines 678-721).
- **Replace** the two `_StateJsonToggleView(...)` calls inside the
`TabBarView` (`_StateJsonToggleView(data: entry.previousState, ...)` /
`…nextState, ...`) with `_StateJsonTabView(data: …)`.
- **Delete** the `_StateJsonToggleView` and `_StateJsonToggleViewState`
classes (lines 779-836).

### Why nested `DefaultTabController`

The outer detail already uses a `DefaultTabController(length: 3)` for the
Diff/Before/After bar (line 651). Nested `DefaultTabController`s work in
Flutter because `TabBarView` looks up the nearest ancestor controller via
`DefaultTabController.of(...)` — a child can wrap a sub-tree in its own
`DefaultTabController` without affecting the parent. Each `_StateJsonTabView`
gets its own controller, so Before's Tree/Pretty selection is independent
of After's.

### Localisation

Use existing strings:
- `S.of(context).tree` → tab label "Tree"
- `S.of(context).pretty` → tab label "Pretty"

No new i18n keys needed.

## Trade-offs

- **Pro:** Same widget (`DetailTabBar`) as Network / All Events / Console —
visual consistency across the app.
- **Pro:** Click target is bigger (full pill segment) instead of a 12×12
icon chip.
- **Pro:** Per-tab independence — user can put Before in Pretty and After
in Tree, or vice versa.
- **Con:** Nested tab bar (Diff/Before/After outside, Tree/Pretty inside).
Recognised pattern in IDEs and code viewers; acceptable for a developer
tool. Vertical screen real estate drops slightly because the inner tab
bar adds ~36 px inside each tab.
- **Con:** No persistent preference — switching tabs resets Tree/Pretty to
default (Tree). Same as current behaviour; if persistence becomes
useful later, add via a shared preference.

## Testing

No automated tests for the state-detail UI today. Per YAGNI, skip writing
new ones for this change. Manual verification:

1. Open state detail from All Events.
2. Click "Pretty" inside Before tab → JSON pretty renders.
3. Switch to After tab → After defaults back to Tree.
4. Switch back to Before → Before still on Pretty (independent state).
5. Switch to Diff tab and back to Before → Before still on Pretty.

## Out of scope

- Persistence of Tree/Pretty preference across sessions.
- Keyboard shortcut to toggle Tree/Pretty.
- Per-state-manager-type default (e.g. Redux always starts in Pretty).
48 changes: 44 additions & 4 deletions lib/features/all_events/presentation/pages/all_events_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ class _AllEventsPageState extends ConsumerState<AllEventsPage> {
int _visibleCount = 0;
final List<UnifiedEvent> _events = [];

/// Pin of the currently-selected [UnifiedEvent]. Survives the
/// `_events..clear()..addAll(next.items)` churn in the listener and
/// survives the display-limit trim that drops older entries — so the
/// detail panel never disappears just because a new entry arrived
/// (Bug B) or because the user picked an older entry that's now
/// outside the visible window.
UnifiedEvent? _pinnedSelectedEvent;

@override
void initState() {
super.initState();
Expand All @@ -64,6 +72,24 @@ class _AllEventsPageState extends ConsumerState<AllEventsPage> {
_eventCount.value = next.items.length;
_visibleCount = next.items.length;
_untrimmedCount.value = next.total;
// Keep `_pinnedSelectedEvent` in sync with the latest copy of
// the selected entry — content (e.g. network body after a
// start→complete merge) updates without dropping the user's
// pinned tab/scroll position.
//
// Only refresh the pin when the refresh list still contains the
// selected ID. If display-limit trimming removed the older entry,
// `_findEvent` returns null and the previous pin stays — the
// detail panel survives an out-of-window selection. The pin is
// cleared only by explicit selection/reset paths (see
// `_clearAll`, `_onSelectRow`, etc.).
final selectedId = _selectedEventId.value;
if (selectedId != null) {
final updated = _findEvent(selectedId);
if (updated != null) {
_pinnedSelectedEvent = updated;
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
setState(() {});
if (_autoScroll) _autoScrollIfNeeded();
},
Expand Down Expand Up @@ -176,6 +202,7 @@ class _AllEventsPageState extends ConsumerState<AllEventsPage> {
ref.read(memoryLeakEntriesProvider.notifier).clear();
ref.read(benchmarkEntriesProvider.notifier).clear();
_selectedEventId.value = null;
_pinnedSelectedEvent = null;
_events.clear();
_eventCount.value = 0;
_untrimmedCount.value = 0;
Expand Down Expand Up @@ -434,8 +461,11 @@ class _AllEventsPageState extends ConsumerState<AllEventsPage> {
showDetail: false,
platform: device?.platform,
onTap: () {
_selectedEventId.value =
final nextId =
isSelected ? null : event.id;
_selectedEventId.value = nextId;
_pinnedSelectedEvent =
nextId == null ? null : event;
if (!isSelected && _autoScroll) {
_autoScroll = false;
_programmaticScroll = false;
Expand Down Expand Up @@ -463,7 +493,15 @@ class _AllEventsPageState extends ConsumerState<AllEventsPage> {
ValueListenableBuilder<String?>(
valueListenable: _selectedEventId,
builder: (context, selectedId, _) {
final selectedEvent = _findEvent(selectedId);
// Prefer the pinned event so the panel
// survives display-limit trims that drop
// older entries and survives the brief
// window during `_events..clear()` when
// `_findEvent` would return null.
final selectedEvent = _pinnedSelectedEvent ??
(selectedId == null
? null
: _findEvent(selectedId));
if (selectedEvent == null) {
return const SizedBox.shrink();
}
Expand All @@ -482,8 +520,10 @@ class _AllEventsPageState extends ConsumerState<AllEventsPage> {
child: EventDetailPanel(
key: ValueKey(selectedEvent.id),
event: selectedEvent,
onClose: () =>
_selectedEventId.value = null,
onClose: () {
_selectedEventId.value = null;
_pinnedSelectedEvent = null;
},
),
),
],
Expand Down
28 changes: 12 additions & 16 deletions lib/features/console/presentation/pages/console_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,25 +44,21 @@ class _ConsolePageState extends ConsumerState<ConsolePage> {
void initState() {
super.initState();
_scrollController.addListener(_onScroll);
// Mirror the network_inspector listener pattern: always bump
// `_generation` on every provider change. The earlier incremental
// vs full-replace split skipped the bump on the append path, so
// `StableListView.shouldRebuild` returned false and new entries
// were never materialised in the viewport even though
// `_visibleCount` had grown.
ref.listenManual<List<LogEntry>>(
filteredConsoleEntriesProvider,
(previous, next) {
final prevLen = _entries.length;
if (next.length > prevLen && previous != null && next.length - prevLen == next.length - previous.length) {
_entries.addAll(next.sublist(prevLen));
_entryCount.value = _entries.length;
if (!_autoScroll) return;
_visibleCount = _entries.length;
setState(() {});
_autoScrollIfNeeded();
} else {
_entries..clear()..addAll(next);
_entryCount.value = _entries.length;
_visibleCount = _entries.length;
_generation++;
setState(() {});
if (_autoScroll) _autoScrollIfNeeded();
}
_entries..clear()..addAll(next);
_entryCount.value = _entries.length;
_visibleCount = _entries.length;
_generation++;
setState(() {});
if (_autoScroll) _autoScrollIfNeeded();
},
fireImmediately: true,
);
Expand Down
Loading
Loading