-
-
Notifications
You must be signed in to change notification settings - Fork 73
refactor: Move StacAlign to Stac_core for DSL support #357
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
WalkthroughRemoves the Freezed-based StacAlign model and its JSON helpers from packages/stac, updates the align parser and exports to reference a new JsonSerializable StacAlign model added under packages/stac_core. Also updates SnackBar action serialization to include actionType in toJson. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Parser as StacAlignParser (stac)
participant Model as StacAlign (stac_core)
note over Model: New JsonSerializable model
Client->>Parser: parse(json)
Parser->>Parser: read alignment/width/height/child (null-safe)
Parser->>Model: create StacAlign.fromJson-like data<br/>or construct with parsed fields
Model-->>Parser: instance
Parser-->>Client: StacAlign instance (with type)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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: 1
🧹 Nitpick comments (2)
packages/stac/lib/src/parsers/widgets/stac_align/stac_align_parser.dart (1)
19-23
: Safe defaults; can simplify once model guarantees non-null alignmentAfter making
alignment
non-null with a default in the core model (see comment on stac_align.dart), this can be simplified.- alignment: model.alignment?.parse ?? Alignment.center, + alignment: model.alignment.parse,packages/stac_core/lib/widgets/align/stac_align.g.dart (1)
23-29
: toJson forces non-null alignment; risk of runtime exception
_$StacAlignToJson
uses_$StacAlignmentDirectionalEnumMap[instance.alignment]!
, which will throw if aStacAlign
is constructed programmatically withalignment == null
. The field is nullable in the model, so this is a real foot‑gun outside of JSON construction.Fix in the model by making
alignment
non‑null with a default (and add@JsonKey(defaultValue: StacAlignmentDirectional.center)
), then regenerate. See suggested diff on stac_align.dart.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/stac/lib/src/parsers/widgets/stac_align/stac_align.dart
(0 hunks)packages/stac/lib/src/parsers/widgets/stac_align/stac_align.freezed.dart
(0 hunks)packages/stac/lib/src/parsers/widgets/stac_align/stac_align.g.dart
(0 hunks)packages/stac/lib/src/parsers/widgets/stac_align/stac_align_parser.dart
(1 hunks)packages/stac/lib/src/parsers/widgets/widgets.dart
(1 hunks)packages/stac_core/lib/actions/snack_bar/stac_snack_bar_action.g.dart
(1 hunks)packages/stac_core/lib/widgets/align/stac_align.dart
(1 hunks)packages/stac_core/lib/widgets/align/stac_align.g.dart
(1 hunks)packages/stac_core/lib/widgets/widgets.dart
(1 hunks)
💤 Files with no reviewable changes (3)
- packages/stac/lib/src/parsers/widgets/stac_align/stac_align.g.dart
- packages/stac/lib/src/parsers/widgets/stac_align/stac_align.dart
- packages/stac/lib/src/parsers/widgets/stac_align/stac_align.freezed.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). (1)
- GitHub Check: analyze
🔇 Additional comments (7)
packages/stac_core/lib/actions/snack_bar/stac_snack_bar_action.g.dart (1)
29-30
: No change required —actionType
is a derived getter; serializing-only is intentional.Verified: packages/stac_core/lib/actions/stac_snack_bar_action.dart defines
String get actionType => ActionType.showSnackBar.name;
(not a constructor field), sotoJson
writing it whilefromJson
ignores it is correct.packages/stac/lib/src/parsers/widgets/stac_align/stac_align_parser.dart (2)
11-11
: LGTM: type mapping is consistent
WidgetType.align.name
correctly identifies the parser type.
14-14
: LGTM: switching to core modelUsing
StacAlign.fromJson
fromstac_core
is correct for the refactor.packages/stac_core/lib/widgets/widgets.dart (1)
4-4
: Expose core Align in barrelExport looks good and maintains ordering.
packages/stac/lib/src/parsers/widgets/widgets.dart (1)
4-4
: Switched to parser exportExporting
stac_align_parser.dart
aligns with moving the model to core. Looks good.packages/stac_core/lib/widgets/align/stac_align.g.dart (1)
9-16
: LGTM: sensible default on deserializationFalling back to
StacAlignmentDirectional.center
whenalignment
is missing is correct.packages/stac_core/lib/widgets/align/stac_align.dart (1)
57-60
: LGTM: stable type identifier
WidgetType.align.name
matches the DSL type key and is consistent with other widgets.
Description
Move StacAlign to Stac_core for DSL support
Type of Change
Summary by CodeRabbit
New Features
Refactor