feat: add StacPopAction for navigation stack management #403
Conversation
📝 WalkthroughWalkthroughAdds a new "pop" action type, its StacPopAction model and JSON wiring, a StacPopActionParser that calls Navigator.pop with an optional result, and registers the parser in StacService; also exports were added to public action APIs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant StacService
participant StacPopActionParser
participant Navigator as Flutter Navigator
Client->>StacService: dispatch action JSON (type: "pop", result: {...})
StacService->>StacPopActionParser: match actionType & getModel(json)
StacPopActionParser->>StacPopActionParser: construct StacPopAction model
StacPopActionParser->>Navigator: if canPop(context) pop(context, model.result)
Navigator-->>Client: previous route receives result (optional)
note right of StacPopActionParser `#DDEFEF`: New/changed interaction: pop with result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-19T20:20:55.561ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
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
🤖 Fix all issues with AI Agents
In @packages/stac/lib/src/framework/stac_service.dart:
- Line 148: The StacPopActionParser's onCall currently calls
Navigator.pop(context, model.result) without guarding against an empty
navigation stack; update the onCall implementation in StacPopActionParser to
either check Navigator.canPop(context) before calling Navigator.pop or wrap
Navigator.pop(context, model.result) in a try-catch that logs the error (use
debugPrint or the existing logger) to avoid an exception when the stack is
empty; ensure you reference the model.result from StacPopAction and still return
null.
In
@packages/stac/lib/src/parsers/actions/stac_pop_action/stac_pop_action_parser.dart:
- Around line 17-21: The onCall implementation in StacPopAction parser calls
Navigator.pop(context, model.result) unconditionally which can throw if there is
no route to pop; update the onCall method to first check
Navigator.canPop(context) and only call Navigator.pop when canPop returns true
(otherwise do nothing or handle accordingly), keeping the method signature and
return behavior intact; reference the onCall method in the StacPopAction parser
and the Navigator.canPop/ Navigator.pop calls when making the change.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/stac/lib/src/framework/stac_service.dartpackages/stac/lib/src/parsers/actions/actions.dartpackages/stac/lib/src/parsers/actions/stac_pop_action/stac_pop_action_parser.dartpackages/stac_core/lib/actions/actions.dartpackages/stac_core/lib/actions/pop/stac_pop_action.dartpackages/stac_core/lib/actions/pop/stac_pop_action.g.dartpackages/stac_core/lib/foundation/specifications/action_type.dart
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-19T20:20:55.561Z
Learnt from: akhil-ge0rge
Repo: StacDev/stac PR: 399
File: packages/stac/lib/src/parsers/theme/stac_tool_tip_theme_data_parser.dart:9-26
Timestamp: 2025-12-19T20:20:55.561Z
Learning: In Flutter, TooltipThemeData.height is deprecated since v3.30.0-0.1.pre. Replace uses with TooltipThemeData.constraints: BoxConstraints(minHeight: value) to achieve the same minimum height. Apply this pattern to review any Dart/Flutter code that uses TooltipThemeData.height, not just this file.
Applied to files:
packages/stac_core/lib/actions/actions.dartpackages/stac_core/lib/foundation/specifications/action_type.dartpackages/stac/lib/src/parsers/actions/actions.dartpackages/stac_core/lib/actions/pop/stac_pop_action.g.dartpackages/stac/lib/src/framework/stac_service.dartpackages/stac_core/lib/actions/pop/stac_pop_action.dartpackages/stac/lib/src/parsers/actions/stac_pop_action/stac_pop_action_parser.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: analyze
- GitHub Check: pub_get_check
🔇 Additional comments (6)
packages/stac/lib/src/parsers/actions/actions.dart (1)
7-7: LGTM! Clean export addition.The export is correctly positioned alphabetically and follows the established pattern for action parser exports.
packages/stac_core/lib/foundation/specifications/action_type.dart (1)
38-40: LGTM! Well-documented enum addition.The
popaction type is clearly documented and follows the established pattern for enum values in this file.packages/stac_core/lib/actions/actions.dart (1)
12-12: LGTM! Export addition looks good.The StacPopAction export is correctly added to the actions barrel file.
packages/stac_core/lib/actions/pop/stac_pop_action.g.dart (1)
1-16: LGTM! Standard generated serialization code.The JSON serialization logic is correctly generated by json_serializable. The code appropriately:
- Deserializes the optional
resultfield asMap<String, dynamic>?- Serializes both
resultandactionTypefor consistencypackages/stac_core/lib/actions/pop/stac_pop_action.dart (1)
1-49: LGTM! Well-documented action model with appropriate design.The implementation is clean and follows the established pattern for STAC actions:
- Proper JSON serialization setup with generated code
- Comprehensive documentation with both Dart and JSON examples
- Flexible
resultfield type (Map<String, dynamic>?) is appropriate for a generic action framework- Const constructor enables compile-time optimization
The action model correctly integrates with the framework's type system via
actionTypeoverride.packages/stac/lib/src/parsers/actions/stac_pop_action/stac_pop_action_parser.dart (1)
10-11: ActionType.pop enum value confirmed.
ActionType.popexists in the ActionType enum at line 40 ofpackages/stac_core/lib/foundation/specifications/action_type.dartwith proper documentation.
| const StacSetValueActionParser(), | ||
| const StacMultiActionParser(), | ||
| const StacDelayActionParser(), | ||
| const StacPopActionParser(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find StacPopActionParser implementation
fd -t f 'stac_pop_action_parser.dart' --exec cat {}Repository: StacDev/stac
Length of output: 621
Consider adding error handling for empty navigation stack.
The parser correctly passes the result value from the model to Navigator.pop(). However, the implementation lacks error handling for when the navigation stack is empty. Navigator.pop() will throw an exception in this case, potentially crashing the app. Add a try-catch block or validate the navigation stack before popping:
@override
FutureOr onCall(BuildContext context, StacPopAction model) {
try {
Navigator.pop(context, model.result);
} catch (e) {
debugPrint('Failed to pop navigation: $e');
}
return null;
}🤖 Prompt for AI Agents
In @packages/stac/lib/src/framework/stac_service.dart at line 148, The
StacPopActionParser's onCall currently calls Navigator.pop(context,
model.result) without guarding against an empty navigation stack; update the
onCall implementation in StacPopActionParser to either check
Navigator.canPop(context) before calling Navigator.pop or wrap
Navigator.pop(context, model.result) in a try-catch that logs the error (use
debugPrint or the existing logger) to avoid an exception when the stack is
empty; ensure you reference the model.result from StacPopAction and still return
null.
packages/stac/lib/src/parsers/actions/stac_pop_action/stac_pop_action_parser.dart
Show resolved
Hide resolved
Implement a new action type that allows popping the current route from the navigation stack. This action is useful for closing dialogs, bottom sheets, or navigating back to the previous screen with an optional result value. Changes: - Add StacPopAction model class in stac_core with optional result parameter - Add StacPopActionParser to handle pop action execution - Register StacPopActionParser in StacService action parsers list - Export StacPopAction in actions.dart barrel file - Add 'pop' action type to ActionType enum
Potatomonsta
force-pushed
the
mn/pop-action
branch
from
83ff138 to
38c094a
Compare
Description
This PR adds a new
StacPopActionthat allows popping the current route from the navigation stack. This action is useful for closing dialogs, bottom sheets, or navigating back to the previous screen with an optional result value.Changes:
StacPopActionmodel class instac_corewith optionalresultparameterStacPopActionParserto handle pop action execution usingNavigator.pop()StacPopActionParserinStacServiceaction parsers listpopaction type toActionTypeenumStacPopActionin actions barrel fileStacPopActionUsage:
The action can be used in JSON configuration:
{ "actionType": "pop", "result": { "key": "value" } }Or in Dart:
Related Issues
Closes #
Type of Change
Summary by CodeRabbit
New Features
Framework Updates
✏️ Tip: You can customize this high-level summary in your review settings.