feat(networkWidget): added loader and error widget support to networkwidget #373
Conversation
|
Sharath S R seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughThe pull request adds support for custom loading and error widgets to the Stac network widget. Two new optional fields ( Changes
Sequence DiagramsequenceDiagram
participant Parser as StacNetworkWidget Parser
participant Service as StacService
participant Network as Stac.fromNetwork
Parser->>Service: Extract loadingWidget (if present)
Service->>Service: Convert via fromStacWidget
Parser->>Service: Extract errorWidget (if present)
Service->>Service: Convert via fromStacWidget
Parser->>Network: Pass request + loadingWidget builder + errorWidget builder
alt Network Request
Network->>Network: Request initiated
Network->>Network: Show loadingWidget if provided
Network->>Network: Success or Error
end
alt Success State
Network->>Network: Display loaded content
else Error State
Network->>Network: Show errorWidget if provided
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ 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: 0
🧹 Nitpick comments (3)
packages/stac_core/test/widgets/network_widget/stac_network_widget_test.dart (1)
6-32: Consider validating widget content, not just presence.The test correctly verifies that the optional widgets are not null, but could be more thorough by checking the actual widget types or properties to ensure correct deserialization.
Consider adding assertions like:
expect(widget.loadingWidget, isA<StacTextWidget>()); expect((widget.loadingWidget as StacTextWidget).data, equals('Loading...'));packages/stac/lib/src/parsers/widgets/stac_network_widget/stac_network_widget_parser.dart (1)
18-35: Consider: Error parameter is available but not propagated to the widget.The
errorWidgetcallback receives anerrorparameter (Line 30), but this error information is not passed to theStacWidgetduring conversion. This means custom error widgets defined in JSON cannot access the actual error details (message, stack trace, etc.) to provide more specific error feedback.This is a design limitation where the
StacWidgetmodel doesn't currently support passing runtime error context. While the current implementation is functional, you may want to:
- Document this limitation in the model's documentation
- Consider future enhancement where error details could be injected via a variable system or context
- For now, error widgets can only show static error messages
The current implementation is acceptable for the initial feature, but documenting this limitation would help users understand why their error widgets can't show specific error messages.
packages/stac_core/lib/widgets/network_widget/stac_network_widget.dart (1)
50-54: Well-documented optional fields.The new
loadingWidgetanderrorWidgetfields are properly typed and documented.Consider updating the class-level documentation examples (lines 15-36) to include the new optional parameters, showing how users can provide custom loading and error widgets. This would make the feature more discoverable.
Example addition to JSON example:
{ "type": "networkWidget", "request": { ... }, "loadingWidget": { "type": "circularProgressIndicator" }, "errorWidget": { "type": "text", "data": "Failed to load" } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/stac_gallery/assets/json/home_screen.json(1 hunks)examples/stac_gallery/assets/json/network_widget_example.json(1 hunks)packages/stac/lib/src/parsers/widgets/stac_network_widget/stac_network_widget_parser.dart(1 hunks)packages/stac_core/lib/widgets/network_widget/stac_network_widget.dart(1 hunks)packages/stac_core/lib/widgets/network_widget/stac_network_widget.g.dart(1 hunks)packages/stac_core/test/widgets/network_widget/stac_network_widget_test.dart(1 hunks)
🔇 Additional comments (10)
examples/stac_gallery/assets/json/home_screen.json (1)
1443-1465: LGTM! Well-structured gallery entry.The new list tile entry for "Stac Network Widget" is properly formatted and follows the existing patterns in the gallery. The icon choice and description clearly communicate the feature's purpose.
packages/stac_core/test/widgets/network_widget/stac_network_widget_test.dart (3)
34-52: LGTM! Proper validation of optional fields.This test correctly verifies that the widget can be created without the optional loadingWidget and errorWidget fields, ensuring backward compatibility.
54-74: LGTM! Comprehensive serialization test.The test thoroughly validates JSON serialization including both presence and content of the optional widget fields.
76-92: LGTM! Validates optional field serialization.This test ensures that when optional widgets are not provided, they serialize as null, maintaining a clean JSON output.
packages/stac_core/lib/widgets/network_widget/stac_network_widget.g.dart (2)
14-19: LGTM! Proper nullable field deserialization.The generated code correctly handles optional widget fields with appropriate null checks and uses
StacWidget.fromJsonfor nested deserialization.
25-26: LGTM! Clean nullable field serialization.The null-aware operator correctly handles serialization of optional widgets, ensuring they only call
toJson()when non-null.examples/stac_gallery/assets/json/network_widget_example.json (3)
11-16: LGTM! Appropriate test endpoint.Using
httpbin.org/delay/2is an excellent choice for demonstrating the loading state, as it provides a reliable 2-second delay that users can observe.
17-36: LGTM! Clean loading state UI.The loading widget follows best practices with a centered layout, progress indicator, appropriate spacing, and informative text. This provides a good user experience during network requests.
37-71: LGTM! User-friendly error state.The error widget provides clear visual feedback with an appropriately colored error icon, bold heading, and actionable message. This offers good UX guidance for users encountering network issues.
packages/stac_core/lib/widgets/network_widget/stac_network_widget.dart (1)
41-45: LGTM! Clean constructor design.The constructor properly defines the new optional parameters while maintaining the required
requestfield. The parameter ordering and naming are consistent with Dart conventions.
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Caution Docstrings generation - FAILED No docstrings were generated. |
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
@Sharath-a26 Can you please sign the CLA @CLAassistant |
| loadingWidget: model.loadingWidget == null | ||
| ? null | ||
| : (ctx) => StacService.fromStacWidget( | ||
| widget: model.loadingWidget!, | ||
| context: ctx, | ||
| ) ?? | ||
| const SizedBox(), | ||
| errorWidget: model.errorWidget == null | ||
| ? null | ||
| : (ctx, error) => StacService.fromStacWidget( | ||
| widget: model.errorWidget!, | ||
| context: ctx, | ||
| ) ?? | ||
| const SizedBox(), | ||
| ); |
There was a problem hiding this comment.
Can we use the parse extension method instead from StacWidgetParser. Like
loadingWidget: model.loadingWidget?.parse(context),
errorWidget: model.errorWidget?.parse(context),
| @@ -0,0 +1,95 @@ | |||
| import 'package:flutter_test/flutter_test.dart'; | |||
There was a problem hiding this comment.
Can we skip this test for now? It will be added again when we include full widget testing.
3 participants
Network Widget
Description
Related Issues
Closes #243
Type of Change
Summary by CodeRabbit
New Features
Tests