Skip to content

fix: banner flicker (WPB-23994)#4672

Open
sbakhtiarov wants to merge 3 commits intodevelopfrom
fix/banner-flicker
Open

fix: banner flicker (WPB-23994)#4672
sbakhtiarov wants to merge 3 commits intodevelopfrom
fix/banner-flicker

Conversation

@sbakhtiarov
Copy link
Copy Markdown
Contributor

@sbakhtiarov sbakhtiarov commented Mar 23, 2026

https://wearezeta.atlassian.net/browse/WPB-23994

https://wearezeta.atlassian.net/browse/WPB-23994

What's new in this PR?

Issues

On every app start, the "Decrypting messages" or "Waiting for network" banner flashed briefly in the top app bar, even when the device was online and sync completed quickly (under one second).

Causes (Optional)

Two independent bugs combined to produce the flash:

  1. SyncState.Waiting was treated as a connectivity problem.
    Waiting is a pure pre-initialisation state — the sync worker has not been scheduled yet and carries no information about network health. Mapping it to WaitingConnection caused "Waiting for network" to appear immediately on every
    cold start, before sync even began.

  2. The debounce operator was placed outside flatMapLatest.
    currentSessionFlow() is a plain SQLDelight Flow that can emit multiple Success values at startup, causing flatMapLatest to restart its inner flow repeatedly. The debounce operator sat outside this restart boundary, so timers from
    cancelled inner flows survived and fired with stale state — producing the "Decrypting messages" flash even after sync had completed.

Solutions

  • Map SyncState.Waiting → Connectivity.Connected so no banner is shown during the pre-init window.
  • Add a debounce inside connectivityFlow that holds Connecting/WaitingConnection for CONNECTIVITY_STATE_DEBOUNCE_DEFAULT (1 s) before emitting, but passes Connected through at 0 ms immediately. If sync or network recovers within the
    window, the timer is cancelled and no banner ever appears.
  • Move the per-session debounce inside the flatMapLatest Success branch so it is cancelled together with its inner flow on session change. Connectivity states arrive here already debounced, so the per-session debounce only adds
    extra delay for the ongoing-call bar (600 ms, to absorb rapid mute/unmute changes without flickering). Everything else passes through at 0 ms.

@MohamadJaara
Copy link
Copy Markdown
Member

not sure how i feel about Map SyncState.Waiting → Connectivity.Connected i would much rather give it its own state to avoid issues and misunderstandings, like someone sees in the logs Connectivity.Connected but they can't send messages somehow

Copy link
Copy Markdown
Member

@MohamadJaara MohamadJaara left a comment

Choose a reason for hiding this comment

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

question about waiting to connected

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

Automated PR Review

Generated by gpt-4.1-mini in 12617 ms.

Summary

This PR addresses a UI flicker issue where the "Decrypting messages" or "Waiting for network" banner briefly flashes on app start, even when the device is online and sync completes quickly. The root causes were identified as incorrect mapping of the SyncState.Waiting state to a connectivity problem and the debounce operator being placed outside the flatMapLatest scope, causing stale timers to trigger banner flashes.

The solution involves:

  • Mapping SyncState.Waiting to Connectivity.Connected to avoid showing a banner during pre-initialization.
  • Adding a debounce inside the connectivity flow that delays emission of Connecting or WaitingConnection states but passes Connected immediately.
  • Moving the per-session debounce inside the flatMapLatest success branch to ensure debounce timers are cancelled on session changes, preventing stale state emissions.

Bugs

  • No new bugs introduced are evident from the diff.
  • The original flicker bug is well explained and addressed by the changes.
  • The debounce logic is carefully adjusted to avoid stale emissions, which should prevent the flicker.

Architectural Issues

  • The approach of mapping SyncState.Waiting to Connectivity.Connected is a pragmatic fix but assumes that Waiting always means no network issue. This is reasonable given the explanation but should be documented clearly as it changes the semantic meaning of Waiting.
  • Debounce placement inside the flatMapLatest scope is a good architectural improvement, ensuring cancellation of timers on session changes and avoiding stale UI states.
  • The use of different debounce durations for connectivity states vs ongoing calls is a thoughtful design to balance responsiveness and flicker prevention.

Improvements / Recommendations

  • Consider adding unit tests or integration tests that simulate the sync states and network states to verify that the banner does not flicker on app start and behaves correctly on network changes.
  • Add comments or documentation in the code explaining why SyncState.Waiting is mapped to Connectivity.Connected to prevent confusion for future maintainers.
  • The debounce durations are hardcoded constants; consider making them configurable or at least document the rationale for chosen values in the code or PR description.
  • The new GitHub workflow .github/workflows/pr-review-service.yml is unrelated to the banner fix but appears to add an automated PR review service. Ensure this addition is intentional and reviewed separately as it affects CI/CD.

Open Questions / Coverage Notes

  • The diff coverage is complete for the relevant file CommonTopAppBarViewModel.kt. No other UI or sync-related files were changed.
  • The PR description is thorough and explains the problem and solution well.
  • The new workflow file .github/workflows/pr-review-service.yml was added but is unrelated to the banner flicker fix. No review of its correctness or security implications was requested.
  • No UI screenshots or logs were provided to verify the fix visually or functionally.
  • It is unclear if the debounce changes affect other parts of the app that consume connectivity state; a broader impact analysis might be warranted.

Overall, this PR provides a well-reasoned and clean fix for the banner flicker issue with thoughtful debounce handling and state mapping. The addition of the automated PR review workflow should be reviewed separately.

@pull-request-size pull-request-size bot added size/XL and removed size/S labels Apr 1, 2026
@pull-request-size
Copy link
Copy Markdown

Ups 🫰🟨

This PR is too big. Please try to break it up into smaller PRs.

@pull-request-size pull-request-size bot added size/S and removed size/XL labels Apr 1, 2026
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 1, 2026

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants