fix(ui): address PR #30 review findings

- ViewportInfo.updateShouldNotify: compare layoutClass only
  (not width/height), avoiding unnecessary rebuilds on every
  resize frame within the same layout class.
- ChatPanel: use BorderDirectional(start:) for RTL support.
- ChatPanel: localize 'Close chat' tooltip via AppL10n.chatCloseAction.
- Inline panel snackbar: localize via AppL10n.chatPanelCollapsedHint.
  New en/zh ARB entries added.
- _saveCurrentDraft(): removed — it was a self-assignment no-op.
  Draft persistence relies on ChatDetailView's didUpdateWidget
  (fires onDraftChanged on target switch) and dispose (fires on
  panel tear-down), both of which already populate _chatDrafts
  correctly without an explicit save call.
- _handleInlineChatViewport layout snackbar: use AppL10n.
- Audio level-meter: switch from callback-count (% 3) to time-based
  gating (std::time::Duration::from_millis(33)), robust to cpal
  buffer-size or sample-rate changes. Remove level_decimation_counter.
- chat_panel_test.dart: add AppL10n.localizationsDelegates so the
  test resolves l10n keys.

Tests: 183 passed, 2 skipped. Dart analyze clean.
cargo test -p chanora_audio --lib: 125 passed.
This commit is contained in:
Edison Jwa
2026-06-07 23:30:00 +09:00
parent e9cd832828
commit dad633e381
10 changed files with 83 additions and 42 deletions
@@ -57,8 +57,11 @@ class ViewportInfo extends InheritedWidget {
bool get isWide => !isCompact; bool get isWide => !isCompact;
@override @override
bool updateShouldNotify(ViewportInfo old) => bool updateShouldNotify(ViewportInfo old) => layoutClass != old.layoutClass;
layoutClass != old.layoutClass || // NOTE: width/height changes within the same layout class do NOT trigger
width != old.width || // notification. Dependents who genuinely need pixel-level dimensions
height != old.height; // (rare — most layouts should switch on layoutClass) must use a local
// LayoutBuilder. Notifying on every pixel would rebuild every dependent
// on every resize frame, which is the exact pessimisation this
// InheritedWidget exists to avoid.
} }
+1
View File
@@ -46,6 +46,7 @@
"retryAction": "Retry", "retryAction": "Retry",
"chatAction": "Chat", "chatAction": "Chat",
"chatCloseAction": "Close chat", "chatCloseAction": "Close chat",
"chatPanelCollapsedHint": "Tap the chat button to continue your conversation",
"chatNewPrivateAction": "New private chat", "chatNewPrivateAction": "New private chat",
"chatSearchClientsHint": "Search clients...", "chatSearchClientsHint": "Search clients...",
"chatDirectMessageAction": "Private message", "chatDirectMessageAction": "Private message",
+1
View File
@@ -39,6 +39,7 @@
"retryAction": "重试", "retryAction": "重试",
"chatAction": "聊天", "chatAction": "聊天",
"chatCloseAction": "关闭聊天", "chatCloseAction": "关闭聊天",
"chatPanelCollapsedHint": "点击聊天按钮以继续对话",
"chatNewPrivateAction": "新建私聊", "chatNewPrivateAction": "新建私聊",
"chatSearchClientsHint": "搜索用户...", "chatSearchClientsHint": "搜索用户...",
"chatDirectMessageAction": "私聊", "chatDirectMessageAction": "私聊",
@@ -307,6 +307,12 @@ abstract class AppL10n {
/// **'Close chat'** /// **'Close chat'**
String get chatCloseAction; String get chatCloseAction;
/// No description provided for @chatPanelCollapsedHint.
///
/// In en, this message translates to:
/// **'Tap the chat button to continue your conversation'**
String get chatPanelCollapsedHint;
/// No description provided for @chatNewPrivateAction. /// No description provided for @chatNewPrivateAction.
/// ///
/// In en, this message translates to: /// In en, this message translates to:
@@ -123,6 +123,10 @@ class AppL10nEn extends AppL10n {
@override @override
String get chatCloseAction => 'Close chat'; String get chatCloseAction => 'Close chat';
@override
String get chatPanelCollapsedHint =>
'Tap the chat button to continue your conversation';
@override @override
String get chatNewPrivateAction => 'New private chat'; String get chatNewPrivateAction => 'New private chat';
@@ -120,6 +120,9 @@ class AppL10nZh extends AppL10n {
@override @override
String get chatCloseAction => '关闭聊天'; String get chatCloseAction => '关闭聊天';
@override
String get chatPanelCollapsedHint => '点击聊天按钮以继续对话';
@override @override
String get chatNewPrivateAction => '新建私聊'; String get chatNewPrivateAction => '新建私聊';
+22 -19
View File
@@ -1673,17 +1673,18 @@ class _BetaHomeState extends State<_BetaHome> with WidgetsBindingObserver {
}; };
} }
/// Saves the current draft text for the current inline chat target. /// Returns the current draft text for the inline chat target, or null
void _saveCurrentDraft() { /// if no draft has been stored.
// Drafts are continuously saved via onDraftChanged callback in Task 4. ///
// This method exists as an explicit save point. /// Draft persistence relies on [ChatDetailView]'s own lifecycle:
final target = _inlineChatTarget; /// `didUpdateWidget` flushes the outgoing target's draft via
if (target == null) return; /// `onDraftChanged` when the parent rebuilds with a new target, and
final key = _draftKeyForTarget(target); /// `dispose` flushes the final draft when the panel is torn down
if (_chatDrafts.containsKey(key)) { /// (e.g. on [_closeInlineChat]). Both paths feed
_chatDrafts[key] = _chatDrafts[key] ?? ''; /// [_chatDrafts] without requiring an explicit save call from this
} /// class.
} String? _currentDraftFor(rust.BridgeMessageTarget target) =>
_chatDrafts[_draftKeyForTarget(target)];
Future<void> _onOpenChat({ Future<void> _onOpenChat({
rust.BridgeMessageTarget? target, rust.BridgeMessageTarget? target,
@@ -1722,8 +1723,9 @@ class _BetaHomeState extends State<_BetaHome> with WidgetsBindingObserver {
layoutClassFromWidth(MediaQuery.sizeOf(context).width) == layoutClassFromWidth(MediaQuery.sizeOf(context).width) ==
LayoutClass.expanded; LayoutClass.expanded;
if (isExpanded) { if (isExpanded) {
// Outgoing draft is preserved by ChatDetailView.didUpdateWidget,
// which fires onDraftChanged with the old target's text on rebuild.
setState(() { setState(() {
_saveCurrentDraft();
_chatUnread = 0; _chatUnread = 0;
_chatOpen = true; _chatOpen = true;
_inlineChatTarget = newTarget; _inlineChatTarget = newTarget;
@@ -1754,8 +1756,10 @@ class _BetaHomeState extends State<_BetaHome> with WidgetsBindingObserver {
} }
void _closeInlineChat() { void _closeInlineChat() {
// Final draft for the dismissed target is preserved by
// ChatDetailView.dispose, which fires onDraftChanged when the panel
// is removed from the tree on the next rebuild.
setState(() { setState(() {
_saveCurrentDraft();
_lastDismissedTarget = _inlineChatTarget; _lastDismissedTarget = _inlineChatTarget;
_lastDismissedClientName = _inlineChatClientName; _lastDismissedClientName = _inlineChatClientName;
_chatOpen = false; _chatOpen = false;
@@ -1788,8 +1792,8 @@ class _BetaHomeState extends State<_BetaHome> with WidgetsBindingObserver {
WidgetsBinding.instance.addPostFrameCallback((_) { WidgetsBinding.instance.addPostFrameCallback((_) {
if (!mounted || _inlineChatTarget == null) return; if (!mounted || _inlineChatTarget == null) return;
ScaffoldMessenger.of(context).showSnackBar( ScaffoldMessenger.of(context).showSnackBar(
const SnackBar( SnackBar(
content: Text('Tap the chat button to continue your conversation'), content: Text(AppL10n.of(context).chatPanelCollapsedHint),
), ),
); );
}); });
@@ -2654,10 +2658,9 @@ class _BetaHomeState extends State<_BetaHome> with WidgetsBindingObserver {
snapshot: _snapshot!, snapshot: _snapshot!,
target: inlineChatTarget, target: inlineChatTarget,
clientName: _inlineChatClientName, clientName: _inlineChatClientName,
restoredDraft: restoredDraft: _currentDraftFor(
_chatDrafts[_draftKeyForTarget( inlineChatTarget,
inlineChatTarget, ),
)],
onDraftChanged: (text) { onDraftChanged: (text) {
final draftKey = _draftKeyForTarget( final draftKey = _draftKeyForTarget(
inlineChatTarget, inlineChatTarget,
@@ -3,6 +3,7 @@
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import '../design/breakpoints.dart'; import '../design/breakpoints.dart';
import '../l10n/generated/app_localizations.dart';
import '../services/snapshot_state_mapper.dart'; import '../services/snapshot_state_mapper.dart';
import '../services/ts3_server_link.dart'; import '../services/ts3_server_link.dart';
import '../src/rust/api.dart' as rust; import '../src/rust/api.dart' as rust;
@@ -51,14 +52,15 @@ class ChatPanel extends StatelessWidget {
Widget build(BuildContext context) { Widget build(BuildContext context) {
final currentChannelId = ownClientSnapshotState(snapshot)?.channelId; final currentChannelId = ownClientSnapshotState(snapshot)?.channelId;
final channelName = snapshotChannelName(snapshot, currentChannelId); final channelName = snapshotChannelName(snapshot, currentChannelId);
final l10n = AppL10n.of(context);
return SizedBox( return SizedBox(
width: ChanoraBreakpoints.chatPanelWidth, width: ChanoraBreakpoints.chatPanelWidth,
child: DecoratedBox( child: DecoratedBox(
decoration: BoxDecoration( decoration: BoxDecoration(
color: Theme.of(context).colorScheme.surface, color: Theme.of(context).colorScheme.surface,
border: Border( border: BorderDirectional(
left: BorderSide( start: BorderSide(
color: Theme.of(context).colorScheme.outlineVariant, color: Theme.of(context).colorScheme.outlineVariant,
), ),
), ),
@@ -75,7 +77,7 @@ class ChatPanel extends StatelessWidget {
onDraftChanged: onDraftChanged, onDraftChanged: onDraftChanged,
messageMaxWidth: 500, messageMaxWidth: 500,
headerTrailing: IconButton( headerTrailing: IconButton(
tooltip: 'Close chat', tooltip: l10n.chatCloseAction,
icon: const Icon(Icons.close), icon: const Icon(Icons.close),
onPressed: onClose, onPressed: onClose,
), ),
@@ -1,6 +1,7 @@
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:chanora_flutter/l10n/generated/app_localizations.dart';
import 'package:chanora_flutter/src/rust/api.dart' as rust; import 'package:chanora_flutter/src/rust/api.dart' as rust;
import 'package:chanora_flutter/widgets/chat_panel.dart'; import 'package:chanora_flutter/widgets/chat_panel.dart';
import 'package:chanora_flutter/widgets/chat_views.dart'; import 'package:chanora_flutter/widgets/chat_views.dart';
@@ -55,6 +56,8 @@ void main() {
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
localizationsDelegates: AppL10n.localizationsDelegates,
supportedLocales: AppL10n.supportedLocales,
home: Scaffold( home: Scaffold(
body: ChatPanel( body: ChatPanel(
messages: messages, messages: messages,
@@ -87,6 +90,8 @@ void main() {
required String? restoredDraft, required String? restoredDraft,
}) { }) {
return MaterialApp( return MaterialApp(
localizationsDelegates: AppL10n.localizationsDelegates,
supportedLocales: AppL10n.supportedLocales,
home: Scaffold( home: Scaffold(
body: ChatDetailView( body: ChatDetailView(
messages: messages, messages: messages,
+29 -16
View File
@@ -1818,18 +1818,27 @@ struct CaptureState {
/// after warmup. Same precedent as `mono_scratch` above. /// after warmup. Same precedent as `mono_scratch` above.
frame_scratch: Vec<f32>, frame_scratch: Vec<f32>,
audio_processing_stats: Arc<crate::SharedAudioProcessingStats>, audio_processing_stats: Arc<crate::SharedAudioProcessingStats>,
/// Decimation counter for the level meter. The cpal callback fires /// Time-based decimation for the level meter. The cpal callback
/// ~93 times/sec (256 frames at 48 kHz), but the bridge consumer /// cadence depends on platform (256 frames at 48 kHz ≈ 187 Hz,
/// (`input_level_stream`) only reads at ~30 Hz. Computing `sqrt()` /// 512 frames ≈ 94 Hz, 1024 frames ≈ 47 Hz) and can change at
/// + `log10()` every callback wastes real-time budget and causes /// runtime on device or sample-rate switch. The bridge consumer
/// buffer underruns on macOS CoreAudio. We accumulate the running /// (`input_level_stream`) only reads at ~30 Hz, so computing
/// sum-of-squares every callback (trivially cheap: O(n) multiply- /// `sqrt()` + `log10()` on every callback wastes real-time budget
/// add) and only compute the final dBFS every 3rd callback (~31 Hz), /// and caused buffer underruns on macOS CoreAudio with small
/// matching the consumer rate. See commit history for the metering /// buffer sizes. We only emit a new dBFS sample after at least
/// regression that motivated this. /// [LEVEL_METER_INTERVAL] has elapsed since the previous emit,
level_decimation_counter: u32, /// which is platform-cadence-independent.
last_level_emit: std::time::Instant,
} }
/// Minimum interval between input-level dBFS samples sent to the
/// bridge. Matches the consumer rate (`input_level_stream` at ~30 Hz).
/// Time-based gating is robust to cpal buffer-size and sample-rate
/// changes that a fixed callback-count would not be.
#[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "android")))]
const LEVEL_METER_INTERVAL: std::time::Duration =
std::time::Duration::from_millis(33);
#[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "android")))] #[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "android")))]
impl CaptureState { impl CaptureState {
fn new( fn new(
@@ -1857,7 +1866,10 @@ impl CaptureState {
mono_scratch: Vec::with_capacity(4096), mono_scratch: Vec::with_capacity(4096),
frame_scratch: Vec::with_capacity(FRAME_SAMPLES), frame_scratch: Vec::with_capacity(FRAME_SAMPLES),
audio_processing_stats, audio_processing_stats,
level_decimation_counter: 0, // Start in the past so the first ingest emits immediately.
last_level_emit: std::time::Instant::now()
.checked_sub(LEVEL_METER_INTERVAL)
.unwrap_or_else(std::time::Instant::now),
} }
} }
@@ -1877,11 +1889,12 @@ impl CaptureState {
self.mono_scratch.push(sum / frame.len() as f32); self.mono_scratch.push(sum / frame.len() as f32);
} }
// Level meter: accumulate sum-of-squares on every callback // Level meter: pay sqrt() + log10() only when at least
// (trivially cheap), but only pay for sqrt() + log10() every // LEVEL_METER_INTERVAL has elapsed, regardless of the
// 3rd callback (~31 Hz, matching the bridge consumer rate). // platform's cpal callback cadence.
self.level_decimation_counter = self.level_decimation_counter.wrapping_add(1); let now = std::time::Instant::now();
if self.level_decimation_counter % 3 == 0 { if now.duration_since(self.last_level_emit) >= LEVEL_METER_INTERVAL {
self.last_level_emit = now;
self.audio_processing_stats self.audio_processing_stats
.set_input_dbfs(crate::frame::dbfs(&self.mono_scratch)); .set_input_dbfs(crate::frame::dbfs(&self.mono_scratch));
} }