Skip to content

fix(navigation): clamp ViewGroup tab index when tab list shrinks#2225

Merged
sharjeelyunus merged 1 commit into
mainfrom
cursor/critical-bug-identification-7a62
May 19, 2026
Merged

fix(navigation): clamp ViewGroup tab index when tab list shrinks#2225
sharjeelyunus merged 1 commit into
mainfrom
cursor/critical-bug-identification-7a62

Conversation

@cursor
Copy link
Copy Markdown

@cursor cursor Bot commented May 19, 2026

Description

Fixes a crash when the global viewGroupNotifier holds a tab index that is no longer valid for the current pagePayloads list (for example after _restoreVisibleScreenOnForeground reapplies a stored index from before the app backgrounded, while the live definition now exposes fewer tabs). Drawer, sidebar, and bottom-nav code paths indexed pagePayloads[viewGroupNotifier.viewIndex] and used that value for IndexedStack / PageController.initialPage without bounds checks, which produced RangeError and broke the app.

Related Issue

None filed; found by tracing _restoreVisibleScreenOnForeground in ensemble_provider.dart and the ViewGroup build paths.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)

What Has Changed

  • Added safeViewGroupPayloadIndex in page_group.dart and used it wherever the UI indexes pagePayloads or drives IndexedStack by tab index.
  • When the notifier index is out of range, schedule a post-frame updatePage with the clamped index so persistence and navigation stay aligned.
  • BottomNavPageGroup / PageGroupWidgetWrapper now clamp PageController.initialPage and reload-view body indexing using the same helper; PageGroupWidgetWrapper takes pageCount for clamping.
  • Added unit tests for safeViewGroupPayloadIndex.

How to Test

  1. From modules/ensemble, run flutter test test/safe_view_group_payload_index_test.dart.
  2. Manually: use a ViewGroup with multiple tabs, select the last tab, background the app, deploy a definition with fewer tabs, resume with artifact refresh enabled so the stored index is restored; the app should show the last valid tab instead of crashing.

Screenshots / Videos

N/A

Checklist

  • I have run flutter analyze and addressed any new warnings
  • I have run flutter test and all tests pass
  • I have tested my changes on the relevant platform(s)
  • I have updated documentation if needed
  • My changes do not introduce new warnings or errors

Note: The automation sandbox did not have the Flutter SDK on PATH, so flutter test / flutter analyze were not executed here. CI or a local run is appreciated.

Duplicate check

Reviewed open and recent PRs with gh pr list (EnsembleUI/ensemble). Open PR #2220 clamps navigateViewGroup action indices only; it does not cover the background-restore path or defensive indexing in PageGroupState / BottomNavPageGroup, so this is not a duplicate of that work. Open #2215 addresses ListViewCore scroll-controller swapping (unrelated). No existing branch or PR fixes the same stale-index-after-resume trigger for pagePayloads indexing.

Open in Web View Automation 

Restoring a stored view-group index after resume or definition updates could
leave viewGroupNotifier pointing past pagePayloads, causing RangeError in
drawer, sidebar, and bottom-nav builds. Clamp at read sites and resync the
notifier post-frame when out of range; clamp PageController initialPage too.

Co-authored-by: Sharjeel Yunus <sharjeelyunus@users.noreply.github.com>
@sharjeelyunus sharjeelyunus marked this pull request as ready for review May 19, 2026 13:03
@sharjeelyunus sharjeelyunus merged commit 2b62b4a into main May 19, 2026
8 checks passed
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