From b90cc6e88c43e14d2480cd3e2cc4bcf65cfa174e Mon Sep 17 00:00:00 2001 From: diffwireauto <330573775+diffwireauto@users.noreply.github.com> Date: Mon, 28 Sep 2026 13:09:34 -0400 Subject: [PATCH] fix(#684): apply pre-PR review, never show the notice while disconnected Gemini gate (2.5-pro) findings, verified against source before acting. ACCEPTED, real defect: the notice would have appeared on every disconnect. `_activeTransport` defaults to bluetooth and is reset to it on teardown, the device is null while disconnected so `mtuNow` is unavailable, and `effectiveMaxFrameSize` therefore returns the same 20-byte floor an unreported MTU produces. Neither chat screen pops on disconnect, so the composer stays built. The banner would have blamed the link for a small packet size when there was no link. Now gated on `isConnected`. ACCEPTED in substance: a permanent error-red banner over a link that legitimately fits only a few characters is noise, and noise is what teaches people to ignore the banner that matters. Kept the 16-byte threshold, since lowering it would let the misleading 4-byte case through, and split the severity instead: 0 bytes is an error, 1-15 is informational. REJECTED, unfounded: a semanticLabel on the icon. Flutter's Icon wraps its glyph in ExcludeSemantics and only announces a label when one is given (icon.dart:348-356), so an unlabelled icon is already decorative, and the two Text children carry the whole message. Comment added so it is not "fixed" later. analyze clean, format clean, 962 tests pass. Co-Authored-By: Claude Opus 5 --- lib/screens/channel_chat_screen.dart | 1 + lib/screens/chat_screen.dart | 1 + lib/widgets/composer_budget_notice.dart | 52 ++++++++++++++++++------- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/lib/screens/channel_chat_screen.dart b/lib/screens/channel_chat_screen.dart index 501ac775..3ccdefd8 100644 --- a/lib/screens/channel_chat_screen.dart +++ b/lib/screens/channel_chat_screen.dart @@ -1435,6 +1435,7 @@ class _ChannelChatScreenState extends State { ComposerBudgetNotice( maxBytes: maxBytes, transport: connector.activeTransport, + isConnected: connector.isConnected, ), if (_replyingToMessage != null) Builder( diff --git a/lib/screens/chat_screen.dart b/lib/screens/chat_screen.dart index b9e93599..0718a3e7 100644 --- a/lib/screens/chat_screen.dart +++ b/lib/screens/chat_screen.dart @@ -609,6 +609,7 @@ class _ChatScreenState extends State { ComposerBudgetNotice( maxBytes: maxBytes, transport: connector.activeTransport, + isConnected: connector.isConnected, ), _buildComposerBar(context, colorScheme, settings, maxBytes, connector), ], diff --git a/lib/widgets/composer_budget_notice.dart b/lib/widgets/composer_budget_notice.dart index ac8403bd..84121a0d 100644 --- a/lib/widgets/composer_budget_notice.dart +++ b/lib/widgets/composer_budget_notice.dart @@ -28,23 +28,39 @@ class ComposerBudgetNotice extends StatelessWidget { /// as generic failure. final MeshCoreTransportType transport; + /// Whether a radio is actually connected. + /// + /// Required, and not cosmetic. While disconnected, `_activeTransport` still + /// reads `bluetooth` (its default) and there is no device, so the frame + /// budget falls to the same 20-byte floor an unreported MTU produces. Without + /// this gate the notice would appear on every disconnect and blame the link + /// for a small packet size when there is no link at all. + final bool isConnected; + const ComposerBudgetNotice({ super.key, required this.maxBytes, required this.transport, + required this.isConnected, }); @override Widget build(BuildContext context) { + if (!isConnected) return const SizedBox.shrink(); if (isComposerBudgetUsable(maxBytes)) return const SizedBox.shrink(); final l10n = context.l10n; final scheme = Theme.of(context).colorScheme; // A budget of 0 blocks every keystroke; 1-15 accepts input but cannot carry - // a sentence. The user needs those described differently: the first is - // "you cannot send", the second is "you have almost no room". - final title = maxBytes <= 0 + // a sentence. Those are different severities and must not look the same. + // Zero is a failure: nothing can be sent. Non-zero is a fact about a + // working link, so it is stated rather than alarmed about - a permanent red + // banner over a link that genuinely only fits a few characters is noise, + // and noise is what teaches people to ignore the banner that matters. + final blocked = maxBytes <= 0; + + final title = blocked ? l10n.composerBudgetBlockedTitle : l10n.composerBudgetLimitedTitle(maxBytes); @@ -52,14 +68,30 @@ class ComposerBudgetNotice extends StatelessWidget { ? l10n.composerBudgetBluetoothCause : l10n.composerBudgetGenericCause; + final background = blocked + ? scheme.errorContainer + : scheme.secondaryContainer; + final foreground = blocked + ? scheme.onErrorContainer + : scheme.onSecondaryContainer; + return Container( width: double.infinity, padding: const EdgeInsets.symmetric(horizontal: 16, vertical: 10), - color: scheme.errorContainer, + color: background, child: Row( crossAxisAlignment: CrossAxisAlignment.start, children: [ - Icon(Icons.error_outline, size: 18, color: scheme.onErrorContainer), + // No semanticLabel on purpose: Icon wraps its glyph in + // ExcludeSemantics and only announces a label when given one, so an + // unlabelled icon is correctly decorative here. The two Text children + // below already carry the whole message to a screen reader, and + // labelling the icon too would just repeat it. + Icon( + blocked ? Icons.error_outline : Icons.info_outline, + size: 18, + color: foreground, + ), const SizedBox(width: 10), Expanded( child: Column( @@ -69,17 +101,11 @@ class ComposerBudgetNotice extends StatelessWidget { title, style: TextStyle( fontWeight: FontWeight.w600, - color: scheme.onErrorContainer, + color: foreground, ), ), const SizedBox(height: 2), - Text( - cause, - style: TextStyle( - fontSize: 12, - color: scheme.onErrorContainer, - ), - ), + Text(cause, style: TextStyle(fontSize: 12, color: foreground)), ], ), ),