-
-
Notifications
You must be signed in to change notification settings - Fork 86
feat: Introduce stacTooltip widget and parser with example JSON #397
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
WalkthroughAdds first-class Tooltip support: new StacTooltip model + generated JSON (de)serialization, WidgetType.tooltip, StacTooltipTriggerMode and parser, Tooltip widget parser and service registration, example JSON asset and documentation. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Gallery as Gallery JSON (asset)
participant Service as StacService
participant Parser as StacToolTipParser
participant Flutter as Flutter Tooltip Widget
Gallery->>Service: load JSON asset (type: "tooltip")
Service->>Parser: select parser for WidgetType.tooltip
Parser->>Parser: getModel(json) -> StacTooltip
Parser->>Parser: parse nested fields (durations, styles, decoration, child)
Parser->>Flutter: construct Tooltip widget with parsed config & child
Service->>Flutter: return widget to UI for rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (6)
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: 3
🧹 Nitpick comments (4)
packages/stac_core/lib/foundation/specifications/widget_type.dart (1)
240-241: Fix comment to match Flutter's naming convention.The comment should use "Tooltip" (one word) instead of "Tool Tip" (two words) to align with Flutter's official widget name.
Apply this diff:
- /// Tool Tip widget + /// Tooltip widget tooltip,examples/stac_gallery/assets/json/toot_tip_example.json (1)
172-193: Consider adding consistent spacing before the last tooltip section.The "Tap to see Tooltip" section at lines 172-193 lacks the
sizedBoxspacer (height: 32) that appears before other major sections in this example. This breaks the visual consistency established by earlier sections.Apply this diff to add consistent spacing:
} ] }, + { + "type": "sizedBox", + "height": 32 + }, { "type": "text", "data": "Tap to see Tooltip",packages/stac/lib/src/parsers/widgets/stac_tool_tip/stac_tool_tip_parser.dart (1)
16-36: Consider validating that eithermessageorrichMessageis provided.The parser allows both
model.messageandmodel.richMessageto be null, which would result in a tooltip with no displayable content. Flutter's Tooltip widget accepts this but produces a tooltip that shows nothing when triggered, which may not be the intended behavior.Consider adding validation in the parse method:
@override Widget parse(BuildContext context, StacTooltip model) { + assert( + model.message != null || model.richMessage != null, + 'Either message or richMessage must be provided for StacTooltip', + ); return Tooltip(Alternatively, handle this validation in the StacTooltip model constructor itself.
packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.dart (1)
120-137: Clarify duration field documentation.The comments state "Defined in milliseconds" for
waitDuration,showDuration, andexitDuration, but these fields are typed asStacDuration?, not as raw numbers. This phrasing could be confusing since the actual JSON structure requires an object (e.g.,{ "milliseconds": 1000 }), not a bare integer.Consider revising the documentation to be more explicit:
- /// The length of time that a pointer must hover over a tooltip's widget - /// before the tooltip will be shown. - /// - /// Defined in milliseconds. + /// The length of time that a pointer must hover over a tooltip's widget + /// before the tooltip will be shown. final StacDuration? waitDuration;Or update to:
/// The length of time that a pointer must hover over a tooltip's widget /// before the tooltip will be shown. /// /// Expects a [StacDuration] object specifying the duration. final StacDuration? waitDuration;Apply similar changes to
showDuration(lines 126-131) andexitDuration(lines 133-137).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
examples/stac_gallery/assets/json/home_screen.json(1 hunks)examples/stac_gallery/assets/json/toot_tip_example.json(1 hunks)packages/stac/lib/src/framework/stac_service.dart(2 hunks)packages/stac/lib/src/parsers/foundation/foundation.dart(1 hunks)packages/stac/lib/src/parsers/foundation/ui_components/stac_tool_tip_trigger_mode_parser.dart(1 hunks)packages/stac/lib/src/parsers/widgets/stac_tool_tip/stac_tool_tip_parser.dart(1 hunks)packages/stac_core/lib/foundation/foundation.dart(1 hunks)packages/stac_core/lib/foundation/specifications/widget_type.dart(1 hunks)packages/stac_core/lib/foundation/ui_components/stac_tool_tip_trigger_mode.dart(1 hunks)packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.dart(1 hunks)packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.g.dart(1 hunks)packages/stac_core/lib/widgets/widgets.dart(1 hunks)
🔇 Additional comments (8)
packages/stac/lib/src/parsers/foundation/foundation.dart (1)
113-113: LGTM!The export is correctly placed and follows the alphabetical ordering convention within the UI components section.
packages/stac/lib/src/parsers/foundation/ui_components/stac_tool_tip_trigger_mode_parser.dart (1)
1-17: LGTM!The parser extension correctly maps all StacTooltipTriggerMode enum values to their corresponding Flutter TooltipTriggerMode values using an exhaustive switch.
packages/stac/lib/src/framework/stac_service.dart (2)
19-19: LGTM!The import for StacToolTipParser is correctly added alongside other widget parser imports.
103-103: LGTM!The StacToolTipParser is correctly registered in the global parsers list.
packages/stac_core/lib/widgets/widgets.dart (1)
82-82: LGTM!The export is correctly placed in alphabetical order within the widgets barrel file.
packages/stac_core/lib/foundation/foundation.dart (1)
109-109: LGTM!The export is correctly placed in alphabetical order within the UI components section of the foundation library.
packages/stac_core/lib/foundation/ui_components/stac_tool_tip_trigger_mode.dart (1)
1-15: LGTM!The enum correctly mirrors Flutter's TooltipTriggerMode with clear documentation for each variant.
packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.g.dart (1)
1-91: Generated code looks correct.The JSON serialization code is properly generated with appropriate null handling, enum mappings, and default values.
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 (2)
docs/widgets/tool_tip.mdx (2)
85-85: Add blank line before heading for consistency.The markdown formatting should include a blank line before the heading for better readability and consistency with other sections.
Apply this diff:
}
Tooltip with Delay & Duration
--- `40-157`: **Consider adding examples for additional properties (optional).** The current examples are excellent and cover common use cases. For completeness, consider adding examples demonstrating: - `triggerMode` (e.g., showing the difference between `longPress` and `tap`) - `enableTapToDismiss` set to `false` - `textAlign` with different alignments - `constraints` for controlling tooltip dimensions This would help users understand the full range of customization options. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between a1720b7af53c17571595998567afb888ac63806f and 0099a40b854060b19678885458544743f13bc96c. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `docs/widgets/tool_tip.mdx` (1 hunks) * `packages/stac_core/lib/foundation/specifications/widget_type.dart` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * packages/stac_core/lib/foundation/specifications/widget_type.dart </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>docs/widgets/tool_tip.mdx (2)</summary><blockquote> `1-14`: **LGTM!** The introduction is clear, well-structured, and appropriately references the official Flutter documentation. --- `15-37`: **Verify property definitions against Flutter's Tooltip API.** The properties table documents the Stac Tooltip widget. Confirm that all properties, their types, default values, and behaviors align with Flutter's Tooltip widget API, especially: - Line 25: `preferBelow` default of `true` - Line 33: `enableTapToDismiss` default of `true` - Line 34: `triggerMode` values (`longPress` or `tap`) </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/stac_gallery/assets/json/home_screen.json(1 hunks)packages/stac/lib/src/parsers/widgets/stac_tool_tip/stac_tool_tip_parser.dart(1 hunks)packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/stac/lib/src/parsers/widgets/stac_tool_tip/stac_tool_tip_parser.dart
- examples/stac_gallery/assets/json/home_screen.json
🔇 Additional comments (3)
packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.dart (3)
1-5: LGTM: Clean imports and standard code generation setup.The imports are appropriate for JSON serialization and the Stac widget foundation. The part directive follows the standard pattern for
json_serializable.
43-152: LGTM: Well-structured widget model with comprehensive documentation.The class declaration, constructor, and field definitions follow Dart best practices:
- Appropriate use of nullable types for optional configurations
- Sensible default for
enableTapToDismiss(true)- Clear documentation for each field, including units and behavioral notes
- Proper alignment with Flutter's Tooltip API
154-163: LGTM: Standard serialization implementation.The type getter and JSON serialization methods follow the established Stac widget patterns correctly, delegating to the generated code as expected.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.dart(1 hunks)
🔇 Additional comments (6)
packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.dart (6)
1-5: LGTM!The imports and part directive are correctly configured for JSON serialization and widget definition.
7-43: LGTM!The documentation is comprehensive and includes helpful Dart and JSON examples. The previously reported issues with type names and JSON structure have been addressed.
43-65: LGTM!The class declaration and constructor are well-structured. The
explicitToJson: trueannotation is appropriate for nested serialization, and theenableTapToDismissdefault value oftruecorrectly matches Flutter's Tooltip default.
154-155: LGTM!The type getter correctly returns the tooltip widget type identifier.
157-163: LGTM!The serialization methods correctly delegate to the generated JSON helpers.
67-152: Theheightproperty omission is intentional and correct.Flutter deprecated the
heightproperty in favor ofconstraints(BoxConstraints) as of v3.30.0. TheStacTooltipalready provides theconstraintsproperty (StacBoxConstraints), which is the modern approach for controlling tooltip sizing.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.dart(1 hunks)
🔇 Additional comments (5)
packages/stac_core/lib/widgets/tool_tip/stac_tool_tip.dart (5)
1-5: LGTM!The imports and part directive are correctly structured for JSON serialization with the Stac framework.
7-42: LGTM!The documentation is comprehensive and provides clear examples in both Dart and JSON formats. All previously flagged issues have been addressed.
43-65: LGTM!The class declaration and constructor are well-structured. The
enableTapToDismiss = truedefault aligns with Flutter's Tooltip API.
133-153: LGTM!The remaining field declarations are well-documented and use appropriate types. The field coverage comprehensively mirrors Flutter's Tooltip API.
155-165: LGTM!The serialization methods follow the standard json_serializable pattern, and the type getter correctly returns
WidgetType.tooltip.name.
|
Hi @divyanshub024 , @Potatomonsta 👋, |
divyanshub024
left a 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.
Thank you for your contribution 🙌🚀
|
@divyanshub024 Should I also add StacTooltipThemeData in this PR? |
Thats a good idea. But can you please do it in another PR? |
|
CI is failing with a Flutter engine download error. |
Description
Add support for Tooltip widget in Stac.
Related Issues
Closes #396
Type of Change
Summary by CodeRabbit
New Features
Examples
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.