feat(ui): floating header#115
Open
renefloor wants to merge 37 commits into
Open
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntroduces floating rendering across app bars, scaffolds, bottom navigation, and avatar widgets. Adds shared theme contracts, gradient utilities, component customization, disabled floating-button styling, gallery previews, and widget/golden coverage. ChangesFloating UI feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/design_system_gallery/lib/components/toolbar/stream_app_bar.dart (1)
100-100: ⚡ Quick winReplace hardcoded margin with spacing token.
The horizontal margin of 32 should use a spacing token from
context.streamSpacingrather than a hardcoded value. As per coding guidelines, always use spacing tokens from the theme context instead of hardcoded values.♻️ Proposed fix
margin: const EdgeInsets.symmetric(horizontal: 32), + // Replace with appropriate spacing token, e.g.: + // margin: EdgeInsets.symmetric(horizontal: spacing.xl),Note: Choose the spacing token that best matches the intended 32-pixel spacing (likely
spacing.xlor similar).🤖 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 `@apps/design_system_gallery/lib/components/toolbar/stream_app_bar.dart` at line 100, Replace the hardcoded horizontal margin of 32 in StreamAppBar with the theme spacing token: locate the margin: const EdgeInsets.symmetric(horizontal: 32) in stream_app_bar.dart (the AppBar/Container/widget that sets margin) and use the spacing token from context.streamSpacing (e.g., EdgeInsets.symmetric(horizontal: context.streamSpacing.xl) or the appropriate token that maps to 32px) so the layout uses the theme spacing instead of a literal value.
🤖 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.
Nitpick comments:
In `@apps/design_system_gallery/lib/components/toolbar/stream_app_bar.dart`:
- Line 100: Replace the hardcoded horizontal margin of 32 in StreamAppBar with
the theme spacing token: locate the margin: const
EdgeInsets.symmetric(horizontal: 32) in stream_app_bar.dart (the
AppBar/Container/widget that sets margin) and use the spacing token from
context.streamSpacing (e.g., EdgeInsets.symmetric(horizontal:
context.streamSpacing.xl) or the appropriate token that maps to 32px) so the
layout uses the theme spacing instead of a literal value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f9bb118-2547-48c3-ab91-fa8c2e73b2eb
📒 Files selected for processing (6)
apps/design_system_gallery/lib/components/toolbar/stream_app_bar.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_app_bar.dartpackages/stream_core_flutter/lib/src/theme/components/stream_app_bar_theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_app_bar_theme.g.theme.dartpackages/stream_core_flutter/test/components/toolbar/stream_app_bar_golden_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_app_bar_test.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/stream_core_flutter/lib/src/theme/stream_app_style.dart (1)
7-11: 💤 Low valueClarify naming:
StreamAppStyle.floating()keepsappBarBehaviorasregular.The
.floating()constructor setscomposerLocation = floatingbutappBarBehavior = AppBarBehavior.regular. This may confuse consumers who expect "floating" to also affect the app bar. Consider either:
- Adding a doc comment explaining this distinction, or
- Renaming to
StreamAppStyle.floatingComposer()for clarity🤖 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_core_flutter/lib/src/theme/stream_app_style.dart` around lines 7 - 11, The floating constructor StreamAppStyle.floating() sets composerLocation = ComposerLocation.floating but leaves appBarBehavior = AppBarBehavior.regular which can confuse callers; update the API to make intent explicit by either adding a doc comment on StreamAppStyle.floating() that documents that only the composerLocation changes (appBarBehavior remains regular), or rename the constructor to StreamAppStyle.floatingComposer() and update all usages to the new name so it clearly indicates only the composer is floating while composerLocation and appBarBehavior fields remain explicit.
🤖 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_core_flutter/lib/src/components/toolbar/stream_app_bar.dart`:
- Around line 1-2: Remove the unused JavaScript interop import: delete the line
"import 'dart:js_interop';" from stream_app_bar.dart (the import at the top of
the file) and run the analyzer or build to confirm there are no remaining
references to dart:js_interop in this file; this file's symbols like the
StreamAppBar widget/constructor do not require JS interop so the import is safe
to remove.
---
Nitpick comments:
In `@packages/stream_core_flutter/lib/src/theme/stream_app_style.dart`:
- Around line 7-11: The floating constructor StreamAppStyle.floating() sets
composerLocation = ComposerLocation.floating but leaves appBarBehavior =
AppBarBehavior.regular which can confuse callers; update the API to make intent
explicit by either adding a doc comment on StreamAppStyle.floating() that
documents that only the composerLocation changes (appBarBehavior remains
regular), or rename the constructor to StreamAppStyle.floatingComposer() and
update all usages to the new name so it clearly indicates only the composer is
floating while composerLocation and appBarBehavior fields remain explicit.
🪄 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: ac5ca1cf-6f3b-4b23-a425-08b5d6d077c5
📒 Files selected for processing (5)
packages/stream_core_flutter/lib/src/components/toolbar/stream_app_bar.dartpackages/stream_core_flutter/lib/src/theme.dartpackages/stream_core_flutter/lib/src/theme/stream_app_style.dartpackages/stream_core_flutter/lib/src/theme/stream_theme.dartpackages/stream_core_flutter/lib/src/theme/stream_theme.g.theme.dart
✅ Files skipped from review due to trivial changes (1)
- packages/stream_core_flutter/lib/src/theme/stream_theme.g.theme.dart
renefloor
force-pushed
the
feat/floating-header
branch
from
May 28, 2026 15:06
234103c to
24ae763
Compare
|
Actionable comments posted: 0 |
renefloor
force-pushed
the
feat/floating-header
branch
from
May 29, 2026 11:19
61b0dfb to
3bd5156
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dart`:
- Around line 203-224: Wrap the floating nav item in a Semantics widget (around
the existing GestureDetector or its child) and set semantics properties to
expose it as a button with selection state: provide button: true, selected:
selected, label: item.label (and optionally a meaningful onTapHint), and include
the onTap callback via the Semantics onTap so assistive tech announces role and
selected/unselected state while preserving the existing onTap behavior; update
the build that uses item, selected, and onTap 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: 7a1618e8-8268-4f3e-a9c4-cd1534d3b36c
📒 Files selected for processing (4)
packages/stream_core_flutter/lib/src/components.dartpackages/stream_core_flutter/lib/src/components/scaffold/stream_scaffold.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dartpackages/stream_core_flutter/lib/src/theme/stream_app_style.dart
✅ Files skipped from review due to trivial changes (1)
- packages/stream_core_flutter/lib/src/components.dart
renefloor
force-pushed
the
feat/floating-header
branch
2 times, most recently
from
June 12, 2026 08:20
e41df72 to
bb75889
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #115 +/- ##
==========================================
+ Coverage 44.53% 50.32% +5.78%
==========================================
Files 178 183 +5
Lines 7243 7531 +288
==========================================
+ Hits 3226 3790 +564
+ Misses 4017 3741 -276 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3 tasks
renefloor
force-pushed
the
feat/floating-header
branch
from
June 16, 2026 13:41
f547a57 to
8c584a4
Compare
renefloor
force-pushed
the
feat/floating-header
branch
from
June 16, 2026 14:03
8c584a4 to
682d15f
Compare
# Conflicts: # packages/stream_core_flutter/lib/src/components/message/stream_message_text.dart
xsahil03x
reviewed
Jun 18, 2026
xsahil03x
reviewed
Jun 18, 2026
# Conflicts: # packages/stream_core_flutter/CHANGELOG.md # packages/stream_core_flutter/lib/src/components.dart # packages/stream_core_flutter/lib/src/theme.dart
renefloor
force-pushed
the
feat/floating-header
branch
2 times, most recently
from
June 19, 2026 09:20
a8ee82d to
a78be57
Compare
renefloor
force-pushed
the
feat/floating-header
branch
from
June 19, 2026 09:30
a78be57 to
7c55b73
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/stream_core_flutter/CHANGELOG.md (1)
12-14: ⚡ Quick winVary the opening phrasing to reduce repetition in the "Upcoming" release notes.
Lines 12–14 begin three consecutive bullet points with "- Added", which reads awkwardly and affects documentation clarity. Consider grouping related additions or rewording for variety (e.g., "Introduced", "Added support for", "New component").
- Added `StreamSnackbar` and `StreamSnackbarTheme` — Stream-styled transient feedback snackbars with a messenger-driven queue. - Added `StreamIcons.megaphone` and `StreamIcons.shield` (20px) to the icon set. - Added `StreamMentionType` identifier for supported mention types and options for mention text customisation per type. + Introduced `StreamSnackbar` and `StreamSnackbarTheme` — Stream-styled transient feedback snackbars with a messenger-driven queue. + Added new icon variants (`StreamIcons.megaphone` and `StreamIcons.shield`, 20px) to the icon set. + Added `StreamMentionType` identifier for supported mention types and options for mention text customisation per type.This pattern also appears frequently in the 0.3.0 section (lines 24–29+); consider applying similar rewording there for consistency.
🤖 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_core_flutter/CHANGELOG.md` around lines 12 - 14, The "Upcoming" release notes section contains three consecutive bullet points (lines 12-14) that all begin with "- Added", creating repetitive and awkward phrasing. Reword these bullet points with varied opening phrases such as "Introduced", "Added support for", or "New component" to improve readability and flow. Additionally, apply the same rewording approach to the 0.3.0 section (lines 24-29+) where the same repetitive "- Added" pattern appears multiple times, ensuring consistent and varied language throughout the changelog.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In `@packages/stream_core_flutter/CHANGELOG.md`:
- Around line 12-14: The "Upcoming" release notes section contains three
consecutive bullet points (lines 12-14) that all begin with "- Added", creating
repetitive and awkward phrasing. Reword these bullet points with varied opening
phrases such as "Introduced", "Added support for", or "New component" to improve
readability and flow. Additionally, apply the same rewording approach to the
0.3.0 section (lines 24-29+) where the same repetitive "- Added" pattern appears
multiple times, ensuring consistent and varied language throughout the
changelog.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 460a0c5a-50a6-4464-88a8-469e7a5907ed
⛔ Files ignored due to path filters (12)
packages/stream_core_flutter/test/components/avatar/goldens/ci/stream_avatar_group_shadow_dark.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/avatar/goldens/ci/stream_avatar_group_shadow_light.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/avatar/goldens/ci/stream_avatar_shadow_dark.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/avatar/goldens/ci/stream_avatar_shadow_light.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/avatar/goldens/ci/stream_avatar_stack_shadow_dark.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/avatar/goldens/ci/stream_avatar_stack_shadow_light.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/buttons/goldens/ci/stream_button_icon_only_floating.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/buttons/goldens/ci/stream_button_icon_only_floating_disabled.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/toolbar/goldens/ci/stream_app_bar_dark.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/toolbar/goldens/ci/stream_app_bar_floating_dark.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/toolbar/goldens/ci/stream_app_bar_floating_light.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/toolbar/goldens/ci/stream_app_bar_light.pngis excluded by!**/*.png
📒 Files selected for processing (27)
apps/design_system_gallery/lib/components/avatar/stream_avatar.dartapps/design_system_gallery/lib/components/avatar/stream_avatar_group.dartapps/design_system_gallery/lib/components/avatar/stream_avatar_stack.dartapps/design_system_gallery/lib/components/toolbar/stream_app_bar.dartpackages/stream_core_flutter/CHANGELOG.mdpackages/stream_core_flutter/lib/core.dartpackages/stream_core_flutter/lib/src/components/avatar/stream_avatar.dartpackages/stream_core_flutter/lib/src/components/avatar/stream_avatar_group.dartpackages/stream_core_flutter/lib/src/components/avatar/stream_avatar_stack.dartpackages/stream_core_flutter/lib/src/components/buttons/stream_button.dartpackages/stream_core_flutter/lib/src/components/scaffold/stream_scaffold.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_app_bar.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dartpackages/stream_core_flutter/lib/src/theme/components/stream_app_bar_theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_app_bar_theme.g.theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_avatar_theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_avatar_theme.g.theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_bottom_app_bar_theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_bottom_app_bar_theme.g.theme.dartpackages/stream_core_flutter/lib/src/theme/stream_app_style.dartpackages/stream_core_flutter/lib/src/theme/stream_floating_fade.dartpackages/stream_core_flutter/lib/src/theme/stream_theme.dartpackages/stream_core_flutter/lib/src/theme/stream_theme.g.theme.dartpackages/stream_core_flutter/test/components/avatar/stream_avatar_golden_test.dartpackages/stream_core_flutter/test/components/buttons/stream_button_golden_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_app_bar_golden_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_app_bar_test.dart
✅ Files skipped from review due to trivial changes (3)
- packages/stream_core_flutter/lib/src/theme/stream_floating_fade.dart
- packages/stream_core_flutter/lib/src/theme/stream_theme.g.theme.dart
- packages/stream_core_flutter/lib/src/theme/components/stream_bottom_app_bar_theme.g.theme.dart
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/stream_core_flutter/lib/src/theme/stream_theme.dart
- apps/design_system_gallery/lib/components/toolbar/stream_app_bar.dart
- packages/stream_core_flutter/test/components/toolbar/stream_app_bar_golden_test.dart
- packages/stream_core_flutter/lib/src/components/scaffold/stream_scaffold.dart
- packages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dart
# Conflicts: # packages/stream_core_flutter/lib/src/components/toolbar/stream_app_bar.dart # packages/stream_core_flutter/test/components/toolbar/stream_app_bar_test.dart
Restructure StreamBottomNavBar onto the standard design-system shape: Props + StreamBottomNavBar (factory lookup) + DefaultStreamBottomNavBar, with styling resolved via effective-value locals off a token-backed _StreamBottomNavBarStyleDefaults. - Add StreamBottomNavBarTheme / ThemeData / Style (@themeGen), registered on StreamTheme, exposed via context.streamBottomNavBarTheme and core.dart. - Add a dedicated StreamBottomNavBarBehavior enum; resolution order is per-instance behavior -> nav-bar theme -> StreamAppStyle. - Own the docked + floating rendering (animated tiles, tap ripple, tab semantics) instead of delegating the regular case to a Material widget. - Register a bottomNavBar hook on StreamComponentFactory. - Add unit + semantics tests and a golden test (regular/floating x light/dark); update the gallery use-cases to sit in a StreamScaffold-style preview. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…wner Toggling the semantics debugger on threw a null-check error: the pipeline owner was only wired in didChangeDependencies (gated on enabled), which isn't re-run on a plain prop toggle, so build hit `_pipelineOwner!` while still null. Sync the owner when enabling and fall back to the child when it is unset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tomNavBar Resolve the regular and floating backgrounds as separate, unconditional effective-value locals (effectiveBackgroundColor / effectiveFloatingBackgroundColor) instead of a behavior switch that collapsed them into one value, and rename effectiveStreamAppBarBehavior to effectiveBehavior. Behavior-preserving — the rendered output is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
xsahil03x
approved these changes
Jul 20, 2026
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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
`@apps/design_system_gallery/lib/components/toolbar/stream_bottom_nav_bar.dart`:
- Around line 214-222: Update the Container wrapping bar to move its border from
BoxDecoration.border into a foregroundDecoration BoxDecoration, preserving the
existing color, radius, and border styling while keeping clipBehavior unchanged.
- Around line 74-86: Convert the result of _currentIndex.clamp(0, items.length -
1) to int when assigning currentIndex, so it satisfies StreamBottomNavBar’s int
currentIndex parameter while preserving the existing bounds.
In `@packages/stream_core_flutter/CHANGELOG.md`:
- Line 24: Correct the changelog entry for StreamBottomNavBar to remove
StreamBottomAppBarTheme from the behavior resolution chain, since
StreamBottomNavBarTheme.of() only resolves the local and ambient bottom
navigation bar themes. Preserve the remaining documented themes and resolution
order.
In
`@packages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dart`:
- Around line 97-111: Add an assertion in the StreamBottomNavBar constructor
validating that currentIndex is within the items range (currentIndex >= 0 &&
currentIndex < items.length), with a clear message. Keep the existing
minimum-items assertion and ensure the same invariant protects _resetState and
didUpdateWidget from out-of-range controller access.
🪄 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: 6535d66f-b59c-46c8-9856-bf1e44295f01
⛔ Files ignored due to path filters (2)
packages/stream_core_flutter/test/components/toolbar/goldens/ci/stream_bottom_nav_bar_floating.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/toolbar/goldens/ci/stream_bottom_nav_bar_regular.pngis excluded by!**/*.png
📒 Files selected for processing (21)
apps/design_system_gallery/lib/app/gallery_app.directories.g.dartapps/design_system_gallery/lib/components/toolbar/stream_bottom_nav_bar.dartapps/design_system_gallery/lib/widgets/scoped_semantics_debugger.dartpackages/stream_core_flutter/CHANGELOG.mdpackages/stream_core_flutter/lib/core.dartpackages/stream_core_flutter/lib/src/components/buttons/stream_button.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_app_bar.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dartpackages/stream_core_flutter/lib/src/factory/stream_component_factory.dartpackages/stream_core_flutter/lib/src/factory/stream_component_factory.g.theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_bottom_nav_bar_theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_bottom_nav_bar_theme.g.theme.dartpackages/stream_core_flutter/lib/src/theme/stream_theme.dartpackages/stream_core_flutter/lib/src/theme/stream_theme.g.theme.dartpackages/stream_core_flutter/lib/src/theme/stream_theme_extensions.dartpackages/stream_core_flutter/test/components/avatar/stream_avatar_test.dartpackages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_app_bar_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_bottom_nav_bar_golden_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_bottom_nav_bar_test.dartpackages/stream_core_flutter/test/theme/stream_theme_test.dart
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/stream_core_flutter/lib/src/components/buttons/stream_button.dart
- packages/stream_core_flutter/test/components/toolbar/stream_app_bar_test.dart
- packages/stream_core_flutter/lib/src/components/toolbar/stream_app_bar.dart
- packages/stream_core_flutter/lib/core.dart
Comment on lines
+74
to
+86
| final items = _navItems(context).take(itemCount).toList(); | ||
| // Keep the selected index in range as the item count changes. | ||
| final currentIndex = _currentIndex.clamp(0, items.length - 1); | ||
|
|
||
| return Align( | ||
| alignment: Alignment.bottomCenter, | ||
| child: StreamBottomNavBar( | ||
| items: items, | ||
| currentIndex: currentIndex, | ||
| onTap: (index) => setState(() => _currentIndex = index), | ||
| behavior: floating ? StreamBottomNavBarBehavior.floating : StreamBottomNavBarBehavior.regular, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Call site ==\n'
sed -n '68,90p' apps/design_system_gallery/lib/components/toolbar/stream_bottom_nav_bar.dart
printf '\n== StreamBottomNavBar signature ==\n'
rg -n "class StreamBottomNavBar|currentIndex" apps/design_system_gallery/lib/components/toolbar -A 4 -B 4Repository: GetStream/stream-core-flutter
Length of output: 6006
🌐 Web query:
Dart int.clamp return type num official docs
💡 Result:
According to the official Dart documentation, the clamp method is defined on the num class, and its static return type is num [1][2][3]. While int inherits this method from num, the static signature remains num clamp(num lowerLimit, num upperLimit) [1][2]. Although the method will return an int at runtime if the receiver and both arguments are integers, the Dart type system statically treats the return value as num [4][2][5].
Citations:
- 1: https://api.dart.dev/dart-core/int-class.html
- 2: https://api.dart.dev/dev/latest/dart-core/num/clamp.html
- 3: https://api.dart.dev/dart-core/num-class.html
- 4: int.clamp() should statically return int if the arguments are ints dart-lang/sdk#28249
- 5: double.clamp should return a double dart-lang/sdk#39652
Convert the clamped index back to int.
clamp() returns num, so currentIndex is not assignable to the int parameter here and the file won’t type-check.
🐛 Proposed fix
- final currentIndex = _currentIndex.clamp(0, items.length - 1);
+ final currentIndex = _currentIndex.clamp(0, items.length - 1).toInt();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final items = _navItems(context).take(itemCount).toList(); | |
| // Keep the selected index in range as the item count changes. | |
| final currentIndex = _currentIndex.clamp(0, items.length - 1); | |
| return Align( | |
| alignment: Alignment.bottomCenter, | |
| child: StreamBottomNavBar( | |
| items: items, | |
| currentIndex: currentIndex, | |
| onTap: (index) => setState(() => _currentIndex = index), | |
| behavior: floating ? StreamBottomNavBarBehavior.floating : StreamBottomNavBarBehavior.regular, | |
| ), | |
| ); | |
| final items = _navItems(context).take(itemCount).toList(); | |
| // Keep the selected index in range as the item count changes. | |
| final currentIndex = _currentIndex.clamp(0, items.length - 1).toInt(); | |
| return Align( | |
| alignment: Alignment.bottomCenter, | |
| child: StreamBottomNavBar( | |
| items: items, | |
| currentIndex: currentIndex, | |
| onTap: (index) => setState(() => _currentIndex = index), | |
| behavior: floating ? StreamBottomNavBarBehavior.floating : StreamBottomNavBarBehavior.regular, | |
| ), | |
| ); |
🤖 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 `@apps/design_system_gallery/lib/components/toolbar/stream_bottom_nav_bar.dart`
around lines 74 - 86, Convert the result of _currentIndex.clamp(0, items.length
- 1) to int when assigning currentIndex, so it satisfies StreamBottomNavBar’s
int currentIndex parameter while preserving the existing bounds.
Comment on lines
+214
to
+222
| Container( | ||
| clipBehavior: Clip.antiAlias, | ||
| decoration: BoxDecoration( | ||
| color: colorScheme.backgroundSurface, | ||
| borderRadius: BorderRadius.all(radius.lg), | ||
| border: Border.all(color: colorScheme.borderSubtle), | ||
| ), | ||
| child: bar, | ||
| ), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Move the border to foregroundDecoration.
Border drawn via BoxDecoration.border here is painted behind the child and can be clipped by clipBehavior: Clip.antiAlias. As per path instructions, apps/design_system_gallery/{lib/components,lib/semantics,lib/primitives,lib/widgets}/**/*.dart: "Use foregroundDecoration for borders to prevent clipping issues, rather than using border property in BoxDecoration."
♻️ Proposed fix
Container(
clipBehavior: Clip.antiAlias,
decoration: BoxDecoration(
color: colorScheme.backgroundSurface,
borderRadius: BorderRadius.all(radius.lg),
- border: Border.all(color: colorScheme.borderSubtle),
+ ),
+ foregroundDecoration: BoxDecoration(
+ borderRadius: BorderRadius.all(radius.lg),
+ border: Border.all(color: colorScheme.borderSubtle),
),
child: bar,
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Container( | |
| clipBehavior: Clip.antiAlias, | |
| decoration: BoxDecoration( | |
| color: colorScheme.backgroundSurface, | |
| borderRadius: BorderRadius.all(radius.lg), | |
| border: Border.all(color: colorScheme.borderSubtle), | |
| ), | |
| child: bar, | |
| ), | |
| Container( | |
| clipBehavior: Clip.antiAlias, | |
| decoration: BoxDecoration( | |
| color: colorScheme.backgroundSurface, | |
| borderRadius: BorderRadius.all(radius.lg), | |
| ), | |
| foregroundDecoration: BoxDecoration( | |
| borderRadius: BorderRadius.all(radius.lg), | |
| border: Border.all(color: colorScheme.borderSubtle), | |
| ), | |
| child: bar, | |
| ), |
🤖 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 `@apps/design_system_gallery/lib/components/toolbar/stream_bottom_nav_bar.dart`
around lines 214 - 222, Update the Container wrapping bar to move its border
from BoxDecoration.border into a foregroundDecoration BoxDecoration, preserving
the existing color, radius, and border styling while keeping clipBehavior
unchanged.
Source: Path instructions
|
|
||
| - Split the public API into `package:stream_core_flutter/core.dart` (shared primitives for any Stream SDK) and `package:stream_core_flutter/chat.dart` (chat-only widgets; re-exports `core.dart`); the convenience barrel `package:stream_core_flutter/stream_core_flutter.dart` is now deprecated. | ||
| - Added `StreamScaffold` — a full-page scaffold that supports both regular and floating app-bar / bottom-bar layouts. Injects `StreamScaffoldInsets` into the widget tree so scrollable bodies can read the effective top and bottom padding from floating bars without coupling to layout details. | ||
| - Added `StreamBottomNavBar` and `StreamBottomNavBarItem` — a bottom navigation bar with icon, selected-icon, and label slots per item. Renders either a regular docked bar or a floating pill with a gradient fade-out beneath it, resolved from the per-instance `behavior`, the `StreamBottomNavBarTheme`, the `StreamBottomAppBarTheme` (kept in sync with `StreamScaffold`), then `StreamAppStyle`. Follows the standard `Props`/`Default`/`StreamComponentFactory` pattern (`bottomNavBar` builder) and is themeable via `StreamBottomNavBarTheme` / `StreamBottomNavBarStyle` (selected/unselected item colours, icon size, label styles, border, pill radius). Items announce themselves as accessible buttons via `Semantics`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Changelog describes StreamBottomAppBarTheme in the resolution chain, but it has no effect.
StreamBottomNavBarTheme.of() only merges the local StreamBottomNavBarTheme with the ambient StreamTheme.of(context).bottomNavBarTheme — it never reads StreamBottomAppBarTheme. This is explicitly confirmed by the new test 'is independent of StreamBottomAppBarTheme' in stream_bottom_nav_bar_test.dart, which asserts a floating StreamBottomAppBarTheme has no effect on the nav bar's behavior. The changelog entry should be corrected to avoid misleading consumers about the resolution order.
🤖 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_core_flutter/CHANGELOG.md` at line 24, Correct the changelog
entry for StreamBottomNavBar to remove StreamBottomAppBarTheme from the behavior
resolution chain, since StreamBottomNavBarTheme.of() only resolves the local and
ambient bottom navigation bar themes. Preserve the remaining documented themes
and resolution order.
Comment on lines
+97
to
+111
| StreamBottomNavBar({ | ||
| super.key, | ||
| required List<StreamBottomNavBarItem> items, | ||
| required int currentIndex, | ||
| required ValueChanged<int> onTap, | ||
| StreamBottomNavBarBehavior? behavior, | ||
| StreamBottomNavBarStyle? style, | ||
| }) : assert(items.length >= 2, 'StreamBottomNavBar requires at least 2 items'), | ||
| props = .new( | ||
| items: items, | ||
| currentIndex: currentIndex, | ||
| onTap: onTap, | ||
| behavior: behavior, | ||
| style: style, | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add a bounds assertion for currentIndex.
items.length is validated but currentIndex isn't; an out-of-range value crashes with an unguarded RangeError in _resetState/didUpdateWidget (_controllers[widget.props.currentIndex]) instead of a clear assertion message.
🛡️ Proposed fix
}) : assert(items.length >= 2, 'StreamBottomNavBar requires at least 2 items'),
+ assert(
+ currentIndex >= 0 && currentIndex < items.length,
+ 'currentIndex must be within the bounds of items',
+ ),
props = .new(Also applies to: 199-218, 243-257
🤖 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_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dart`
around lines 97 - 111, Add an assertion in the StreamBottomNavBar constructor
validating that currentIndex is within the items range (currentIndex >= 0 &&
currentIndex < items.length), with a clear message. Keep the existing
minimum-items assertion and ensure the same invariant protects _resetState and
didUpdateWidget from out-of-range controller access.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Submit a pull request
Linear: FLU-502
CLA
Description of the pull request
This adds the option to create a floating appbar. It automatically adjusts the background from a color to a gradient and removes the border. Also makes the back button floating by default.
Screenshots / Videos
Summary by CodeRabbit
StreamScaffoldwith built-in support for floating and regular app bar/bottom layoutsStreamAppBarfloating mode with gradient fade over contentStreamBottomNavBar/StreamBottomNavBarItemwith regular docked and floating pill stylesisFloating(including theme-level controls for consistent behavior)