feat(ui): add StreamScaffold with floating app-bar / bottom-bar support - #146
feat(ui): add StreamScaffold with floating app-bar / bottom-bar support#146xsahil03x wants to merge 59 commits into
Conversation
xsahil03x
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR standardizes toolbar behavior on ChangesToolbar contracts and styles
Scaffold, media, and validation
Platform integration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_core_flutter/lib/src/components/scaffold/stream_scaffold.dart (1)
274-281: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGive the body tight constraints so a shrink-wrapping body still fills the scaffold.
_BodyBoxConstraintssets onlymaxWidthandmaxHeight, so the body slot receives loose constraints. A body that shrink-wraps (for example aColumnwithmainAxisSize.min, or a bareText) then sizes to its intrinsic extent instead of the scaffold area, and is positioned atOffset.zerowith that smaller size. The regular (non-floating) path does not have this behaviour, becauseScaffoldgives its body tight constraints. The existing tests all use expanding bodies (SizedBox.expand,ListView), so the difference is not covered.Set the minimum extents as well to match
Scaffoldbody semantics.🐛 Proposed fix: tight body constraints
layoutChild( _Slot.body, _BodyBoxConstraints( + minWidth: size.width, maxWidth: size.width, + minHeight: size.height, maxHeight: size.height, bottomHeight: bottomHeight, ), );Also widen the forwarded constructor parameters:
class _BodyBoxConstraints extends BoxConstraints { const _BodyBoxConstraints({ + super.minWidth, super.maxWidth, + super.minHeight, super.maxHeight, required this.bottomHeight, }) : assert(bottomHeight >= 0, 'bottomHeight must be non-negative');🤖 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/scaffold/stream_scaffold.dart` around lines 274 - 281, Update the floating scaffold body layout around _BodyBoxConstraints to provide tight constraints by forwarding minimum width and height equal to the available size, alongside the existing maximum extents. Widen the relevant _BodyBoxConstraints constructor parameters as needed, while preserving bottomHeight handling and the existing layoutChild flow.
🧹 Nitpick comments (6)
packages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dart (5)
12-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding an Alchemist golden test for
StreamScaffold.The repository guideline for
packages/stream_core_flutter/test/components/**/*.dartasks for golden tests with Alchemist, stored ingoldens/ci/andgoldens/macos/, and taggedgolden. This file covers layout and inset behaviour only. A golden for the floating app-bar plus floating bottom-bar combination would lock in the visual result of the new layout path.As per coding guidelines: "Flutter components should have golden tests using Alchemist, with goldens stored in the component's
goldens/ci/andgoldens/macos/directories; tag golden tests withgolden."🤖 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/test/components/scaffold/stream_scaffold_test.dart` around lines 12 - 49, Add an Alchemist golden test to the StreamScaffold tests covering the floating app-bar and floating bottom-bar combination, using the repository’s established golden-test conventions. Store generated goldens under the component’s goldens/ci and goldens/macos directories and tag the test with golden; preserve the existing inset and layout tests.Source: Coding guidelines
573-575: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact bar position instead of an upper bound.
lessThanOrEqualTopasses for any position at or above the keyboard top, including a bar collapsed to the top of the screen. WhenresizeToAvoidBottomInsetis true, theScaffoldbody ends exactly at the keyboard top, so the bar bottom issurfaceHeight - keyboard. Use the samemoreOrLessEqualsstyle already used in theresizeToAvoidBottomInset: falsecase. The same applies to the regular-bottom assertion at Line 630.♻️ Proposed assertion
- expect(barBottom, lessThanOrEqualTo(surfaceHeight - keyboard + 0.5)); + expect(barBottom, moreOrLessEquals(surfaceHeight - keyboard, epsilon: 0.5));🤖 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/test/components/scaffold/stream_scaffold_test.dart` around lines 573 - 575, Update the keyboard-visible bar assertion in the resizeToAvoidBottomInset: true test to require barBottom moreOrLessEquals surfaceHeight - keyboard, matching the exact-position style used by the false case. Apply the same exact moreOrLessEquals assertion to the regular-bottom assertion near the later test assertion, replacing the permissive lessThanOrEqualTo checks while preserving the existing tolerance.
246-247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten this assertion and cover the bottom edge.
lessThan(_kBarHeight)passes for any value from 0 to 55. With a regular app bar theScaffoldconsumes the system top inset, sopadding.topis0. The test also configures a regularbottomof 64 withdevicePadding.bottomof 34 but asserts nothing aboutpadding.bottom, so this matrix cell does not verify the docked-bottom half of its own name.♻️ Proposed assertions
- // A regular app bar consumes the system top; nothing enlarges it. - expect(captured.padding!.top, lessThan(_kBarHeight)); + // A regular app bar consumes the system top; nothing enlarges it, and the + // docked bottom owns the home-indicator inset. + expect(captured.padding!.top, 0); + expect(captured.padding!.bottom, 0);🤖 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/test/components/scaffold/stream_scaffold_test.dart` around lines 246 - 247, In the regular app bar test case, replace the broad padding.top less-than assertion with an exact zero assertion, then add an assertion that captured.padding!.bottom equals the expected bottom inset behavior for a regular 64-height bottom with devicePadding.bottom of 34. Keep the existing scaffold configuration and verify both top and bottom edges in this matrix cell.
481-484: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the dependency on an external consumer in this comment.
The comment explains the scenario by referring to "the channel list's
bottomPadding > 0 ? … : null". That code is not instream_core_flutter, so a reader of this package cannot resolve the reference, and the comment goes stale if the consumer changes. Describe the pattern itself: a nested scrollable whose padding is conditional on a non-zero bottom inset re-consumes the injected top inset when the bottom inset is zero.🤖 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/test/components/scaffold/stream_scaffold_test.dart` around lines 481 - 484, Update the mixed-mode comment to remove the reference to the external channel list and its implementation. Describe the behavior generically: when a nested scrollable conditionally applies padding only for a non-zero bottom inset, a zero bottom inset causes it to re-consume the injected top inset, producing a gap after the header.
94-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the forwarding test to the newly added
Scaffoldproperties.This test asserts only
drawerandendDrawer. The PR forwards nine more values:onDrawerChanged,onEndDrawerChanged,drawerScrimColor,drawerEdgeDragWidth,drawerEnableOpenDragGesture,endDrawerEnableOpenDragGesture,drawerDragStartBehavior,drawerBarrierDismissible, andrestorationId. A missing or mis-wired forward inStreamScaffold.buildwould not fail any test. Assert the full set in one pass over the resolvedScaffold.🤖 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/test/components/scaffold/stream_scaffold_test.dart` around lines 94 - 111, Extend the test named “forwards drawer/endDrawer to the underlying Scaffold” to configure and assert all newly forwarded Scaffold properties: onDrawerChanged, onEndDrawerChanged, drawerScrimColor, drawerEdgeDragWidth, drawerEnableOpenDragGesture, endDrawerEnableOpenDragGesture, drawerDragStartBehavior, drawerBarrierDismissible, and restorationId. Resolve the underlying Scaffold once and verify each property matches the corresponding StreamScaffold input alongside drawer and endDrawer.apps/design_system_gallery/lib/components/scaffold/stream_scaffold.dart (1)
86-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a standard Widgetbook use-case name.
Drawersdoes not follow the gallery use-case naming convention. It also sorts beforePlaygroundin the generated directory. Rename this contextual example toReal-world Example, then regenerate the directory.
- apps/design_system_gallery/lib/components/scaffold/stream_scaffold.dart#L86-L90: Rename the
Drawersannotation value toReal-world Example.- apps/design_system_gallery/lib/app/gallery_app.directories.g.dart#L1144-L1149: Regenerate this file with
dart run build_runner build --delete-conflicting-outputs; do not edit it manually.As per coding guidelines, component use cases should use
Playground,Type/Size Variants, orReal-world Example.Proposed source change
- name: 'Drawers', + name: 'Real-world Example',🤖 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/scaffold/stream_scaffold.dart` around lines 86 - 90, Rename the Widgetbook use-case name in stream_scaffold.dart from Drawers to Real-world Example. Then regenerate apps/design_system_gallery/lib/app/gallery_app.directories.g.dart using dart run build_runner build --delete-conflicting-outputs; do not edit the generated file manually.Source: Coding guidelines
🤖 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/scaffold/stream_scaffold.dart`:
- Around line 270-272: Update the documentation comment for StreamBottomNavBar
so its measured height is described as becoming the body’s bottom inset only
when floating is true; clarify that regular docked bars remain below the body,
matching the StreamScaffold contract.
In
`@packages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dart`:
- Around line 450-452: Update the comment above the first-item gap assertion in
the scaffold test to state that the inner ListView injects a top padding/inset
of 56 pixels, matching _kBarHeight, rather than describing the gap as bar height
plus system-top.
---
Outside diff comments:
In
`@packages/stream_core_flutter/lib/src/components/scaffold/stream_scaffold.dart`:
- Around line 274-281: Update the floating scaffold body layout around
_BodyBoxConstraints to provide tight constraints by forwarding minimum width and
height equal to the available size, alongside the existing maximum extents.
Widen the relevant _BodyBoxConstraints constructor parameters as needed, while
preserving bottomHeight handling and the existing layoutChild flow.
---
Nitpick comments:
In `@apps/design_system_gallery/lib/components/scaffold/stream_scaffold.dart`:
- Around line 86-90: Rename the Widgetbook use-case name in stream_scaffold.dart
from Drawers to Real-world Example. Then regenerate
apps/design_system_gallery/lib/app/gallery_app.directories.g.dart using dart run
build_runner build --delete-conflicting-outputs; do not edit the generated file
manually.
In
`@packages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dart`:
- Around line 12-49: Add an Alchemist golden test to the StreamScaffold tests
covering the floating app-bar and floating bottom-bar combination, using the
repository’s established golden-test conventions. Store generated goldens under
the component’s goldens/ci and goldens/macos directories and tag the test with
golden; preserve the existing inset and layout tests.
- Around line 573-575: Update the keyboard-visible bar assertion in the
resizeToAvoidBottomInset: true test to require barBottom moreOrLessEquals
surfaceHeight - keyboard, matching the exact-position style used by the false
case. Apply the same exact moreOrLessEquals assertion to the regular-bottom
assertion near the later test assertion, replacing the permissive
lessThanOrEqualTo checks while preserving the existing tolerance.
- Around line 246-247: In the regular app bar test case, replace the broad
padding.top less-than assertion with an exact zero assertion, then add an
assertion that captured.padding!.bottom equals the expected bottom inset
behavior for a regular 64-height bottom with devicePadding.bottom of 34. Keep
the existing scaffold configuration and verify both top and bottom edges in this
matrix cell.
- Around line 481-484: Update the mixed-mode comment to remove the reference to
the external channel list and its implementation. Describe the behavior
generically: when a nested scrollable conditionally applies padding only for a
non-zero bottom inset, a zero bottom inset causes it to re-consume the injected
top inset, producing a gap after the header.
- Around line 94-111: Extend the test named “forwards drawer/endDrawer to the
underlying Scaffold” to configure and assert all newly forwarded Scaffold
properties: onDrawerChanged, onEndDrawerChanged, drawerScrimColor,
drawerEdgeDragWidth, drawerEnableOpenDragGesture,
endDrawerEnableOpenDragGesture, drawerDragStartBehavior,
drawerBarrierDismissible, and restorationId. Resolve the underlying Scaffold
once and verify each property matches the corresponding StreamScaffold input
alongside drawer and endDrawer.
🪄 Autofix
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 Plus
Run ID: 0efbffda-12f9-4913-b395-4d2523b69aaf
⛔ Files ignored due to path filters (1)
packages/stream_core_flutter/test/components/toolbar/goldens/ci/stream_bottom_nav_bar_floating.pngis excluded by!**/*.png
📒 Files selected for processing (6)
apps/design_system_gallery/lib/app/gallery_app.directories.g.dartapps/design_system_gallery/lib/components/scaffold/stream_scaffold.dartpackages/stream_core_flutter/CHANGELOG.mdpackages/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/test/components/scaffold/stream_scaffold_test.dart
ab3feea to
57d2fab
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
==========================================
+ Coverage 58.17% 59.49% +1.32%
==========================================
Files 187 190 +3
Lines 7615 7725 +110
==========================================
+ Hits 4430 4596 +166
+ Misses 3185 3129 -56 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
57d2fab to
df0d10c
Compare
… support Composes appBar/body/bottom slots and, when a bar floats, enlarges the body's MediaQuery.padding by the bar's height so standard scrollables and SafeArea auto-inset. Floors the floating StreamBottomNavBar pill margin so it never sits flush when the device reports no bottom inset. Adds a gallery use-case and an inset-injection test suite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
df0d10c to
229720f
Compare
| ), | ||
| ), | ||
| if (hasBottom) LayoutId(id: _Slot.bottom, child: bottom!), | ||
| LayoutId(id: _Slot.bottom, child: bottom!), |
There was a problem hiding this comment.
bottom can be null here right?
There was a problem hiding this comment.
It can be but we have an assertion which throws before we reach this point.
}) : assert(!floating || bottom != null, 'A floating body requires a bottom widget.');
…tton
Merge StreamAppBarBehavior and StreamBottomAppBarBehavior into a single
StreamToolbarBehavior { regular, floating } (breaking, unreleased). Each bar
publishes its resolved behaviour through a StreamToolbarScope so slot widgets
match the bar, and StreamToolbarButton adapts its outline/ghost look to that
scope.
Align the floating-fade solidFraction with the painted box height in all three
bars. Give StreamBottomNavBar a public kStreamBottomNavBarHeight (72), a single
SafeArea for its pill insets, and a themeable floatingElevation. Standardise
floating Material elevations on StreamElevation.level3 (avatar + nav bar).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/design_system_gallery/lib/components/media_viewer/stream_media_viewer.dart (1)
44-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a dropdown knob for the behavior enum.
The knob selects between
StreamToolbarBehavior.floatingandStreamToolbarBehavior.regular. Usecontext.knobs.object.dropdownwith the enum values so the control matches the modelled type and the gallery conventions.As per path instructions: "Use `context.knobs.object.dropdown` for enums".♻️ Proposed refactor
- final floating = context.knobs.boolean( - label: 'Floating chrome', - initialValue: true, + final behavior = context.knobs.object.dropdown( + label: 'Chrome behavior', + options: StreamToolbarBehavior.values, + initialOption: StreamToolbarBehavior.floating, description: 'Floating chrome overlays full-bleed media with a gradient fade; ' 'regular chrome insets the media between opaque bars.', );Then pass
behaviordirectly to both styles instead offloating ? .floating : .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/media_viewer/stream_media_viewer.dart` around lines 44 - 50, Replace the boolean floating knob with a context.knobs.object.dropdown control using both StreamToolbarBehavior.floating and StreamToolbarBehavior.regular as enum options. Store the selected behavior and pass it directly to both relevant styles, removing the floating conditional conversion.Source: Path instructions
packages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dart (1)
107-118: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚖️ Poor tradeoffAddress the remaining
StreamBottomNavBarStyle.behaviorusages.
packages/stream_core_flutter/CHANGELOG.mddocuments the public migration. The only remaining direct uses live in generated Widgetbook factory code; regenerate the Widgetbook directory after the publicStreamBottomNavBarProps/DefaultStreamBottomNavBarAPI is migrated.🤖 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 107 - 118, Regenerate the Widgetbook directory after migrating the public StreamBottomNavBarProps and DefaultStreamBottomNavBar APIs, removing all remaining direct StreamBottomNavBarStyle.behavior usages from generated Widgetbook factory code. Ensure the regenerated factories use the migrated API while preserving existing toolbar behavior.
🤖 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/scaffold/stream_scaffold.dart`:
- Around line 86-90: Update the Widgetbook use-case annotation for
StreamScaffold to rename the use case from “Drawers” to the required “Real-world
Example” name, leaving the type and path unchanged.
In
`@apps/design_system_gallery/lib/components/toolbar/stream_bottom_app_bar.dart`:
- Line 106: Update the floating toolbar margin in the preview content to use the
Figma-defined context.streamSpacing token instead of the hardcoded horizontal
value 32, preserving the existing symmetric horizontal margin behavior.
In
`@packages/stream_core_flutter/lib/src/components/media_viewer/stream_media_viewer.dart`:
- Around line 149-163: Update DefaultStreamMediaViewer’s headerFloating and
footerFloating resolution to use the same merged bar styles as
DefaultStreamAppBar and DefaultStreamBottomAppBar, merging each ambient bar
theme style with the corresponding local props.style before reading
behavior.isFloating. Preserve the StreamAppStyle fallback only when the merged
bar style has no pinned behavior, and keep the existing inset calculations
unchanged.
In
`@packages/stream_core_flutter/lib/src/components/scaffold/stream_scaffold.dart`:
- Around line 195-199: Update the behavior resolution in the scaffold’s app-bar
and bottom-bar flow to account for per-instance bar styles, or explicitly
document that `StreamAppBarStyle`/bottom-bar instance styles with a pinned
`behavior` require matching `appBarBehavior`/`bottomBarBehavior` values on the
scaffold. Ensure the behavior used for `extendBodyBehindAppBar` and published
insets matches the behavior rendered by each bar.
---
Nitpick comments:
In
`@apps/design_system_gallery/lib/components/media_viewer/stream_media_viewer.dart`:
- Around line 44-50: Replace the boolean floating knob with a
context.knobs.object.dropdown control using both StreamToolbarBehavior.floating
and StreamToolbarBehavior.regular as enum options. Store the selected behavior
and pass it directly to both relevant styles, removing the floating conditional
conversion.
In
`@packages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dart`:
- Around line 107-118: Regenerate the Widgetbook directory after migrating the
public StreamBottomNavBarProps and DefaultStreamBottomNavBar APIs, removing all
remaining direct StreamBottomNavBarStyle.behavior usages from generated
Widgetbook factory code. Ensure the regenerated factories use the migrated API
while preserving existing toolbar behavior.
🪄 Autofix
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 Plus
Run ID: 10523aa0-44e2-4d5f-8ffb-550101886e96
⛔ Files ignored due to path filters (7)
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_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/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 (32)
apps/design_system_gallery/lib/app/gallery_app.directories.g.dartapps/design_system_gallery/lib/components/media_viewer/stream_media_viewer.dartapps/design_system_gallery/lib/components/scaffold/stream_scaffold.dartapps/design_system_gallery/lib/components/toolbar/stream_app_bar.dartapps/design_system_gallery/lib/components/toolbar/stream_bottom_app_bar.dartapps/design_system_gallery/lib/components/toolbar/stream_bottom_nav_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/media_viewer/stream_media_viewer.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_app_bar.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_toolbar_button.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_toolbar_scope.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_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/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/components/stream_toolbar_behavior.dartpackages/stream_core_flutter/lib/src/theme/stream_app_style.dartpackages/stream_core_flutter/test/components/media_viewer/stream_media_viewer_test.dartpackages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_app_bar_golden_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_app_bar_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_bottom_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.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/design_system_gallery/lib/app/gallery_app.directories.g.dart
- packages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dart
…rough generated code StreamColorScheme.brightness (added in #131) was never regenerated into the theme extension, so the generated copyWith, lerp, merge, ==, and hashCode silently dropped it — copyWith(brightness:) was impossible, a light/dark lerp resolved brightness to the default, merge lost it, and two schemes differing only in brightness compared equal. Regenerate to sync. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
renefloor
left a comment
renefloor
left a comment
There was a problem hiding this comment.
Reviewed with a fan-out of specialized checks (general correctness, silent-failure/error-handling, test coverage, type design, comment accuracy). Summary below; inline comments point at the specific lines.
Critical: StreamScaffold resolves floating/regular behaviour for a slot from a different source than the bar/nav-bar widget itself resolves from, so the two can silently disagree — see the inline comment on stream_scaffold.dart. This affects both a StreamAppBar/StreamBottomAppBar with a per-instance style and, more concretely, StreamBottomNavBar (which is resolved from an entirely different theme/enum that the scaffold never reads at all).
Other notable gaps not tied to one line:
StreamToolbarButtonandStreamToolbarScope(new public API) have no dedicated test file — only indirect coverage via the app bar's auto-implied back button.StreamBottomAppBar's new floating visual mode has no golden test, and the bottom-nav-bar pill-margin fix (the headline bug fix in this PR) has no unit assertion on the margin math, only golden images at zero device inset.- Nice test coverage overall on the scaffold's inset-injection matrix and the "documented surprises" regression group — that pattern is worth extending to pin the behaviour-mismatch issue above.
Nothing here should block merge outright if the team's comfortable following up, but I'd suggest at least fixing the critical resolution mismatch or adding a debug-mode assertion that surfaces it loudly, since StreamScaffold + StreamBottomNavBar is the documented, expected combination.
| final appStyle = context.streamTheme.appStyle; | ||
| final effectiveStreamAppBarBehavior = | ||
| appBarBehavior ?? | ||
| context.streamAppBarTheme.style?.behavior ?? | ||
| (appStyle.isFloating ? StreamAppBarBehavior.floating : StreamAppBarBehavior.regular); | ||
| final effectiveStreamBottomAppBarBehavior = | ||
| bottomBarBehavior ?? | ||
| context.streamBottomAppBarTheme.style?.behavior ?? | ||
| (appStyle.isFloating ? StreamBottomAppBarBehavior.floating : StreamBottomAppBarBehavior.regular); | ||
| final effectiveBackgroundColor = backgroundColor ?? context.streamColorScheme.backgroundApp; | ||
|
|
||
| final appBarFloating = effectiveStreamAppBarBehavior == StreamAppBarBehavior.floating; | ||
| final bottomFloating = effectiveStreamBottomAppBarBehavior == StreamBottomAppBarBehavior.floating && bottom != null; | ||
|
|
||
| final topInset = appBarFloating ? (appBar?.preferredSize.height ?? 0) + MediaQuery.paddingOf(context).top : 0.0; | ||
|
|
||
| // When neither slot is floating, use a plain Scaffold for maximum | ||
| // compatibility (e.g. keyboard avoidance, Scaffold.of, etc.). | ||
| // The bottom widget lives inside the body Column (not bottomNavigationBar) | ||
| // because bottomNavigationBar is not repositioned above the keyboard on | ||
| // Android, which causes text-input composers to be hidden behind the IME. | ||
| if (!appBarFloating && !bottomFloating) { | ||
| return Scaffold( | ||
| backgroundColor: effectiveBackgroundColor, | ||
| resizeToAvoidBottomInset: resizeToAvoidBottomInset, | ||
| appBar: appBar, | ||
| drawer: drawer, | ||
| endDrawer: endDrawer, | ||
| body: Column( | ||
| children: [ | ||
| Expanded( | ||
| child: StreamScaffoldInsets( | ||
| topPadding: 0, | ||
| bottomPadding: 0, | ||
| child: body, | ||
| ), | ||
| ), | ||
| ?bottom, | ||
| ], | ||
| ), | ||
| ); | ||
| } | ||
| final appBarStyle = context.streamAppBarTheme.style; | ||
| final bottomAppBarStyle = context.streamBottomAppBarTheme.style; | ||
|
|
||
| var effectiveAppBarBehavior = appBarBehavior ?? appBarStyle?.behavior; | ||
| effectiveAppBarBehavior ??= appStyle.isFloating ? .floating : .regular; | ||
|
|
||
| var effectiveBottomBarBehavior = bottomBarBehavior ?? bottomAppBarStyle?.behavior; | ||
| effectiveBottomBarBehavior ??= appStyle.isFloating ? .floating : .regular; | ||
|
|
||
| final effectiveBackgroundColor = backgroundColor ?? colorScheme.backgroundApp; | ||
|
|
||
| final appBarFloating = effectiveAppBarBehavior == .floating; | ||
| final bottomFloating = effectiveBottomBarBehavior == .floating && bottom != null; |
There was a problem hiding this comment.
Critical — resolved behaviour can diverge from what the bar/nav-bar actually renders.
This only consults the ambient StreamAppBarTheme/StreamBottomAppBarTheme (appBarStyle/bottomAppBarStyle above) and the global StreamAppStyle. It never looks at a style passed directly to the appBar:/bottom: widget instance — but DefaultStreamAppBar and StreamBottomAppBar both let their own props.style win over the ambient theme (style.merge(...) pattern). So e.g.
StreamScaffold(
appBar: StreamAppBar(style: StreamAppBarStyle(behavior: .floating)),
body: ListView(...),
)with no ambient override renders a floating (translucent/gradient) bar while the scaffold computes .regular here — extendBodyBehindAppBar: false and no top inset injected. The bar ends up in a non-extended slot with floating-style chrome fading into nothing.
More concretely: the documented, gallery-recommended bottom widget is StreamBottomNavBar, but it resolves its own floating look from context.streamBottomNavBarTheme / StreamBottomNavBarBehavior — a completely different theme and enum than bottomAppBarStyle here. Wrapping a StreamBottomNavBarTheme(style: behavior: .floating) around this scaffold makes the pill render floating while this code still computes .regular (no ambient StreamBottomAppBarTheme, non-floating StreamAppStyle) — wrong insets, no overlap, and the pill's own margin fix loses its paired layout. This combination isn't tested anywhere (no test builds a StreamBottomNavBar inside StreamScaffold).
Suggest making propagation one-directional (scaffold resolves once and pushes the result down via the ambient themes) rather than each side resolving independently from divergent sources, and unifying StreamBottomNavBarBehavior into the shared StreamToolbarBehavior so there's one source of truth for the bottom slot.
There was a problem hiding this comment.
@xsahil03x should we maybe unify the themes for the behavior to make this scaffold work?
| static StreamToolbarBehavior of(BuildContext context) { | ||
| final scope = context.dependOnInheritedWidgetOfExactType<StreamToolbarScope>(); | ||
| assert(scope != null, 'StreamToolbarScope.of() called outside a Stream toolbar.'); | ||
| return scope!.behavior; | ||
| } |
There was a problem hiding this comment.
In a release build (asserts stripped), calling of() outside a toolbar throws a bare null-check TypeError instead of the descriptive message here. Since StreamToolbarButton (public API, exported from core.dart) calls this asserting variant, dropping it into a slot that doesn't publish a scope — e.g. StreamSheetHeader, which is another toolbar-height widget in this same directory but doesn't wrap its slots in StreamToolbarScope — is an easy crash for downstream SDKs with a confusing error.
Consider having StreamToolbarButton use maybeOf(context) ?? StreamToolbarBehavior.regular instead, degrading to the docked look rather than crashing.
There was a problem hiding this comment.
Fixed in 2beebd3. StreamToolbarScope.of() now throws a descriptive FlutterError (same shape as StreamChat.of / Scaffold.of) instead of a bare null-check. And StreamToolbarButton now reads via maybeOf(context) ?? .regular, so it degrades to the docked style outside a toolbar rather than crashing — with a debug assert to flag the likely misuse (e.g. dropping it into StreamSheetHeader). Added stream_toolbar_button_test.dart covering the scope contract and the button styling in both modes.
| /// carry this value per component. | ||
| /// * [StreamToolbarScope], which publishes the resolved value to a bar's slots. | ||
| /// * [StreamAppStyle], the global app-wide style that acts as fallback. | ||
| enum StreamToolbarBehavior { |
There was a problem hiding this comment.
This is structurally identical to the pre-existing StreamBottomNavBarBehavior (stream_bottom_nav_bar_theme.dart:18-24) — same two-value "docked vs floating" concept, just scoped to a different component. Every call site converts between the two via the same appStyle.isFloating ? .floating : .regular idiom.
This PR was a natural point to unify them (it already touches StreamBottomNavBarTheme to add floatingElevation), and not doing so is part of why the nav-bar case in stream_scaffold.dart falls through the cracks — StreamScaffold has no single enum/theme it can check for "is the bottom slot floating" across both bar types. Worth a follow-up to rename StreamBottomNavBarBehavior → this enum and drop the duplicate before a third toolbar-shaped component copies the pattern again.
There was a problem hiding this comment.
Done in c49bf29. Since StreamBottomNavBarBehavior is unreleased (not in v0.4.1), replaced it outright with StreamToolbarBehavior — no deprecation needed. StreamBottomNavBarStyle.behavior now uses the shared enum, so a toolbar-shaped component's floating state has one source of truth. Regenerated the theme; gallery and CHANGELOG updated.
| // Resolve the chrome's floating state the same way the bars (and | ||
| // StreamScaffold) do, so the media extends full-bleed behind floating | ||
| // chrome and is inset under docked (regular) chrome. Falls back to the | ||
| // ambient StreamAppStyle when the chrome style doesn't pin a behavior. | ||
| final fallbackFloating = context.streamTheme.appStyle.isFloating; | ||
| final headerFloating = effectiveAppBarStyle?.behavior?.isFloating ?? fallbackFloating; | ||
| final footerFloating = effectiveBottomAppBarStyle?.behavior?.isFloating ?? fallbackFloating; |
There was a problem hiding this comment.
"the same way the bars ... do" isn't quite accurate: effectiveAppBarStyle/effectiveBottomAppBarStyle here only come from the media-viewer's theme (theme.appBarStyle) and the ambient StreamAppStyle fallback — never from props.header's/props.footer's own per-instance style. But DefaultStreamAppBar itself does let a per-instance style.behavior override win over its ambient theme.
Concretely: header: StreamAppBar(style: StreamAppBarStyle(behavior: .regular)) while the media-viewer theme/ambient style says floating — the bar correctly renders opaque/regular, but headerInset still computes 0.0 (floating branch), so the media renders full-bleed underneath an opaque header. Worth narrowing the comment to say it mirrors only the theme/app-style fallback levels, not any per-instance override on the header/footer widget itself.
There was a problem hiding this comment.
Fixed in 2beebd3 — narrowed the comment to say it mirrors only the theme/app-style fallback levels, not a per-instance style on the header/footer widget itself.
| /// [appBar] (e.g. `StreamChannelListHeader`) can find the drawer via | ||
| /// `Scaffold.maybeOf(context)?.openDrawer()`. | ||
| /// Provide a [drawer] and/or [endDrawer] to add slide-in side panels; a widget | ||
| /// in [appBar] can open them (as `StreamChannelListHeader` does for its menu). |
There was a problem hiding this comment.
StreamChannelListHeader doesn't exist anywhere in this repo (it's written in plain backticks rather than a dartdoc [...] link, which suggests it wasn't expected to resolve). Either point at a component that actually exists here, or make clear this is an example from an external SDK (e.g. stream_chat_flutter).
There was a problem hiding this comment.
Fixed in 2beebd3 — dropped the non-existent StreamChannelListHeader reference. The doc now points at Scaffold.of(context).openDrawer() and frames it as an example from an external chat SDK.
| drawerDragStartBehavior: drawerDragStartBehavior, | ||
| drawerBarrierDismissible: drawerBarrierDismissible, | ||
| extendBodyBehindAppBar: appBarFloating, | ||
| extendBody: bottomFloating, |
There was a problem hiding this comment.
extendBody: bottomFloating looks like a no-op: Scaffold.bottomNavigationBar/persistentFooterButtons are never set on this Scaffold, so Flutter's own extendBody handling has nothing to extend around — the actual bottom-slot overlap is done entirely by _StreamScaffoldBody below. Worth double-checking whether this line does anything, since a test currently asserts on it (stream_scaffold_test.dart), which cements a possibly-misleading signal if it's dead.
There was a problem hiding this comment.
Good catch — confirmed it was dead. extendBody only affects a Scaffold that sets bottomNavigationBar / persistentFooterButtons, and this one sets neither (the bottom bar lives in the body), so the flag did nothing; the overlap is all _StreamScaffoldBody. Removed it in 69feed0, and re-pointed the two tests that asserted on it at observable outcomes instead (the bottomNavigationBar slot stays empty; no bottom inset is injected when there's no bottom widget). The full inset/layout matrix still passes unchanged, which confirms it was inert.
| // The gradient spans the whole chrome — top margin, pill, and bottom inset. | ||
| // Keep it solid across the bottom inset and fade up through the pill into | ||
| // the content behind the bar. | ||
| final totalHeight = topInset + kStreamBottomNavBarHeight + bottomInset; |
There was a problem hiding this comment.
This reads as an exact geometric description, but kStreamBottomNavBarHeight is only a minHeight on the tiles' ConstrainedBox, not the guaranteed rendered height. A pre-diff version of this code had an explicit "approximate" caveat that seems to have been dropped in the rewrite. A long/wrapped localized label pushing a tile taller than the constant would make solidFraction under-represent the real bottom-inset proportion, drifting the gradient's fade boundary off the pill edge — probably fine in practice, but worth restoring the caveat so a future reader doesn't treat this as exact.
There was a problem hiding this comment.
Fixed in 2beebd3 — restored the "approximate" caveat noting kStreamBottomNavBarHeight is the tiles' ConstrainedBox minHeight, not the guaranteed rendered height, so a taller wrapped label drifts the fade boundary slightly.
- StreamToolbarButton: read the scope via maybeOf and degrade to the docked style outside a toolbar (assert flags the misuse in debug) instead of crashing on a hard StreamToolbarScope.of(). - StreamToolbarScope.of(): throw a descriptive FlutterError.fromParts when no scope is in context, matching Flutter's *.of() convention. - StreamScaffold: give the body tight min/max constraints so a shrink- wrapping child fills the slot; clarify the behavior-resolution and drawer docs. - StreamBottomNavBar / media viewer: doc-only clarifications. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cessor - kStreamBottomNavBarHeight 72 -> 64 per the design system. - Regenerate the CI bottom-nav-bar goldens for the new height. - Fix StreamToolbarButton test to read type/isFloating via .props (the committed accessor did not compile). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/stream_core_flutter/test/components/toolbar/stream_toolbar_button_test.dart (2)
34-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the complete no-scope toolbar contract.
Lines 34-47 only verify the
FlutterErrortype. Verify that the error identifiesStreamToolbarScope.
Lines 72-102 do not verify theStreamToolbarButtonfallback outside a scope. Add a test that expects the regular, ghost, non-floating button in that case.Also applies to: 72-102
🤖 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/test/components/toolbar/stream_toolbar_button_test.dart` around lines 34 - 47, The no-scope toolbar tests do not verify the full error and fallback behavior. Update the test for StreamToolbarScope.of to assert that the FlutterError identifies StreamToolbarScope, and add coverage for StreamToolbarButton outside a scope confirming it renders the regular, ghost, non-floating button variant.
72-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd golden coverage for regular and floating toolbar buttons.
These tests inspect
StreamButton.props. They cannot detect rendering regressions in the outlined floating and ghost docked variants. Add tagged Alchemist golden tests and regenerate their CI and macOS goldens.As per coding guidelines, Flutter components must have Alchemist golden tests tagged
golden, and visual changes require golden regeneration.🤖 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/test/components/toolbar/stream_toolbar_button_test.dart` around lines 72 - 102, Add Alchemist golden widget tests for both StreamToolbarButton variants in the StreamToolbarButton test group, tagging each test with golden and capturing the outlined floating and ghost docked renderings rather than only inspecting StreamButton.props. Regenerate and commit the corresponding CI and macOS golden artifacts.Source: Coding guidelines
🤖 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/test/components/toolbar/stream_toolbar_button_test.dart`:
- Around line 34-47: The no-scope toolbar tests do not verify the full error and
fallback behavior. Update the test for StreamToolbarScope.of to assert that the
FlutterError identifies StreamToolbarScope, and add coverage for
StreamToolbarButton outside a scope confirming it renders the regular, ghost,
non-floating button variant.
- Around line 72-102: Add Alchemist golden widget tests for both
StreamToolbarButton variants in the StreamToolbarButton test group, tagging each
test with golden and capturing the outlined floating and ghost docked renderings
rather than only inspecting StreamButton.props. Regenerate and commit the
corresponding CI and macOS golden artifacts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 048df629-38e2-4d27-af0c-c35d28b16e84
⛔ 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 (18)
apps/design_system_gallery/lib/app/gallery_app.directories.g.dartapps/design_system_gallery/lib/components/media_viewer/stream_media_viewer.dartapps/design_system_gallery/lib/components/scaffold/stream_scaffold.dartapps/design_system_gallery/lib/components/toolbar/stream_bottom_app_bar.dartapps/design_system_gallery/macos/Runner.xcodeproj/project.pbxprojapps/design_system_gallery/macos/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcschemepackages/stream_core_flutter/CHANGELOG.mdpackages/stream_core_flutter/lib/src/components/media_viewer/stream_media_viewer.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/components/toolbar/stream_toolbar_button.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_toolbar_scope.dartpackages/stream_core_flutter/lib/src/theme/semantics/stream_color_scheme.g.theme.dartpackages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_toolbar_button_test.dartpackages/stream_thumbnail/example/linux/flutter/generated_plugin_registrant.ccpackages/stream_thumbnail/example/linux/flutter/generated_plugin_registrant.hpackages/stream_thumbnail/example/linux/flutter/generated_plugins.cmake
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/stream_core_flutter/CHANGELOG.md
- apps/design_system_gallery/lib/app/gallery_app.directories.g.dart
- apps/design_system_gallery/lib/components/media_viewer/stream_media_viewer.dart
- packages/stream_core_flutter/lib/src/components/toolbar/stream_toolbar_button.dart
- apps/design_system_gallery/lib/components/toolbar/stream_bottom_app_bar.dart
- packages/stream_core_flutter/lib/src/components/media_viewer/stream_media_viewer.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
- apps/design_system_gallery/lib/components/scaffold/stream_scaffold.dart
- packages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dart
Replace imperative phrasing with the "Consider" / non-imperative form the style guide calls for in dartdoc prose. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop how-it-resolves details from StreamToolbarScope's public dartdoc and reword the toolbar-button knob note in contract terms. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ssertions - Golden coverage for the new floating StreamBottomAppBar chrome (gradient fade, no border) in light/dark. - Unit assertions on the floating StreamBottomNavBar pill margin: floors the bottom gap at spacing.xl with no device inset, grows with the inset otherwise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
extendBody only affects a Scaffold that sets bottomNavigationBar / persistentFooterButtons; this scaffold sets neither (the bottom bar lives in the body), so the flag did nothing — the body-behind-bar overlap is done by _StreamScaffoldBody. Removing it, and re-pointing the two tests that asserted on the flag at observable outcomes (empty bottomNavigationBar slot; no bottom inset injected without a bottom widget). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Removed 9 tests that assert framework behavior, are trivially true, or duplicate existing coverage (cross-checked via three independent reviews): - scaffold: the "documented surprises" group (4 tests exercising Flutter's MediaQuery-consumption and downstream consumer padding patterns, not our contract) and one redundant bottomNavigationBar-null cell. - nav bar: a BottomNavigationBar-absence assert (trivially true in both chromes) and a floating onTap test duplicating the shared-tile path. - bottom app bar: a title-only render whose load-bearing assert cannot fail. - app bar: a heading-default semantics test that is a strict subset of the Android namesRoute test. Also drop the stray extendBody comment. Suites stay green (99 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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/test/components/toolbar/stream_bottom_nav_bar_test.dart`:
- Around line 249-252: Update gapBelowPill to measure the aligned nav-bar render
box rather than the StreamBottomNavBar widget. Add a unique Key to the Align
wrapping the nav bar, create or reuse a finder for that key, and pass it to
tester.getRect when calculating barBottom; leave the pill measurement unchanged.
🪄 Autofix
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 Plus
Run ID: 675ce64b-1665-4cd8-846e-1a05335adfcd
⛔ Files ignored due to path filters (2)
packages/stream_core_flutter/test/components/toolbar/goldens/ci/stream_bottom_app_bar_floating.pngis excluded by!**/*.pngpackages/stream_core_flutter/test/components/toolbar/goldens/ci/stream_bottom_app_bar_regular.pngis excluded by!**/*.png
📒 Files selected for processing (8)
packages/stream_core_flutter/lib/src/components/scaffold/stream_scaffold.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_toolbar_button.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_toolbar_scope.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_app_bar_golden_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_bottom_app_bar_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_bottom_nav_bar_test.dart
💤 Files with no reviewable changes (2)
- packages/stream_core_flutter/test/components/toolbar/stream_bottom_app_bar_test.dart
- packages/stream_core_flutter/test/components/toolbar/stream_app_bar_test.dart
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/stream_core_flutter/lib/src/components/toolbar/stream_toolbar_scope.dart
- packages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dart
- packages/stream_core_flutter/lib/src/components/scaffold/stream_scaffold.dart
…rBehavior StreamBottomNavBarBehavior duplicated StreamToolbarBehavior (same regular / floating). Replace it with the shared enum so a toolbar-shaped component's floating state has one source of truth. Unreleased API, so no deprecation. Regenerated the theme; gallery and CHANGELOG updated (also corrects the stale nav-bar height in the changelog to 64). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…reamSurfaceStyle
Both were structurally identical {regular, floating} enums — the app-wide
default and the per-component override — which forced constant conversions
between them. Merge into one StreamSurfaceStyle used at both scopes:
- StreamTheme.appStyle and every component's `behavior` field share the type.
- Drops the `isFloating ? .floating : .regular` conversion boilerplate.
- Named for the surface (not "toolbar"/"app"): triangulated with Material 3
(docked/floating toolbars) and iOS 26 (inline/expanded placement + a separate
Glass material), leaving room for a future material axis (e.g. liquid glass)
as its own property rather than a value here.
Unreleased API, so no deprecation. Regenerated themes; tests, gallery, and
CHANGELOG updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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/theme/stream_surface_style.dart`:
- Around line 1-32: Document the StreamAppStyle to StreamSurfaceStyle rename in
CHANGELOG.md with migration guidance for existing consumers, or add a deprecated
StreamAppStyle compatibility alias that forwards to StreamSurfaceStyle. Preserve
the single export of StreamSurfaceStyle from core.dart and ensure the old public
API remains discoverable through either documentation or compatibility support.
🪄 Autofix
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 Plus
Run ID: e62718be-74c2-4162-9a57-670d038ac244
📒 Files selected for processing (30)
apps/design_system_gallery/lib/components/media_viewer/stream_media_viewer.dartapps/design_system_gallery/lib/components/scaffold/stream_scaffold.dartapps/design_system_gallery/lib/components/toolbar/stream_app_bar.dartapps/design_system_gallery/lib/components/toolbar/stream_bottom_app_bar.dartpackages/stream_core_flutter/CHANGELOG.mdpackages/stream_core_flutter/lib/core.dartpackages/stream_core_flutter/lib/src/components/media_viewer/stream_media_viewer.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_app_bar.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dartpackages/stream_core_flutter/lib/src/components/toolbar/stream_toolbar_scope.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_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/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_surface_style.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/media_viewer/stream_media_viewer_test.dartpackages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_app_bar_golden_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_app_bar_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_bottom_app_bar_golden_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_bottom_app_bar_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_bottom_nav_bar_test.dartpackages/stream_core_flutter/test/components/toolbar/stream_toolbar_button_test.dartpackages/stream_core_flutter/test/theme/stream_theme_test.dart
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/stream_core_flutter/lib/src/theme/components/stream_app_bar_theme.g.theme.dart
- packages/stream_core_flutter/lib/core.dart
- packages/stream_core_flutter/test/components/toolbar/stream_bottom_app_bar_golden_test.dart
- packages/stream_core_flutter/lib/src/components/media_viewer/stream_media_viewer.dart
- apps/design_system_gallery/lib/components/toolbar/stream_bottom_app_bar.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
- apps/design_system_gallery/lib/components/scaffold/stream_scaffold.dart
- packages/stream_core_flutter/test/components/toolbar/stream_toolbar_button_test.dart
- packages/stream_core_flutter/test/components/toolbar/stream_bottom_app_bar_test.dart
- packages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_app_bar.dart
- apps/design_system_gallery/lib/components/media_viewer/stream_media_viewer.dart
- packages/stream_core_flutter/test/components/toolbar/stream_app_bar_test.dart
- packages/stream_core_flutter/test/components/media_viewer/stream_media_viewer_test.dart
- packages/stream_core_flutter/test/components/scaffold/stream_scaffold_test.dart
- packages/stream_core_flutter/lib/src/components/toolbar/stream_app_bar.dart
- packages/stream_core_flutter/lib/src/components/toolbar/stream_bottom_nav_bar.dart
Make StreamSafeArea a general, app-agnostic SafeArea + margin widget: maintainBottomViewPadding is now a parameter (on the widget and resolveInsets), defaulting to true so the floating case works out of the box while any app can flip it to SafeArea's standard behaviour. No Stream-specific dependencies — the margin value stays at the call site. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rewrite the doc comments to describe observable behaviour (a fixed margin kept beyond the system insets, keyboard-stable bottom) instead of the implementation (no "composes SafeArea", inner Padding, or which MediaQuery field is read), and add a usage snippet. Replace the two pill-margin tests with a matrix over the real bottom insets — iOS no-inset (0) and home indicator (34), Android gesture (24) and 2-/3-button (48) — asserting the pill floats spacing.xl above each. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Match SafeArea's dartdoc shape — "A widget that insets its child…" opener,
a one-line note on the margin, a {@tool snippet}, a "### MediaQuery impact"
section, and a See also list — with terse field docs in SafeArea's voice
("Whether to avoid system intrusions on the left"). Reorder the edge flags
to left/top/right/bottom to match SafeArea too.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Prove the widget is direction-agnostic — SafeArea's physical left/right plus a physical EdgeInsets margin, so an asymmetric inset lands on the same physical edges in LTR and RTL (no swap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pill floated spacing.xl (24) above the bottom system inset, which sat too high. Drop the bottom margin to spacing.xs (8) while keeping the sides/top at xl, so it hugs the inset more closely. Regenerates the floating CI golden and updates the navigation-mode matrix test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Match the Figma spec: the pill's bottom gap is now max(systemInset, 2xl) — a 32 floor that a larger inset absorbs, landing in a 32→48 band across iOS (34 portrait) and Android (24 gesture, 48 button nav) instead of the too-tight inset+8. Adds a `minimum` (floor) param to StreamSafeArea and resolveInsets — each edge is now max(systemInset, minimum) + margin — and switches the nav bar to a minimum-only floor. Regenerates the CI golden. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the bottom floor with max(inset, xl) + xs: a uniform spacing.xs (8) above whatever the system reserves, floored so small-inset devices land on spacing.xxl (24 + 8 = 32, matching Figma). The pill no longer touches the opaque 3-button bar (48 -> 56) and never over-floats, with no fragile gesture-vs-button detection. StreamSafeArea is unchanged — this just uses its existing minimum (floor) + margin (beyond-inset). Golden unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the spacing.xs bottom gap conditionally — only when the bottom inset looks like a tappable 2-/3-button bar (>= 40), which fills its whole extent. Thin overlays (gesture ~24, iOS indicator ~34) already have space around them and sit flush. Flutter exposes no nav-mode API, so this keys off the inset height (40 cleanly splits the tallest overlay from a button bar). Gaps: gesture 24, iOS 34, 3-button 56. Regenerates the CI golden (no-inset pill drops from 32 to 24 — no opaque bar there). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The inset >= 40 threshold was a hack (Flutter exposes no nav-mode signal; systemGestureInsets is unreliable on iOS). Material sidesteps this by docking its nav bar rather than floating it, so there's no clean precedent. Go back to a uniform max(inset, xl) + xs: the pill floats a consistent spacing.xs above whatever the system reserves — never flush, never guessing. Restores the no-inset golden to 32. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Describe the behavioural difference (insets only to the safe area, no margin beyond) instead of "which this builds on". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A named constructor (Flutter's idiom for "same widget, alternate mode", like ListView.builder) that lerps the resolved inset between full (0) and nothing (1) by an Animation<double>. Lets a floating surface release its safe area full-bleed as a panel slides in beneath it, while keeping the same StreamSafeArea call shape as the static/nav-bar usage. The default constructor is unchanged (SafeArea-based, consumes); collapsing applies a lerped Padding and does not consume. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
….driven Rework the collapsing constructor into a publishable, generic one: - StreamSafeArea.driven (not .collapsing/.lerp — .lerp reads as a static interpolation fn, not a widget constructor). - Driven by any ValueListenable<double>, not just Animation<double>. - Interpolates toward a configurable `to` target (default EdgeInsets.zero), not hardcoded zero. At 0 the full inset applies; at 1 it is `to`. Values clamp to [0, 1]. Rebuilds via ValueListenableBuilder. Test covers the default (collapse to zero) and a non-zero target. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The driven path only applied a Padding, leaving the child's MediaQuery untouched — so it didn't consume the inset like the default (SafeArea) path, and a nested safe area could double-apply it. Wrap the child in MediaQuery.removePadding on the avoided edges (binary, independent of the interpolation), matching the default. Adds a test that a descendant sees the bottom inset removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nits Review follow-ups: - Note the default behaves like SafeArea *with maintainBottomViewPadding on* (SafeArea's default is off), so the bottom diverges under a keyboard. - Generalize the appliedInsets helper comment (driven uses Padding, not SafeArea). - Add a test that .driven rebuilds when a live ValueListenable changes (was only ever pumped with fresh AlwaysStoppedAnimation). - Add a default-constructor consume test to mirror the .driven one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both constructors now apply resolveInsets via Padding and consume via MediaQuery.removePadding — the default no longer composes Flutter's SafeArea. This removes the two-implementation split (SafeArea for the static path, hand-rolled for the driven path, which could drift) and makes resolveInsets the single source of truth, so default == driven-at-0 by construction. Behaviour is identical (SafeArea is Padding(max(inset, minimum)) + removePadding); nav-bar tests + golden unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… value Clarify that resolveInsets is the endpoint StreamSafeArea.driven interpolates from, so it isn't mistaken for the current interpolated inset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cover the cases Flutter's SafeArea tests that we were missing: - add debugFillProperties (edge flags + minimum/margin/maintainBottomViewPadding + driven listenable/to) and a test asserting the "avoid X padding" flags, - nested StreamSafeArea doesn't double-inset (removePadding), - rebuilds when the MediaQuery padding changes, - doesn't crash at zero area. (We already exceed the suite on margin, RTL, and the driven lerp/consume/ reactivity; no sliver variant, so SliverSafeArea's cases don't apply.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… false Match Flutter's SafeArea, which defaults it to false. Reading the bottom gap from padding (not viewPadding) lets it collapse with the keyboard and lets a parent that injects the inset via padding (e.g. StreamScaffold) flow through unchanged. Callers that pin a surface the keyboard should slide under can opt into viewPadding with maintainBottomViewPadding: true. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Use max(inset, spacing.md) on the pill's bottom instead of adding a margin. A larger system inset (iOS home indicator, Android nav bar) is used as-is so the pill sits flush above it rather than lifting an extra margin higher, while a device that reserves nothing still gets the spacing.md floor. Behaviour is unchanged on zero-inset devices (still spacing.md); it only stops over-lifting where the inset already exceeds the floor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an optional `type` that overrides the shape the toolbar otherwise resolves from the enclosing bar (outline when floating, ghost when docked), so a slot can render e.g. a solid primary action while keeping the bar-driven elevation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These generated_plugin_registrant.* / generated_plugins.cmake files under the stream_thumbnail example's linux/ folder are regenerated on every Linux build and were committed by mistake in 2beebd3. Remove them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d9c1ebe to
401b891
Compare
The design_system_gallery's macOS Runner.xcodeproj/project.pbxproj and Runner.xcscheme were auto-modified by Xcode (build-setting / scheme churn) and committed by mistake. Restore them to their base (main) version so they drop out of the PR diff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| final footer = props.footer?.preferredSize; | ||
|
|
||
| final headerInset = (header == null || headerFloating) ? 0.0 : header.height + mediaQueryPadding.top; | ||
| final footerInset = (footer == null || footerFloating) ? 0.0 : footer.height + mediaQueryPadding.bottom; |
There was a problem hiding this comment.
This assumes the header/footer bar self-insets the safe area, which only happens when the bar's own primary is true (the default on StreamAppBar/StreamBottomAppBar). Pass header: StreamAppBar(primary: false, ...) — a supported, documented option on the bar itself — and the rendered bar height stays at the fixed toolbar height (no device padding added), but this still reserves header.height + mediaQueryPadding.top / footer.height + mediaQueryPadding.bottom. That leaves a visible gap between the chrome's edge and where the media content actually starts.
stream_media_viewer_test.dart's _FakeBar fixture (used in the "regular chrome adds the device inset to the bar heights" case) is a bare SizedBox(height: height) that doesn't self-inset either — i.e. it behaves like a primary: false bar — but the test only asserts the abstract AnimatedPadding.padding value, never the actual rendered position of the header vs. the content's first pixel, so it doesn't catch the gap.
Suggest gating the + mediaQueryPadding.top / + mediaQueryPadding.bottom addition on the chrome's own resolved primary state (mirroring how headerFloating/footerFloating are already resolved above), plus a test that pairs a real StreamAppBar(primary: false) / StreamBottomAppBar(primary: false) with the media viewer.
The bottom slot holds an arbitrary widget (composer, nav bar, bottom app bar), so name the override after the slot rather than a bar, and resolve it from the per-instance value then the ambient StreamSurfaceStyle only. Drop the StreamBottomAppBarTheme rung, which wrongly applied bottom-app-bar semantics to whatever occupied the slot; the app bar slot keeps its own theme chain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Introduces a `StreamSpacing.safeAreaBottom()` extension that provides platform-specific minimum insets (xxl on iOS/macOS, xxxl elsewhere) for floating surfaces. - Updates `StreamBottomNavBar` to use this adaptive spacing for its minimum bottom inset instead of a hardcoded `spacing.md`. - Corrects a documentation reference from `bottomBarSurfaceStyle` to `bottomSurfaceStyle`. - Updates widget tests and golden images to reflect the platform-aware spacing logic.
| final StreamElevation _elevation; | ||
|
|
||
| @override | ||
| double get floatingElevation => _elevation.level2; |
There was a problem hiding this comment.
I thought we made it level2 because level3 was a bit too much on group avatars.
| /// a docked bar. | ||
| /// | ||
| /// In a [StreamScaffold] `bottom` slot, drive floating through the ambient | ||
| /// [StreamSurfaceStyle] (or the scaffold's `bottomBarSurfaceStyle`) so the |
There was a problem hiding this comment.
| /// [StreamSurfaceStyle] (or the scaffold's `bottomBarSurfaceStyle`) so the | |
| /// [StreamSurfaceStyle] (or the scaffold's `bottomSurfaceStyle`) so the |
| /// system inset (home indicator, navigation bar) is honored as-is. | ||
| /// | ||
| /// Adapts to [platform], defaulting to the current platform. | ||
| double safeAreaBottom({TargetPlatform? platform}) { |
There was a problem hiding this comment.
"The recommended minimum bottom inset for a floating surface"
I would also put that in the method name, something like
double safeAreaFloatingBottom({TargetPlatform? platform}) {There was a problem hiding this comment.
In figma the name of the token is device/safe-area-bottom.
There was a problem hiding this comment.
Yeah but in Figma it's actually supposed to be the safe area, now this is a custom value that is not in any way really coming from the safe area the device specifies. This is only supposed to be used for floating components right? In every other case I would expect the safearea on the bottom to be exactly the same as the official safe area from the device.
2 participants
What
Adds
StreamScaffold— a full-page scaffold that supports both regular and floating app-bar / bottom-bar layouts — and rounds out floating support across the toolbar bars:StreamAppBarnow propagates its resolved floating state to its slots.StreamBottomAppBargains a floating variant to matchStreamAppBar.StreamSafeArea, a shared safe-area primitive, and uses it for the floatingStreamBottomNavBarpill's inset.StreamToolbarButtongains an optionaltypeoverride.How it handles insets
When a bar floats, the scaffold enlarges the body's
MediaQuery.paddingby the measured height of that bar (top for a floating app bar, bottom for a floating bottom bar). So:ListView/GridView) andSafeAreaauto-inset their content — no manual padding needed.MediaQuery.padding(CustomScrollView,ScrollablePositionedList,SingleChildScrollView) can readMediaQuery.paddingOf(context)and fold it in explicitly.A docked (regular) bottom bar sits below the body and owns the bottom safe-area inset; the scaffold
removePaddings it from the body so scrollables don't reserve it twice.StreamAppBar — floating-aware slots
StreamAppBarresolves itsbehavior(floating/regular), but previously only its own auto-implied back button knew about it. A caller-provided slot that resolves floating from context — e.g. a channel header's avatar or a custom button callingisFloatingAppBar(context)— couldn't see abehaviorthat was set viastylepassed only to the bar.The bar now republishes its resolved
behaviorinto an ambientStreamAppBarThemescoped to its slot subtree — the same way it already republishes button and title styles to its slots, and the way Material'sAppBarrepublishesIconTheme/DefaultTextStyleto its toolbar. Behaviour only; padding, colours, and spacing stay the bar's own chrome.This fixes a live bug in the chat SDK (motivating consumer:
stream-chat-flutter#2748), where a floating channel header rendered a flat avatar + ghost back button because those slots resolvedbehaviorfrom the ambient theme, which thestyleprop never reached.StreamBottomAppBar — floating variant
StreamBottomAppBaralready had abehaviorfield, but only the scaffold read it (for layout) — the component itself always rendered a solid bar, so a scaffold-floated bottom app bar would sit opaque over the content behind it. It now mirrorsStreamAppBar:effectiveBehaviorfromstyle.behavior→ ambientStreamBottomAppBarTheme→StreamAppStyle.StreamBottomAppBarStyle.floatingBackgroundColor, defaulting tobackgroundElevation0), mirroring the nav bar's fade direction.behaviorto itsleading/title/trailingslots, symmetric withStreamAppBar.Two notes for reviewers:
StreamBottomAppBarThemebehaviour yet (StreamButton.isFloatingis prop-only, so floating slot buttons are still wired explicitly, as the gallery shows). It's pinned by tests, ready for when a reader lands (aStreamButtonambient-resolution or anisFloatingBottomAppBarhelper). This differs from the app-bar republish, which has a live reader today.StreamBottomAppBar(style: floating)floats the bar, whileStreamScaffolddecidesextendBody/padding from its ownbottomBarBehavior/ the ambientStreamBottomAppBarTheme. Set both consistently for the full effect (same shape as the app bar).StreamSafeArea
A shared safe-area primitive (exported from
core.dart) — aSafeAreavariant that insets its child bymax(systemInset, minimum) + marginper edge, so a pinned or floating surface keeps a controlled gap from the status bar / navigation bar / home indicator instead of sitting flush.minimumfloors an edge (likeSafeArea.minimum);marginadds beyond the safe area.maintainBottomViewPaddingdefaults tofalse, matching Flutter'sSafeArea(settrueto readviewPaddingso a pinned surface doesn't move when the keyboard opens).StreamSafeArea.resolveInsetsreturns the same insets as a value — e.g. to size a gradient painted behind the child.StreamSafeArea.driveninterpolates the inset toward a target (defaultEdgeInsets.zero) driven by anyValueListenable<double>, for a floating surface that releases its space as a panel slides in beneath it.StreamBottomNavBar fix
The floating pill derived its bottom gap solely from
SafeArea, so it sat flush on devices reporting no bottom inset (emulators / non-edge-to-edge 3-button nav). It now insets viaStreamSafeArea: the sides and top floor atspacing.xl(the pill's Figma side margins), and the bottom floors atspacing.md. Flooring (rather than adding a margin) means a larger system inset — an iOS home indicator, an Android navigation bar — is used as-is so the pill sits flush above it without lifting an extra margin, while a device that reserves nothing still gets thespacing.mdfloor.StreamToolbarButton — type override
StreamToolbarButtongains an optionaltypethat overrides the shape it otherwise resolves from the enclosing bar (outline when floating, ghost when docked) — so a slot can render e.g. a solid primary action while keeping the bar-driven elevation.Tests & gallery
ListViewre-consuming the top inset).StreamAppBarslot-behaviour tests (republish + ambient-theme precedence).StreamBottomAppBarfloating-visual + slot-behaviour tests.StreamSafeAreatest suite mirroring Flutter's ownSafeArea(edge flags, minimums, nesting, keyboard /maintainBottomViewPadding, zero-area), plusresolveInsets,.driven, RTL, anddebugFillProperties.StreamToolbarButtontype-override test.StreamScaffoldandStreamBottomAppBar(floating knob) gallery use-cases.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
StreamScaffoldgallery with playground and real-world examples, including configurable drawers, app bars, bottom navigation, gestures, and restoration.Updates
StreamSurfaceStyle.