feat(ui): integrate StreamSnackbar into composer hold-to-record#2739
feat(ui): integrate StreamSnackbar into composer hold-to-record#2739xsahil03x wants to merge 6 commits into
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughPins stream_core_flutter to a git commit, re-exports snackbar types, deprecates recorder message APIs, wraps the app with StreamSnackbarScope, refactors message-input voice-record UI to use snackbars, and adds controller and widget/golden tests for hold-to-record behavior. ChangesHold-to-Record Snackbar Integration
Sequence DiagramsequenceDiagram
participant User
participant StreamChatMessageInput
participant StreamAudioRecorderController
participant StreamSnackbarMessenger
participant StreamMessageComposer
participant StreamSnackbarPopup
User->>StreamChatMessageInput: Long-press microphone
StreamChatMessageInput->>StreamAudioRecorderController: onRecordStart()
User->>StreamChatMessageInput: Release (cancel)
StreamChatMessageInput->>StreamAudioRecorderController: onRecordStartCancel()
StreamChatMessageInput->>StreamSnackbarMessenger: show(holdLabel, replace: true)
StreamMessageComposer->>StreamSnackbarPopup: composer wrapped
StreamSnackbarPopup->>User: Display hold-to-record hint
StreamSnackbarPopup->>StreamAudioRecorderController: onDismiss -> hideInfo()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- `StreamChatMessageInput`'s `onLongPressCancel` now surfaces the hold-to-record hint via `StreamSnackbarMessenger.maybeOf(context).show( snackbar, replace: true)`. `onLongPressStart` clears any in-flight hint via `removeCurrent()` so the most recent gesture wins. - `StreamMessageComposer` wraps its output in `StreamSnackbarPopup` at the factory-dispatch level, giving every composer variant (including custom builders via `chatComponentBuilder<MessageComposerProps>`) a stable snackbar surface anchored above the composer. - `StreamChat` wraps its child in `StreamSnackbarScope` so any subtree without a nearer `StreamSnackbarPopup` falls back to an app-wide surface. - Deprecate `StreamAudioRecorderController.showInfo` and `RecordStateIdle.message` — the composer no longer reads either; external consumers should fire snackbars directly. - Re-export the snackbar API (`StreamSnackbar`, `StreamSnackbarMessenger`, `StreamSnackbarPopup`, `StreamSnackbarPopupPlacement`, `StreamSnackbarScope`, etc.) from `stream_chat_flutter.dart`. - Pin `stream_core_flutter` to the PR #118 commit until release. Tests: 6 widget tests in `message_input_test.dart` covering the gesture wiring, dedupe via `replace`, clear-on-hold, and the global-scope fallback. One golden test for the composer + snackbar geometry.
8ee7d96 to
8b5798c
Compare
There was a problem hiding this comment.
The snapshot in the docs tests is broken
| /// Shows an info message to the user for the given [duration]. | ||
| /// | ||
| /// This is useful for showing messages like "Hold to record" or "Recording". | ||
| @Deprecated('Use StreamSnackbar via StreamSnackbarMessenger instead.') |
There was a problem hiding this comment.
Can't we still use this state, but just use this state to show a self-controllled snackbar? This way we're immediately breaking potential custom implementations.
There was a problem hiding this comment.
yes, but it will be a tech debt for us, but i get your point
The earlier direct-fire approach silently broke external consumers of the deprecated `showInfo` / `RecordStateIdle.message` API — their hint no longer rendered through the composer at all (e.g. the docs-screenshots golden test that pre-populates the state). Reinstate the bridge: `_StreamChatMessageInputState` listens to the audio recorder controller. On transitions to `RecordStateIdle(message: nonEmpty)` it fires `StreamSnackbar` via `messenger.show(replace: true)`; on transitions away (state.message cleared or out of idle), it calls `messenger.removeCurrent()`. When the snackbar is dismissed (swipe / timeout / programmatic), the listener notifies the recorder via the new `hideInfo()` companion so a repeat `showInfo(sameMessage)` isn't swallowed by `showInfo`'s in-built dedupe. Also: - Cancel `_infoTimer` in `StreamAudioRecorderController.dispose()` (real bug: was leaking the timer beyond the controller's lifetime). - Null out `_infoTimer` after the timer fires so the field state matches "is there a pending timer?". - Add 3 unit tests for `hideInfo` (immediate clear, cancels pending timer, no-op on empty state) and 2 widget tests covering the swipe-clears-state + hold-to-record gesture flow.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_chat_message_input.dart`:
- Around line 122-125: After swapping audioRecorderController listeners in
StreamChatMessageInput, immediately reconcile the snackbar/UI by applying the
new controller's current state rather than waiting for the next notifier event:
after calling
oldWidget.audioRecorderController?.removeListener(_onAudioRecorderChanged) and
widget.audioRecorderController?.addListener(_onAudioRecorderChanged), invoke the
same update logic (e.g., call _onAudioRecorderChanged() or read
widget.audioRecorderController's current value and run the snackbar update
routine) so any existing idle/recording state on the new controller is handled
right away and the snackbar isn't missed.
- Around line 154-157: The callback on controller.closed currently calls
widget.audioRecorderController?.hideInfo() for any snackbar closure, which can
clear state for newer snackbars; update the callback in the
StreamChatMessageInput where controller.closed.then(...) is registered to only
call hideInfo() if the closed controller is the same snackbar that created the
audio info UI. Concretely, capture the controller instance used to show the
snackbar (the local controller variable in the controller.closed.then block) and
compare it against the active/snackbar controller tracked on
widget.audioRecorderController (or another stored currentSnackbar reference)
before invoking widget.audioRecorderController?.hideInfo(), so hideInfo() runs
only when the same controller is closing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b39cbc58-e203-45f1-b922-2364c388534f
⛔ Files ignored due to path filters (3)
docs/docs_screenshots/test/localization/goldens/macos/localization_support.pngis excluded by!**/*.pngdocs/docs_screenshots/test/voice_recording/goldens/macos/voice_recording_idle_tooltip.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/message_input/goldens/ci/composer_hold_to_record_snackbar.pngis excluded by!**/*.png
📒 Files selected for processing (4)
packages/stream_chat_flutter/lib/src/message_input/audio_recorder/audio_recorder_controller.dartpackages/stream_chat_flutter/lib/src/message_input/stream_chat_message_input.dartpackages/stream_chat_flutter/test/src/message_input/audio_recorder/audio_recorder_controller_test.dartpackages/stream_chat_flutter/test/src/message_input/message_input_test.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_chat_flutter/test/src/message_input/message_input_test.dart
The explicit `messenger.removeCurrent()` in `onLongPressStart` is already covered by the listener: when `startRecord` transitions `value` from `RecordStateIdle` to `RecordStateRecordingHold`, the listener observes the change and dismisses the in-flight hint via `removeCurrent()` itself. The explicit call was racing the listener by ~50ms for marginal latency win. Refreshed test: `starting a hold clears the in-flight hold-to-record snackbar` now invokes the real long-press handler (mocks recorder permission + start), pumps under 1s to avoid ticking the periodic duration timer, and verifies the snackbar disappears + state is in `RecordStateRecordingHold` — exercising the listener path end-to-end. Also: scope the empty-attachments SizedBox finders in `message_input_attachment_list_test.dart` to descendants of the widget under test. The earlier bare `find.byType(SizedBox)` was brittle and started matching the SizedBox.shrink() rendered by the empty `_SnackbarStage` now that `StreamChat` wraps with `StreamSnackbarScope`.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2739 +/- ##
==========================================
+ Coverage 68.16% 68.48% +0.31%
==========================================
Files 413 413
Lines 24851 24868 +17
==========================================
+ Hits 16940 17030 +90
+ Misses 7911 7838 -73 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`addListener` on a `ValueNotifier` doesn't replay the current value — only subsequent `notifyListeners()` calls fire it. That left two gaps in `StreamChatMessageInput`: 1. **Controller swap (didUpdateWidget).** Reviewer-flagged: when the parent swaps `audioRecorderController`, we re-register the listener but never reconcile against the new controller's existing state. Any in-flight snackbar from the old controller is orphaned, and any state already set on the new controller is silently dropped. Fix: invoke `_onAudioRecorderChanged()` synchronously after re-registering. 2. **Initial mount (initState).** Same gap on first frame: a pre-populated controller (e.g. `RecordStateIdle(message: 'X')` set before mount) is ignored because the listener registers after the value is already in place. Fix: defer the sync to a post-frame callback so the inherited `StreamSnackbarMessenger` lookup is safe (inherited lookups aren't valid in `initState`). Also: re-anchor the docs `voice_recording_idle_tooltip` golden against the new flow — the scaffold now wraps in `StreamSnackbarPopup` so the listener-fired snackbar appears above the composer, matching the pre-refactor portal-target visual.
Summary
StreamChatMessageInput'sonLongPressCancelnow surfaces the hold-to-record hint throughStreamSnackbarMessenger.maybeOf(context).show(snackbar, replace: true);onLongPressStartclears any in-flight hint viaremoveCurrent()so the most recent gesture wins.StreamMessageComposerwraps its output inStreamSnackbarPopupat the factory-dispatch level, giving every composer variant (including custom builders registered viachatComponentBuilder<MessageComposerProps>) a stable snackbar surface anchored above the composer.StreamChatwraps its child inStreamSnackbarScopeso any subtree without a nearerStreamSnackbarPopupfalls back to an app-wide surface (keyboard-safe viaMediaQuery.viewInsets.bottom).StreamAudioRecorderController.showInfoandRecordStateIdle.message— the composer no longer reads either; external consumers should fire snackbars directly.StreamSnackbar,StreamSnackbarMessenger,StreamSnackbarPopup,StreamSnackbarPopupPlacement,StreamSnackbarScope,StreamSnackbarHost, theme types) fromstream_chat_flutter.dart.stream_core_flutterto the PR #118 commit until release.Tests
message_input_test.dartcovering the gesture wiring (cancel → snackbar, hold-clears-snackbar, rapid-cancel dedupe viareplace, feedback ordering, messenger lookup, global-scope fallback).Test plan
dart analyze --fatal-infos libcleanflutter test test/src/message_input/message_input_test.dart— 27/27 passmelos run update:goldens:sdk) — local-onlymacos/golden is gitignoredStreamSnackbarPopupancestor (e.g. custom layout that bypasses the composer wrapper), snackbars still surface viaStreamSnackbarScopeat the bottom of the screenDepends on
GetStream/stream-core-flutter#118 —
melos.yamlpinsstream_core_flutterto that branch's head; replace with the released version before merging.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Deprecations
Documentation
Tests