From e9c0cb8d59b6b0275d8e04f149a9354243d06b08 Mon Sep 17 00:00:00 2001 From: crackedhandle Date: Thu, 23 Apr 2026 11:23:01 +0530 Subject: [PATCH 1/2] feat: make kind chip clickable to filter logs by kind --- .../src/screens/logging/_message_column.dart | 11 ++- .../lib/src/screens/logging/metadata.dart | 97 +++++++++++-------- 2 files changed, 65 insertions(+), 43 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/logging/_message_column.dart b/packages/devtools_app/lib/src/screens/logging/_message_column.dart index 96a6c916e70..85d63dde39e 100644 --- a/packages/devtools_app/lib/src/screens/logging/_message_column.dart +++ b/packages/devtools_app/lib/src/screens/logging/_message_column.dart @@ -5,6 +5,7 @@ import 'package:devtools_app_shared/ui.dart'; import 'package:flutter/material.dart'; +import '../../shared/globals.dart'; import '../../shared/primitives/utils.dart'; import '../../shared/table/table.dart'; import '../../shared/table/table_data.dart'; @@ -54,7 +55,6 @@ class MessageColumn extends ColumnData // This needs to be a function because the details may be computed after the // initial build. bool hasDetails() => !data.details.isNullOrEmpty; - return FutureBuilder( future: data.detailsComputed.future, builder: (context, _) { @@ -92,7 +92,14 @@ class MessageColumn extends ColumnData padding: const EdgeInsets.symmetric(vertical: densePadding), child: LayoutBuilder( builder: (context, constraints) { - return MetadataChips(data: data); + return MetadataChips( + data: data, + onKindTapped: (kind) { + final controller = screenControllers + .lookup(); + controller.setActiveFilter(query: 'k:$kind'); + }, + ); }, ), ), diff --git a/packages/devtools_app/lib/src/screens/logging/metadata.dart b/packages/devtools_app/lib/src/screens/logging/metadata.dart index 9ac216e67e0..c881ba9ea46 100644 --- a/packages/devtools_app/lib/src/screens/logging/metadata.dart +++ b/packages/devtools_app/lib/src/screens/logging/metadata.dart @@ -14,9 +14,10 @@ import '../../shared/primitives/utils.dart'; import 'logging_controller.dart'; class MetadataChips extends StatelessWidget { - const MetadataChips({super.key, required this.data}); + const MetadataChips({super.key, required this.data, this.onKindTapped}); final LogData data; + final void Function(String kind)? onKindTapped; @override Widget build(BuildContext context) { @@ -91,6 +92,7 @@ class MetadataChips extends StatelessWidget { iconAsset: kindIcon.iconAsset, backgroundColor: kindColors.background, foregroundColor: kindColors.foreground, + onTap: onKindTapped != null ? () => onKindTapped!(data.kind) : null, ), logLevelChip, if (elapsedFrameTimeAsString != null) @@ -115,6 +117,7 @@ abstract class MetadataChip extends StatelessWidget { this.foregroundColor, this.outlined = false, this.includeLeadingMargin = true, + this.onTap, }); final IconData? icon; @@ -125,6 +128,7 @@ abstract class MetadataChip extends StatelessWidget { final Color? foregroundColor; final bool outlined; final bool includeLeadingMargin; + final VoidCallback? onTap; static const horizontalPadding = densePadding; static const verticalPadding = borderPadding; @@ -140,50 +144,60 @@ abstract class MetadataChip extends StatelessWidget { final foregroundColor = this.foregroundColor ?? theme.colorScheme.onSecondaryContainer; - return maybeWrapWithTooltip( - tooltip: tooltip, - child: Container( - decoration: BoxDecoration( - color: backgroundColor, - borderRadius: BorderRadius.circular(_borderRadius), - border: outlined - ? Border.all(color: theme.colorScheme.subtleTextColor) - : null, - ), - margin: includeLeadingMargin - ? const EdgeInsets.only(left: denseSpacing) + Widget chip = Container( + decoration: BoxDecoration( + color: backgroundColor, + borderRadius: BorderRadius.circular(_borderRadius), + border: outlined + ? Border.all(color: theme.colorScheme.subtleTextColor) : null, - padding: const EdgeInsets.symmetric( - horizontal: horizontalPadding, - vertical: verticalPadding, - ), - child: Row( - mainAxisSize: MainAxisSize.min, - children: [ - if (icon != null || iconAsset != null) ...[ - DevToolsIcon( - icon: icon, - iconAsset: iconAsset, - size: _metadataIconSize, - color: foregroundColor, - ), - const SizedBox(width: iconPadding), - ] else - // Include an empty SizedBox to ensure a consistent height for the - // chips, regardless of whether the chip includes an icon. - const SizedBox(height: _metadataIconSize), - RichText( - text: TextSpan( - text: text, - style: theme - .regularTextStyleWithColor(foregroundColor) - .copyWith(fontSize: smallFontSize), - ), + ), + margin: includeLeadingMargin + ? const EdgeInsets.only(left: denseSpacing) + : null, + padding: const EdgeInsets.symmetric( + horizontal: horizontalPadding, + vertical: verticalPadding, + ), + child: Row( + mainAxisSize: MainAxisSize.min, + children: [ + if (icon != null || iconAsset != null) ...[ + DevToolsIcon( + icon: icon, + iconAsset: iconAsset, + size: _metadataIconSize, + color: foregroundColor, ), - ], - ), + const SizedBox(width: iconPadding), + ] else + // Include an empty SizedBox to ensure a consistent height for the + // chips, regardless of whether the chip includes an icon. + const SizedBox(height: _metadataIconSize), + RichText( + text: TextSpan( + text: text, + style: theme + .regularTextStyleWithColor(foregroundColor) + .copyWith(fontSize: smallFontSize), + ), + ), + ], ), ); + + if (onTap != null) { + chip = MouseRegion( + cursor: SystemMouseCursors.click, + child: GestureDetector( + onTap: onTap, + behavior: HitTestBehavior.opaque, + child: chip, + ), + ); + } + + return maybeWrapWithTooltip(tooltip: tooltip, child: chip); } } @@ -195,6 +209,7 @@ class KindMetaDataChip extends MetadataChip { super.iconAsset, super.backgroundColor, super.foregroundColor, + super.onTap, }) : super(text: kind, includeLeadingMargin: false); static ({IconData? icon, String? iconAsset}) generateIcon(String kind) { From 2a69142f29a4b8ef15b865abac0c2fd162be14aa Mon Sep 17 00:00:00 2001 From: crackedhandle Date: Thu, 23 Apr 2026 15:44:40 +0530 Subject: [PATCH 2/2] fix: address review feedback on kind chip filter --- .../src/screens/logging/_message_column.dart | 8 +++--- .../lib/src/screens/logging/metadata.dart | 27 ++++++++++++------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/logging/_message_column.dart b/packages/devtools_app/lib/src/screens/logging/_message_column.dart index 85d63dde39e..dbbd45226a5 100644 --- a/packages/devtools_app/lib/src/screens/logging/_message_column.dart +++ b/packages/devtools_app/lib/src/screens/logging/_message_column.dart @@ -94,11 +94,9 @@ class MessageColumn extends ColumnData builder: (context, constraints) { return MetadataChips( data: data, - onKindTapped: (kind) { - final controller = screenControllers - .lookup(); - controller.setActiveFilter(query: 'k:$kind'); - }, + onKindTapped: (kind) => screenControllers + .lookup() + .setActiveFilter(query: 'k:"$kind"'), ); }, ), diff --git a/packages/devtools_app/lib/src/screens/logging/metadata.dart b/packages/devtools_app/lib/src/screens/logging/metadata.dart index c881ba9ea46..e965c532455 100644 --- a/packages/devtools_app/lib/src/screens/logging/metadata.dart +++ b/packages/devtools_app/lib/src/screens/logging/metadata.dart @@ -152,9 +152,6 @@ abstract class MetadataChip extends StatelessWidget { ? Border.all(color: theme.colorScheme.subtleTextColor) : null, ), - margin: includeLeadingMargin - ? const EdgeInsets.only(left: denseSpacing) - : null, padding: const EdgeInsets.symmetric( horizontal: horizontalPadding, vertical: verticalPadding, @@ -187,14 +184,26 @@ abstract class MetadataChip extends StatelessWidget { ); if (onTap != null) { - chip = MouseRegion( - cursor: SystemMouseCursors.click, - child: GestureDetector( - onTap: onTap, - behavior: HitTestBehavior.opaque, - child: chip, + chip = Padding( + padding: includeLeadingMargin + ? const EdgeInsets.only(left: denseSpacing) + : EdgeInsets.zero, + child: MouseRegion( + cursor: SystemMouseCursors.click, + child: GestureDetector( + onTap: onTap, + behavior: HitTestBehavior.opaque, + child: chip, + ), ), ); + } else { + chip = Padding( + padding: includeLeadingMargin + ? const EdgeInsets.only(left: denseSpacing) + : EdgeInsets.zero, + child: chip, + ); } return maybeWrapWithTooltip(tooltip: tooltip, child: chip);