refactor(ui): decouple StreamComponentFactory from StreamTheme#47
refactor(ui): decouple StreamComponentFactory from StreamTheme#47xsahil03x merged 3 commits intomain-design-systemfrom
Conversation
- Convert `StreamComponentFactory` to `InheritedWidget` for subtree overrides. - Remove `componentFactory` from `StreamTheme`. - Introduce `StreamComponentBuilders` for component customization. - Update `StreamButton` and `StreamFileTypeIcon` to use the new factory lookup.
📝 WalkthroughWalkthroughThis PR refactors the component customization architecture by introducing an InheritedWidget-based StreamComponentFactory that replaces theme-integrated factory management. Multiple components adopt a props-based configuration pattern with dedicated Default* implementations, decoupling rendering from configuration and enabling factory-based builder injection. Changes
Sequence DiagramsequenceDiagram
participant Widget as Component Widget
participant Factory as StreamComponentFactory
participant Builder as Custom Builder
participant Default as Default Implementation
Widget->>Widget: build(context)
Widget->>Factory: maybeOf(context)
alt Factory Found & Builder Available
Factory-->>Widget: StreamComponentBuilders
Widget->>Builder: builder(context, props)
Builder-->>Widget: Custom Widget
else Factory Not Found or No Builder
Factory-->>Widget: null / no builder
Widget->>Default: DefaultComponent(props)
Default-->>Widget: Default Rendered Widget
end
Default-->>Widget: Widget Tree
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (6.06%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main-design-system #47 +/- ##
=====================================================
Coverage ? 37.69%
=====================================================
Files ? 78
Lines ? 2056
Branches ? 0
=====================================================
Hits ? 775
Misses ? 1281
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Refactors StreamAvatar, StreamAvatarGroup, StreamAvatarStack, StreamBadgeCount, and StreamOnlineIndicator to use StreamComponentFactory.
…ent factory and theme extensions
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/stream_core_flutter/lib/src/components/accessories/stream_file_type_icon.dart (1)
188-188:⚠️ Potential issue | 🟡 MinorInconsistent file type mapping for tar archives.
'application/x-tar'is mapped to.codewith extension'tar', but tar is a compression/archive format, not a code format. This appears to be a copy-paste error given the surrounding compression format mappings.🐛 Proposed fix
- 'application/x-tar' => (.code, 'tar'), + 'application/x-tar' => (.compression, 'tar'),packages/stream_core_flutter/lib/src/components/avatar/stream_avatar_stack.dart (1)
221-227:⚠️ Potential issue | 🟡 MinorBadge positioning may be slightly off when badge size differs from avatar size.
The positioning loop uses
i * visiblePortionfor all children including the overflow badge (line 224). However,visiblePortionis derived from the avatar diameter, while the badge may have a different diameter (badgeDiameter). This means the badge might not align perfectly with the expected overlap when badge and avatar sizes differ.Currently
badgeVisiblePortion(line 203) is only used fortotalWidthcalculation but not for positioning the badge itself.💡 Suggested approach
If precise alignment is needed, calculate the badge position separately:
for (var i = 0; i < displayChildren.length; i++) Positioned( - left: i * visiblePortion, + left: i < visible.length + ? i * visiblePortion + : (visible.length - 1) * visiblePortion + badgeVisiblePortion, child: displayChildren[i], ),However, if the current visual result is acceptable, this can be deferred.
🧹 Nitpick comments (2)
packages/stream_core_flutter/lib/src/factory/stream_component_factory.g.theme.dart (1)
87-107: Function equality comparison relies on reference identity.The
==operator compares builder functions directly. In Dart, function equality uses reference identity, so two identical anonymous functions created separately will not be equal. This is standard behavior but worth noting: widgets using this equality check may rebuild more often than expected if builders are recreated on each build.packages/stream_core_flutter/lib/src/components/avatar/stream_avatar_group.dart (1)
150-185: Materializechildrenonce to avoid repeated Iterable traversal.
Iterable.length/first/last/elementAtcan re-traverse or re-evaluate lazy iterables. Converting once avoids extra work and ensures consistent ordering.♻️ Proposed refactor
- if (props.children.isEmpty) return const SizedBox.shrink(); + final children = props.children.toList(growable: false); + if (children.isEmpty) return const SizedBox.shrink(); @@ - builder: (context) => switch (props.children.length) { - 1 => _buildForOne(context, props.children), - 2 => _buildForTwo(context, props.children), - 3 => _buildForThree(context, props.children), - 4 => _buildForFour(context, props.children), - _ => _buildForFourOrMore(context, props.children), + builder: (context) => switch (children.length) { + 1 => _buildForOne(context, children), + 2 => _buildForTwo(context, children), + 3 => _buildForThree(context, children), + 4 => _buildForFour(context, children), + _ => _buildForFourOrMore(context, children), },
2 participants
Description of the pull request
StreamComponentFactorytoInheritedWidgetfor subtree overrides.componentFactoryfromStreamTheme.StreamComponentBuildersfor component customization.StreamButtonandStreamFileTypeIconto use the new factory lookup.Summary by CodeRabbit
Release Notes
New Features
Refactor