diff --git a/CHANGELOG.md b/CHANGELOG.md index 473fbf18..e4975753 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -194,6 +194,12 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Changed +- The conversation stream's **collapsible cards** (activity runs, edit runs, + sub-agent runs) now share one `ClideCollapserCard` primitive with consistent + chrome: the collapsed ticker leads with the card's label, the step/edit count + sits in a fixed-width slot, and the status tick (spinner / check / cross) is + pinned hard against the card's right edge while the chevron hugs the left. A + `color` drives the border + label tint per card type. (T-305) - The Claude composer's **slash typeahead**, the team-chat **@-mention** list, and the status-bar **theme switcher** now ride the shared `ClideAnchoredOverlay` + `ClideMenu` popover primitive, alongside the menu diff --git a/lib/builtin/claude/src/conversation_view.dart b/lib/builtin/claude/src/conversation_view.dart index fc320c74..25ba8120 100644 --- a/lib/builtin/claude/src/conversation_view.dart +++ b/lib/builtin/claude/src/conversation_view.dart @@ -16,7 +16,6 @@ import 'dart:io'; import 'package:clide/builtin/claude/src/activity_cluster.dart'; import 'package:clide/builtin/claude/src/conversation_card.dart'; import 'package:clide/builtin/claude/src/conversation_controller.dart'; -import 'package:clide/builtin/claude/src/holder_card.dart'; import 'package:clide/builtin/claude/src/image_thumbnail.dart'; import 'package:clide/builtin/claude/src/prompt_card.dart'; import 'package:clide/builtin/claude/src/transcript_reader.dart'; @@ -603,10 +602,10 @@ class _ConversationTurn extends StatelessWidget { card, Padding( padding: const EdgeInsets.only(left: 12), - child: ClideHolderCard( - title: 'agent run', + child: ClideCollapserCard( + label: 'agent run', collapsedSummary: _summarizeActivity(runItems.last), - stepLabel: runItems.length == 1 ? '1 step' : '${runItems.length} steps', + counter: runItems.length == 1 ? '1 step' : '${runItems.length} steps', children: [ for (final r in runItems) _ConversationTurn( @@ -711,11 +710,11 @@ class _ConversationTurn extends StatelessWidget { } /// A folded run of meta items rendered as one collapsible activity card -/// (T-230), now through the shared [ClideHolderCard] container (T-266). +/// (T-230), now through the shared [ClideCollapserCard] container (T-305). /// Collapsed (default): a one-line live ticker of the latest step + a step /// count — re-grouped on every rebuild, so the ticker updates in place as the -/// run grows. Expanded: every folded step, wrapped in the holder frame whose -/// background toggles collapse. Stateless — the holder owns the expand state. +/// run grows. Expanded: every folded step, wrapped in the collapser frame whose +/// background toggles collapse. Stateless — the collapser owns the expand state. class _ActivityCard extends StatelessWidget { const _ActivityCard({ super.key, @@ -739,9 +738,10 @@ class _ActivityCard extends StatelessWidget { @override Widget build(BuildContext context) { final count = items.length; - return ClideHolderCard( + return ClideCollapserCard( + label: 'Activity', collapsedSummary: _summarizeActivity(items.last), - stepLabel: count == 1 ? '1 step' : '$count steps', + counter: count == 1 ? '1 step' : '$count steps', status: _runStatus(items, resultByToolUseId), children: [ for (final item in items) @@ -774,7 +774,7 @@ ClideRunStatus? _runStatus(List items, Map children; - - /// Title shown in the expanded header (default `Activity`). - final String title; - - final bool initiallyExpanded; - - /// Optional aggregate run status (spinner / check / cross) shown at the head - /// of the ticker + header — the run's live state (T-296). Null shows nothing. - final ClideRunStatus? status; - - @override - State createState() => _ClideHolderCardState(); -} - -class _ClideHolderCardState extends State { - late bool _expanded = widget.initiallyExpanded; - final FocusNode _controlFocus = FocusNode(debugLabel: 'holder-control'); - - @override - void dispose() { - _controlFocus.dispose(); - super.dispose(); - } - - void _toggle() => setState(() => _expanded = !_expanded); - - @override - Widget build(BuildContext context) { - final tokens = ClideTheme.of(context).surface; - return Semantics( - button: true, - expanded: _expanded, - label: '${widget.title}, ${widget.stepLabel}, ${_expanded ? 'expanded' : 'collapsed'}', - excludeSemantics: true, - child: Padding( - // Match ConversationCard's inter-card margin (bottom 14, no top) so a - // folded activity / agent-run card sits in the same rhythm as the prose - // cards around it — not crammed 3px below the next one (T-282). - padding: const EdgeInsets.only(bottom: 14), - child: _expanded ? _expandedFrame(tokens) : _tickerRow(tokens), - ), - ); - } - - /// Collapsed: the ticker row IS the toggle, focusable for keyboard/AT. - Widget _tickerRow(SurfaceTokens tokens) => ClideTappable( - focusNode: _controlFocus, - onTap: _toggle, - tooltip: 'Expand', - builder: (context, hovered, focused) => Container( - padding: const EdgeInsets.symmetric(horizontal: 10, vertical: 6), - decoration: BoxDecoration( - color: (hovered || focused) ? tokens.listItemHoverBackground : tokens.listItemBackground, - border: Border.all(color: tokens.panelBorder), - borderRadius: BorderRadius.circular(4), - ), - child: Row( - children: [ - ClideIcon(const ChevronRightIcon(), size: 12, color: tokens.globalTextMuted), - const SizedBox(width: 8), - Expanded( - child: ClideText( - widget.collapsedSummary, - fontSize: clideFontCaption, - fontFamily: clideMonoFamily, - color: tokens.globalTextMuted, - maxLines: 1, - overflow: TextOverflow.ellipsis, - ), - ), - const SizedBox(width: 8), - if (widget.status != null) ...[ - ClideStatusIndicator(status: widget.status!, size: 12), - const SizedBox(width: 8), - ], - ClideText(widget.stepLabel, fontSize: clideFontCaption, color: tokens.globalTextMuted), - ], - ), - ), - ); - - /// Expanded: a framed container wrapping the sub-cards. The frame BACKGROUND - /// is a gesture target behind the children that only fires for hits the - /// children don't consume. - Widget _expandedFrame(SurfaceTokens tokens) => DecoratedBox( - decoration: BoxDecoration( - border: Border.all(color: tokens.panelBorder), - borderRadius: BorderRadius.circular(4), - ), - child: Stack( - children: [ - // Background toggle: behind the children, not a whole-card overlay, - // so child taps are never intercepted. Excluded from focus traversal - // — the header caret is the single keyboard stop. - Positioned.fill( - child: ExcludeFocus( - child: ClideTappable( - onTap: _toggle, - tooltip: 'Collapse', - builder: (_, __, ___) => const SizedBox.expand(), - ), - ), - ), - Column( - crossAxisAlignment: CrossAxisAlignment.stretch, - children: [ - _headerRow(tokens), - Padding( - padding: const EdgeInsets.fromLTRB(10, 0, 10, 8), - child: Column( - crossAxisAlignment: CrossAxisAlignment.stretch, - children: [ - for (final child in widget.children) - // Each child opaquely consumes its full bounds so a body - // tap interacts with the card (or does nothing), never - // the holder background. Deeper controls (caret/copy) - // still win; only taps are absorbed, so selection drags - // pass through to the SelectionArea. - GestureDetector( - behavior: HitTestBehavior.opaque, - onTap: () {}, - child: child, - ), - ], - ), - ), - ], - ), - ], - ), - ); - - /// The explicit, focusable collapse control. A background tap is not - /// keyboard/AT reachable on its own, so this keeps the control on the Tab - /// path and Enter/Space-activatable (D-78). - Widget _headerRow(SurfaceTokens tokens) => ClideTappable( - focusNode: _controlFocus, - onTap: _toggle, - tooltip: 'Collapse', - builder: (context, hovered, focused) => Container( - padding: const EdgeInsets.symmetric(horizontal: 10, vertical: 6), - decoration: BoxDecoration( - color: (hovered || focused) ? tokens.listItemHoverBackground : tokens.listItemBackground, - border: Border(bottom: BorderSide(color: tokens.panelBorder)), - ), - child: Row( - children: [ - ClideIcon(const ChevronDownIcon(), size: 12, color: tokens.globalTextMuted), - const SizedBox(width: 8), - Expanded( - child: ClideText(widget.title, fontSize: clideFontCaption, fontFamily: clideMonoFamily, color: tokens.globalTextMuted), - ), - const SizedBox(width: 8), - if (widget.status != null) ...[ - ClideStatusIndicator(status: widget.status!, size: 12), - const SizedBox(width: 8), - ], - ClideText(widget.stepLabel, fontSize: clideFontCaption, color: tokens.globalTextMuted), - ], - ), - ), - ); -} diff --git a/test/builtin/claude/holder_card_test.dart b/test/builtin/claude/holder_card_test.dart deleted file mode 100644 index 0f3aa11c..00000000 --- a/test/builtin/claude/holder_card_test.dart +++ /dev/null @@ -1,138 +0,0 @@ -/// Widget tests for the shared [ClideHolderCard] container primitive (T-266): -/// collapsed ticker, background-as-toggle, child taps not hijacked, the copy -/// control still works, and the keyboard/AT collapse path. -library; - -import 'package:clide/builtin/claude/src/conversation_card.dart'; -import 'package:clide/builtin/claude/src/holder_card.dart'; -import 'package:flutter/gestures.dart'; -import 'package:flutter/services.dart'; -import 'package:flutter/widgets.dart'; -import 'package:flutter_test/flutter_test.dart'; - -import '../../helpers/kernel_fixture.dart'; -import '../../helpers/widget_harness.dart'; - -void main() { - late KernelFixture f; - setUp(() async { - f = await KernelFixture.create(); - TestWidgetsFlutterBinding.ensureInitialized().defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, (_) async => null); - }); - tearDown(() { - TestWidgetsFlutterBinding.ensureInitialized().defaultBinaryMessenger.setMockMethodCallHandler(SystemChannels.platform, null); - f.dispose(); - }); - - // A tight, positioned tree so the holder's Row/Expanded get a bounded width - // and a deterministic on-screen rect for coordinate hit-testing. - Future pump(WidgetTester tester, {List? children, String summary = 'latest step', bool expanded = false}) async { - await tester.pumpWidget(harness( - f, - Align( - alignment: Alignment.topLeft, - child: SizedBox( - width: 400, - child: ClideHolderCard( - collapsedSummary: summary, - stepLabel: '2 steps', - initiallyExpanded: expanded, - children: children ?? - const [ - ConversationCard( - variant: ConversationCardVariant.bordered, - accent: Color(0xFFFFFFFF), - label: 'step', - body: Text('child body', textDirection: TextDirection.ltr), - ), - ], - ), - ), - ), - )); - await tester.pump(); - } - - testWidgets('collapsed shows the ticker summary + step count; tap expands', (tester) async { - await pump(tester); - expect(find.text('latest step'), findsOneWidget); - expect(find.text('2 steps'), findsOneWidget); - expect(find.bySemanticsLabel('Activity, 2 steps, collapsed'), findsOneWidget); - expect(find.text('child body'), findsNothing); // children hidden while collapsed - - await tester.tap(find.bySemanticsLabel('Activity, 2 steps, collapsed')); - await tester.pump(); - expect(find.bySemanticsLabel('Activity, 2 steps, expanded'), findsOneWidget); - expect(find.text('child body'), findsOneWidget); - }); - - testWidgets('a tap on the holder BACKGROUND (gutter) collapses it', (tester) async { - await pump(tester, expanded: true); - expect(find.text('child body'), findsOneWidget); // expanded - - // The left gutter (x = left+4) is holder background — children start at - // left+10 — and below the header, so this hits the background toggle. - final r = tester.getRect(find.byType(ClideHolderCard)); - await tester.tapAt(Offset(r.left + 4, r.center.dy)); - await tester.pump(); - expect(find.text('child body'), findsNothing); // collapsed - expect(find.bySemanticsLabel('Activity, 2 steps, collapsed'), findsOneWidget); - }); - - testWidgets('a tap on a child sub-card does NOT toggle the holder', (tester) async { - await pump(tester, expanded: true); - expect(find.text('child body'), findsOneWidget); - - // Tapping the child's body must be absorbed by the child, not bubble to the - // holder background toggle. - await tester.tap(find.text('child body')); - await tester.pump(); - expect(find.text('child body'), findsOneWidget); // still expanded - expect(find.bySemanticsLabel('Activity, 2 steps, expanded'), findsOneWidget); - }); - - testWidgets('a child copy button still copies — not swallowed by the holder', (tester) async { - await pump(tester, expanded: true, children: const [ - ConversationCard( - variant: ConversationCardVariant.bordered, - accent: Color(0xFFFFFFFF), - label: 'step', - copyText: 'copied from a held card', - body: Text('child body', textDirection: TextDirection.ltr), - ), - ]); - - // Hover the child to reveal its copy action, then tap it. - final g = await tester.createGesture(kind: PointerDeviceKind.mouse); - await g.addPointer(location: Offset.zero); - addTearDown(() => g.removePointer()); - await g.moveTo(tester.getCenter(find.byType(ConversationCard))); - await tester.pump(); - - await tester.tap(find.text('copy')); - await tester.runAsync(() => Future.delayed(const Duration(milliseconds: 20))); - expect(f.services.clipboard.readAs(), 'copied from a held card'); - // And the holder stayed expanded (the copy tap wasn't a background toggle). - expect(find.bySemanticsLabel('Activity, 2 steps, expanded'), findsOneWidget); - - // The hover starts a ClideTooltip show-delay (a Future.delayed that exit - // can't cancel). Move off and advance past the delay so it fires and leaves - // no pending timer at teardown. - await g.moveTo(const Offset(-100, -100)); - await tester.pump(const Duration(milliseconds: 600)); - }); - - testWidgets('the explicit control is keyboard-focusable and toggles on Activate (a11y)', (tester) async { - await pump(tester); // collapsed; the ticker row is the control - final focusWidget = tester.widget( - find.ancestor(of: find.text('latest step'), matching: find.byType(Focus)).first, - ); - focusWidget.focusNode!.requestFocus(); - await tester.pump(); - expect(focusWidget.focusNode!.hasFocus, isTrue); - - Actions.invoke(focusWidget.focusNode!.context!, const ActivateIntent()); - await tester.pump(); - expect(find.bySemanticsLabel('Activity, 2 steps, expanded'), findsOneWidget); - }); -} diff --git a/test/goldens/goldens/linux/holder_card.png b/test/goldens/goldens/linux/holder_card.png deleted file mode 100644 index 155dbb87..00000000 Binary files a/test/goldens/goldens/linux/holder_card.png and /dev/null differ diff --git a/test/goldens/holder_card_goldens_test.dart b/test/goldens/holder_card_goldens_test.dart deleted file mode 100644 index eb7646dc..00000000 --- a/test/goldens/holder_card_goldens_test.dart +++ /dev/null @@ -1,66 +0,0 @@ -import 'package:alchemist/alchemist.dart'; -import 'package:clide/builtin/claude/src/conversation_card.dart'; -import 'package:clide/builtin/claude/src/holder_card.dart'; -import 'package:flutter/widgets.dart'; -import 'package:flutter_test/flutter_test.dart'; - -import '../helpers/kernel_fixture.dart'; -import '../helpers/widget_harness.dart'; - -void main() { - late KernelFixture f; - - setUp(() async => f = await KernelFixture.create()); - tearDown(() async => f.dispose()); - - goldenTest( - 'ClideHolderCard (T-266): collapsed ticker + expanded container of sub-cards', - fileName: 'holder_card', - builder: () => GoldenTestGroup( - columns: 1, - children: [ - GoldenTestScenario( - name: 'collapsed (ticker + step count)', - child: _wrap( - f, - const ClideHolderCard( - collapsedSummary: 'Bash ls -la', - stepLabel: '3 steps', - children: [], - ), - ), - ), - GoldenTestScenario( - name: 'expanded (framed container wrapping sub-cards)', - child: _wrap( - f, - const ClideHolderCard( - collapsedSummary: 'Bash ls -la', - stepLabel: '3 steps', - initiallyExpanded: true, - children: [ - ConversationCard( - variant: ConversationCardVariant.bordered, - accent: Color(0xFF4C9AFF), - label: 'Bash', - body: Text('ls -la', textDirection: TextDirection.ltr), - ), - ConversationCard( - variant: ConversationCardVariant.bordered, - accent: Color(0xFF4C9AFF), - label: 'Read', - body: Text('/lib/main.dart', textDirection: TextDirection.ltr), - ), - ], - ), - ), - ), - ], - ), - ); -} - -Widget _wrap(KernelFixture f, Widget child) => SizedBox( - width: 360, - child: harness(f, child), - ); diff --git a/test/widgets/src/clide_collapser_card_test.dart b/test/widgets/src/clide_collapser_card_test.dart index 48a3df19..cd62cf80 100644 --- a/test/widgets/src/clide_collapser_card_test.dart +++ b/test/widgets/src/clide_collapser_card_test.dart @@ -96,6 +96,23 @@ void main() { expect(find.bySemanticsLabel('Edits, 3 edits, expanded'), findsOneWidget); }); + testWidgets('a deeper interactive control inside an item still fires (not swallowed)', (tester) async { + var tapped = false; + await pump(tester, expanded: true, children: [ + ClideTappable( + onTap: () => tapped = true, + builder: (_, __, ___) => const Padding( + padding: EdgeInsets.all(8), + child: Text('press me', textDirection: TextDirection.ltr), + ), + ), + ]); + await tester.tap(find.text('press me')); + await tester.pump(); + expect(tapped, isTrue); // the item's own control won the hit + expect(find.bySemanticsLabel('Edits, 3 edits, expanded'), findsOneWidget); // collapser unchanged + }); + testWidgets('the ticker is keyboard-focusable and toggles on Activate (a11y)', (tester) async { await pump(tester); final focusWidget = tester.widget(