-
-
Notifications
You must be signed in to change notification settings - Fork 89
feat: Update modal bottom sheet action and example JSON #405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Changed the action type for closing the modal bottom sheet from "pop" to "navigate" with a "pop" navigation style in the `StacModalBottomSheetAction` class. - Enhanced the bottom sheet example JSON to include a structured widget definition, featuring a title, close button, and descriptive text, improving the demonstration of modal bottom sheet usage.
📝 WalkthroughWalkthroughUpdated documentation and example files to reflect navigation-based modal bottom sheet closure. The modal bottom sheet now closes using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/stac_gallery/assets/json/bottom_sheet_example.json (1)
38-38: Consider: Fixed height may not accommodate varying content.The container has a fixed height of 200, which works for this simple example but may not be suitable for all use cases. For production implementations, consider using dynamic sizing or making the height configurable based on content requirements.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/actions/modal_bottom_sheet.mdxexamples/stac_gallery/assets/json/bottom_sheet_example.json
⏰ 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)
- GitHub Check: analyze
🔇 Additional comments (3)
examples/stac_gallery/assets/json/bottom_sheet_example.json (2)
36-86: Good improvement: Inline widget definition provides a clearer, self-contained example.The replacement of the
assetPathreference with an inline widget definition significantly improves the example's clarity and educational value. The widget structure demonstrates a typical modal bottom sheet pattern with a header (title + close button) and descriptive content.
67-70: Consistent with documentation: Using navigate action to close the modal bottom sheet.The close button correctly uses
actionType: "navigate"withnavigationStyle: "pop", which is consistent with the updated documentation pattern. This demonstrates the recommended approach for dismissing modal bottom sheets.docs/actions/modal_bottom_sheet.mdx (1)
56-59: Pattern verified—navigatewithpopstyle is the standard approach for closing modal bottom sheets in Stac applications.The change to
actionType: "navigate"withnavigationStyle: "pop"is consistent with the established pattern used throughout the codebase for closing modals, dialogs, and bottom sheets.
Description
StacModalBottomSheetActionclass.Type of Change
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.