feat: add SelectableText widget and parser with documentation and exa…#395
feat: add SelectableText widget and parser with documentation and exa…#395divyanshub024 merged 6 commits intoStacDev:devfrom
Conversation
|
|
WalkthroughAdds SelectableText support: new serializable core model, generated JSON (de)serialization, parser, parser registration and export, docs and gallery example; removes a few unused parser imports and unpins some example dependency versions. Changes
Sequence Diagram(s)sequenceDiagram
participant JSON as JSON asset
participant Service as StacService (registry)
participant Parser as StacSelectableTextParser
participant Model as StacSelectableText
participant Flutter as Flutter SelectableText.rich
participant Stac as Stac (action handler)
JSON->>Service: request parse (type: selectableText)
Service->>Parser: dispatch to selectableText parser
Parser->>Model: StacSelectableText.fromJson(json)
Parser->>Parser: build TextSpan tree (children, styles, per-span taps)
Parser->>Flutter: return SelectableText.rich(textSpanTree, options)
Note over Flutter: renders selectable text (cursor, selection, semantics)
Flutter->>Stac: user taps span or widget (TapGestureRecognizer)
Stac->>Stac: onCallFromJson invoked (resolve action)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-19T20:20:55.561ZApplied to files:
🔇 Additional comments (1)
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: 5
🧹 Nitpick comments (2)
docs/widgets/selectable_text.mdx (1)
13-14: Clarify the relationship betweendataandchildrenproperties.The documentation lists both
data(for plain text) andchildren(for rich text with multiple spans), but doesn't explain that these represent two different usage patterns—similar to Flutter'sSelectableTextvsSelectableText.richconstructors. Consider adding a note that these are mutually exclusive.packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart (1)
53-68: Gesture recognizers in child spans are not disposed.
TapGestureRecognizerinstances created in_buildTextSpanare not explicitly disposed. While Flutter'sRichText(underlyingSelectableText.rich) does dispose recognizers when the widget is unmounted, this relies on internal behavior that may not hold in all scenarios (e.g., widget rebuilds with different children).Consider caching recognizers in a stateful widget or documenting the reliance on Flutter's internal disposal. For a stateless parser, the current approach is generally acceptable but worth noting for maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/docs.json(1 hunks)docs/widgets/selectable_text.mdx(1 hunks)examples/stac_gallery/assets/json/home_screen.json(1 hunks)examples/stac_gallery/assets/json/selectable_text_example.json(1 hunks)packages/stac/lib/src/framework/stac_service.dart(1 hunks)packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart(1 hunks)packages/stac/lib/src/parsers/widgets/widgets.dart(1 hunks)packages/stac_core/lib/foundation/specifications/widget_type.dart(1 hunks)packages/stac_core/lib/widgets/selectable_text/stac_selectable_text.dart(1 hunks)packages/stac_core/lib/widgets/widgets.dart(1 hunks)
🔇 Additional comments (14)
docs/widgets/selectable_text.mdx (1)
25-25: Verify thecursorRadiustype specification.The documentation specifies
cursorRadiusasdouble?, but Flutter's SelectableText API expects aRadius?object. Confirm whether the parser accepts a single double value and converts it to a Radius, or if this should be documented differently.docs/docs.json (1)
140-140: LGTM! Selectable text documentation properly integrated.The placement after "text" makes semantic sense, grouping text-related widgets together.
packages/stac_core/lib/widgets/widgets.dart (1)
64-64: LGTM! Export correctly placed in alphabetical order.The new selectable_text export is properly positioned between scaffold and set_value, maintaining the file's alphabetical organization.
packages/stac/lib/src/parsers/widgets/widgets.dart (1)
60-60: LGTM! Parser export correctly placed in alphabetical order.The new parser export maintains the file's alphabetical organization and aligns with the corresponding widget export.
packages/stac/lib/src/framework/stac_service.dart (1)
131-131: LGTM! Parser registered in the framework.The StacSelectableTextParser is properly instantiated and added to the global parsers list, enabling JSON-based parsing of selectable text widgets.
examples/stac_gallery/assets/json/home_screen.json (1)
822-843: LGTM! Gallery entry properly configured.The new list tile for Selectable Text is well-placed after the Text widget, uses an appropriate icon, and correctly navigates to the example file.
packages/stac_core/lib/foundation/specifications/widget_type.dart (1)
192-193: LGTM! Enum member correctly added in alphabetical order.The selectableText enum member is properly positioned and documented, maintaining consistency with the existing enum structure.
examples/stac_gallery/assets/json/selectable_text_example.json (1)
1-117: Excellent example coverage demonstrating key SelectableText features.The example file effectively showcases:
- Basic selectable text usage
- Rich text with styled children
- Cursor customization
- Disabling interactive selection
The structure is clear and well-organized with appropriate spacing between examples.
packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart (1)
14-22: LGTM!Parser class structure follows the established pattern with const constructor, type getter, and getModel delegation.
packages/stac_core/lib/widgets/selectable_text/stac_selectable_text.dart (5)
1-10: LGTM!Imports are appropriate and the
partdirective correctly references the generated JSON serialization file.
12-42: Documentation is thorough and helpful.The doc comments with both Dart and JSON examples provide clear usage guidance for developers.
43-67: LGTM!Constructor follows the established pattern with
dataas the only required field, matching Flutter'sSelectableTextAPI where the text content is mandatory.
126-127:selectionColormay not be directly applicable.As noted in the parser review, Flutter's
SelectableTextdoes not expose a directselectionColorparameter. The selection color is typically controlled viaTextSelectionTheme. Consider verifying whether this field can be utilized or should be removed to avoid confusion.
129-138: LGTM!The
typegetter and JSON serialization methods follow the established widget pattern.
packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart
Show resolved
Hide resolved
packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart
Outdated
Show resolved
Hide resolved
packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart
Outdated
Show resolved
Hide resolved
…ntroduce textScaler
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/stac_core/lib/widgets/selectable_text/stac_selectable_text.dart (1)
92-93: ClarifytextScalerdocumentation.The documentation says "font scaling strategy" but the field type is
double?, representing a scale factor (similar totextScaleFactor), not aTextScalerobject. The parser converts this toTextScaler.linear()during rendering.Consider updating the documentation to clarify:
- /// The font scaling strategy to use. + /// The text scale factor to use (converted to TextScaler.linear during rendering). final double? textScaler;Alternatively, if the field name
textScaleris meant to align with Flutter's API terminology, add a note explaining that it stores the numeric scale factor for JSON serialization purposes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/docs.json(1 hunks)docs/widgets/selectable_text.mdx(1 hunks)examples/stac_gallery/assets/json/selectable_text_example.json(1 hunks)packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart(1 hunks)packages/stac_core/lib/widgets/selectable_text/stac_selectable_text.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/widgets/selectable_text.mdx
- examples/stac_gallery/assets/json/selectable_text_example.json
🔇 Additional comments (6)
docs/docs.json (1)
136-136: LGTM!The navigation entry for the new SelectableText widget documentation is correctly placed in the Data Widgets group.
packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart (3)
1-22: LGTM!The parser structure follows the standard pattern with proper imports and type registration.
24-54: LGTM! All previous issues addressed.The parse method correctly:
- Includes
selectionColorparameter (previously missing)- Defaults
textScalertonullto preserve system accessibility scaling (previously usedTextScaler.noScaling)- Handles both the new
textScalerand deprecatedtextScaleFactorfields- Maps all model properties to the SelectableText.rich widget
56-71: LGTM!The TextSpan construction correctly handles hierarchical text with per-child styles and tap handlers.
packages/stac_core/lib/widgets/selectable_text/stac_selectable_text.dart (2)
1-42: LGTM!The documentation is comprehensive with clear examples in both Dart and JSON formats, properly illustrating the widget's usage.
134-143: LGTM!The type getter and JSON serialization methods follow the standard pattern with proper code generation setup.
packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart
Show resolved
Hide resolved
|
Hey @xkaper001 !! The PR checks are failing. Can you please check? |
…deprecated `textScaleFactor` property.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
examples/movie_app/pubspec.yaml (1)
37-38: Same concern as in counter_example/pubspec.yaml.The unpinned
stacandstac_coredependencies should resolve to local workspace packages. This is already flagged for verification in the counter_example review.examples/stac_gallery/pubspec.yaml (1)
40-41: Same concern as in counter_example/pubspec.yaml.The unpinned
stacandstac_webviewdependencies should resolve to local workspace packages. This is already flagged for verification in the counter_example review.packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart (1)
34-36: Accessibility issue persists: defaulttextScalerdisables system scaling.As flagged in previous reviews, setting
textScalertonullwhenmodel.textScaleris absent allows Flutter to use the system's accessibility text scale factor. However, the current implementation would need to conditionally omit the parameter entirely rather than passingnull, OR the SelectableText.rich constructor should handlenullappropriately.According to Flutter's API, when
textScaleris omitted, the widget inherits fromMediaQuery. Passing an explicit value (evennull) may override this behavior depending on the Flutter version.Does Flutter SelectableText.rich textScaler parameter accept null to use MediaQuery default?
🧹 Nitpick comments (1)
packages/stac_core/lib/widgets/selectable_text/stac_selectable_text.dart (1)
87-88: ClarifytextScalerfield documentation.The
textScalerfield is documented as "The font scaling strategy" but it's typed asdouble?. This differs from Flutter'sSelectableText.textScalerwhich expects aTextScalerobject. Consider documenting that this double value will be converted toTextScaler.linear()by the parser, or note the relationship to the deprecatedtextScaleFactorparameter.📝 Suggested documentation improvement
- /// The font scaling strategy to use. + /// The font scaling factor to apply to text. + /// + /// This value is converted to [TextScaler.linear] during parsing. + /// When null, the widget uses the system's text scale factor from [MediaQuery]. final double? textScaler;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/counter_example/pubspec.yaml(1 hunks)examples/movie_app/pubspec.yaml(1 hunks)examples/stac_gallery/pubspec.yaml(1 hunks)packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart(1 hunks)packages/stac_core/lib/widgets/selectable_text/stac_selectable_text.dart(1 hunks)packages/stac_core/lib/widgets/selectable_text/stac_selectable_text.g.dart(1 hunks)
🧰 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/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dartpackages/stac_core/lib/widgets/selectable_text/stac_selectable_text.dartpackages/stac_core/lib/widgets/selectable_text/stac_selectable_text.g.dart
🔇 Additional comments (7)
packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart (3)
47-47: Good:selectionColorparameter added.The previous review comment about missing
selectionColorhas been addressed. The parameter is now correctly passed toSelectableText.rich.
81-81: Good: Style resolution fixed for null base.The previous issue where
overrideParsedwas discarded whenbaseStyleis null has been resolved. The method now correctly returnsoverrideParsedin this case.
54-69: LGTM: TextSpan tree construction is correct.The
_buildTextSpanmethod properly constructs a nestedTextSpantree from the model, correctly handling:
- Root text (
model.data)- Child spans with individual styles
- Per-child tap gesture recognizers via
TapGestureRecognizerpackages/stac_core/lib/widgets/selectable_text/stac_selectable_text.g.dart (1)
1-91: LGTM: Generated JSON serialization code.The generated code correctly implements:
fromJsonwith proper null handling and type conversionstoJsonwith all fields including thetypediscriminator- Enum mappings for
StacTextAlign,StacTextDirection, andStacTextWidthBasisAs this is generated code, no manual changes should be made here.
packages/stac_core/lib/widgets/selectable_text/stac_selectable_text.dart (2)
12-42: LGTM: Clear documentation with examples.The documentation provides helpful Dart and JSON examples that demonstrate typical usage of the
StacSelectableTextwidget. The examples are concise and cover key properties.
43-139: LGTM: Well-structured widget model.The
StacSelectableTextclass is properly structured with:
- Comprehensive parameter set matching Flutter's
SelectableText- Proper JSON serialization annotations
- Type discriminator via
WidgetType.selectableText.name- Clear field documentation
examples/counter_example/pubspec.yaml (1)
37-37: Dependency resolution forstacis correctly configured via Melos monorepo pattern.The empty version constraint in
pubspec.yamlworks in conjunction withpubspec_overrides.yaml, which is the standard pattern for managing dependencies in pub workspaces/monorepos. Thepubspec_overrides.yamlfile explicitly provides a path override (path: ../../packages/stac), ensuring the example always resolves to the local workspace package. This is the intended design for this Melos-based monorepo and does not cause instability.Likely an incorrect or invalid review comment.
packages/stac/lib/src/parsers/widgets/stac_selectable_text/stac_selectable_text_parser.dart
Show resolved
Hide resolved
|
@divyanshub024 Does it look good? |
divyanshub024
left a comment
divyanshub024
left a comment
There was a problem hiding this comment.
Thank you for your contribution, @xkaper001 🎉 💯
3 participants
Description
SelectableTextwidget support end-to-end (core widget + framework parser).SelectableTextand includes it in the docs index.stac_galleryJSON to include aSelectableTextexample and wires it into the gallery home screen.Closes #385
Preview
Clipboard-20251215-214405-748.mp4
Type of Change
Summary by CodeRabbit
New Features
Documentation
Examples
Chores
✏️ Tip: You can customize this high-level summary in your review settings.