Skip to content

fix(ui): avoid spurious StreamTypingIndicator rebuilds on unchanged typing users#2826

Open
VelikovPetar wants to merge 1 commit into
masterfrom
fix/FLU-600_typing_indicator_comparator
Open

fix(ui): avoid spurious StreamTypingIndicator rebuilds on unchanged typing users#2826
VelikovPetar wants to merge 1 commit into
masterfrom
fix/FLU-600_typing_indicator_comparator

Conversation

@VelikovPetar

@VelikovPetar VelikovPetar commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Submit a pull request

Linear: FLU-600

Github Issue: #

CLA

  • I have signed the Stream CLA (required).
  • The code changes follow best practices
  • Code changes are tested (add some information if not applicable)

Description of the pull request

Problem. StreamTypingIndicator feeds a BetterStreamBuilder<Iterable<User>> a lazy MappedIterable (from .where().map()) with no comparator, so BetterStreamBuilder falls back to identity equality (event == _lastEvent). Every typing-stream emission produces a fresh Iterable identity — including repeated typing.start re-sends while a user keeps typing, the 1s stale-typing cleanup timer, and out-of-scope thread/main-channel events — forcing a rebuild even when the set of typing users hasn't changed.

Fix. Add a comparator to the BetterStreamBuilder that compares the typing users by user id (order-sensitive ListEquality<String>). Order-sensitivity is deliberate: the label renders users.first, so a reorder that changes who is first must still rebuild; a repeated typing.start for an already-typing user preserves insertion order and is therefore correctly suppressed. Comparing by id (rather than full User equality) also ignores unrelated churn such as presence updates on the same typing user.

Impact. Benchmarked against the real widget, before vs after, over an identical 43-event typing churn (one user typing → repeated re-emissions → a second user joins → more re-emissions → first stops):

Builds
Before (identity equality) 42
After (compare by id) 3

~14× fewer builds — but the more important change is the shape: build count goes from linear in the number of typing events to constant (= the number of distinct typing-user sets), independent of how much churn the backend re-sends or how often the stale-cleanup timer ticks.

Tests. The component previously had a single render smoke test. Added widget tests covering empty → one → many → empty updates (driven through a real stream) and parentId/thread filtering, closing pre-existing coverage gaps.

Breaking change. None. Not source-breaking (no signature changes). Behavioral-only and benign — the indicator no longer re-renders for unrelated User-object churn (e.g. presence updates) while the same users keep typing.

Screenshots / Videos

No UI changes. (Rendered output is identical; only the rebuild frequency changes.)

Summary by CodeRabbit

  • Bug Fixes

    • Improved typing indicators so they update only when the set of typing users changes.
    • Prevented unnecessary visual refreshes during repeated typing events.
    • Ensured typing indicators correctly filter events for the relevant conversation thread.
  • Tests

    • Added coverage for user updates, empty typing states, and thread-specific typing indicators.

…yping users

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

StreamTypingIndicator now compares typing users by ID before rebuilding. Tests cover changing typing-user sets and filtering events by parentId.

Changes

Typing indicator optimization

Layer / File(s) Summary
Typing user comparison
packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart, packages/stream_chat_flutter/CHANGELOG.md
Adds an ID-based comparator to BetterStreamBuilder and documents the reduced rebuild behavior.
Typing indicator behavior tests
packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart
Adds shared widget setup and stream-driven tests for typing-user changes, animation settling, and parentId filtering.

Estimated code review effort: 2 (Simple) | ~15 minutes

Suggested reviewers: xsahil03x

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing unnecessary StreamTypingIndicator rebuilds when typing users are unchanged.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/FLU-600_typing_indicator_comparator

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@VelikovPetar
VelikovPetar marked this pull request as ready for review July 17, 2026 17:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart (1)

53-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Filter the initial typing state using the same parentId predicate as stream emissions. Otherwise, a thread indicator can initially display a main-channel user until another event arrives.

  • packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart#L53-L58: filter channelState.typingEvents.entries by parentId before mapping the initial users.
  • packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart#L172-L194: seed initial typing data with a nonmatching main-channel event and verify it is never displayed.
🤖 Prompt for 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.

In `@packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart` around
lines 53 - 58, Filter the initial typing users in BetterStreamBuilder using
channelState.typingEvents.entries and the same parentId predicate applied to
stream emissions before mapping entries to users. In
packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart
lines 172-194, seed initial data with a nonmatching main-channel event and
verify it is never displayed.
🧹 Nitpick comments (1)
packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart (1)

147-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise the comparator’s unchanged-ID and reordered-ID branches.

These assertions only emit different user sets, so they pass even without comparator. Emit replacement User objects with the same ordered IDs and verify no visible update, then reverse their order and verify the displayed first user changes.

🤖 Prompt for 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.

In `@packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart`
around lines 147 - 158, Update the typing indicator test around emit and the
existing text assertions to cover comparator behavior: emit replacement User
objects with the same ordered IDs and assert the displayed text remains
unchanged, then emit the same users in reversed order and assert the displayed
first user changes accordingly.
🤖 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/test/src/indicators/typing_indicator_test.dart`:
- Around line 172-194: Extend the test around StreamTypingIndicator to seed
channelState.typingEvents with a main-channel typing event before mounting the
widget, then assert that User(id: 'user-a') is not rendered immediately after
the initial pump. Keep the existing stream-emission assertion to verify
filtering continues to work for subsequent events.

---

Outside diff comments:
In `@packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart`:
- Around line 53-58: Filter the initial typing users in BetterStreamBuilder
using channelState.typingEvents.entries and the same parentId predicate applied
to stream emissions before mapping entries to users. In
packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart
lines 172-194, seed initial data with a nonmatching main-channel event and
verify it is never displayed.

---

Nitpick comments:
In `@packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart`:
- Around line 147-158: Update the typing indicator test around emit and the
existing text assertions to cover comparator behavior: emit replacement User
objects with the same ordered IDs and assert the displayed text remains
unchanged, then emit the same users in reversed order and assert the displayed
first user changes accordingly.
🪄 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: 10a89c91-ee80-4cb4-8bfc-7b6f83f9f4cb

📥 Commits

Reviewing files that changed from the base of the PR and between 5f7f33c and 0c59e6d.

📒 Files selected for processing (3)
  • packages/stream_chat_flutter/CHANGELOG.md
  • packages/stream_chat_flutter/lib/src/indicators/typing_indicator.dart
  • packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart

Comment on lines +172 to +194
when(() => channelState.typingEvents).thenReturn(const {});
when(() => channelState.typingEventsStream).thenAnswer(
(_) => typingController.stream,
);

await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: StreamTypingIndicator(
channel: channel,
child: const Scaffold(
body: StreamTypingIndicator(
key: typingKey,
),
),
parentId: 'thread-1',
),
),
),
);
await tester.pump();

// wait for the initial state to be rendered.
await tester.pump(Duration.zero);
// Typing in the main channel (no parentId) is ignored by a thread
// indicator.
await emit(tester, typingController, {
User(id: 'user-a', name: 'Alice'): Event(type: EventType.typingStart),
});
expect(find.byType(Flexible), findsNothing);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Cover filtering of the initial typing state.

Initializing typingEvents to {} only tests filtering after stream emission. Seed it with a main-channel typing event before mounting the thread indicator and assert that user is never rendered; this exposes the unfiltered initialData at typing_indicator.dart Line 54.

🤖 Prompt for 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.

In `@packages/stream_chat_flutter/test/src/indicators/typing_indicator_test.dart`
around lines 172 - 194, Extend the test around StreamTypingIndicator to seed
channelState.typingEvents with a main-channel typing event before mounting the
widget, then assert that User(id: 'user-a') is not rendered immediately after
the initial pump. Keep the existing stream-emission assertion to verify
filtering continues to work for subsequent events.

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.28%. Comparing base (5f7f33c) to head (0c59e6d).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2826   +/-   ##
=======================================
  Coverage   71.28%   71.28%           
=======================================
  Files         430      430           
  Lines       26934    26939    +5     
=======================================
+ Hits        19199    19204    +5     
  Misses       7735     7735           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants