-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Add ViewModel capability #51
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
* Updated Makefile to include new test and lint commands. * Introduced ViewModelCapabilityDelegate for managing view interactions. * Exported view_capability.dart in index.dart. * Deprecated WidgetDisplayStateNotifier interface in favor of new capabilities.
WalkthroughAdds ViewModelCapabilityDelegate and DriverRefHolder, updates UIDriver and several capability delegates to use driver-linking, removes multiple deprecation annotations, adjusts focus API, adds scripting capability, new unit tests, mocktail dev dependency, and Makefile targets for test/lint/format. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks✅ Passed checks (3 passed)
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
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
Makefilelib/src/capabilities/index.dartlib/src/capabilities/view_capability.dartlib/src/driver_api/display_state_notifier.dartlib/src/driver_api/ui_driver.dart
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 17-17: Missing required phony target "all"
(minphony)
[warning] 17-17: Missing required phony target "clean"
(minphony)
⏰ 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). (3)
- GitHub Check: test (3.27.4)
- GitHub Check: test (3.29.3)
- GitHub Check: test (3.32.7)
🔇 Additional comments (6)
Makefile (1)
7-20: Good additions for development workflow.The new
test,lint, andformattargets are well-structured with proper.PHONYdeclarations. These will help standardize the development workflow.Regarding the static analysis hints about missing
allandcleantargets: these are common conventions but not strictly required. Consider adding them if you want a standard entry point (all) and cleanup mechanism (clean), but this is optional for a project-specific Makefile.lib/src/capabilities/index.dart (1)
4-4: LGTM!The export follows the existing pattern and correctly exposes the new
ViewModelCapabilityDelegatecapability to consumers.lib/src/driver_api/ui_driver.dart (2)
11-12: LGTM!The
ViewModelCapabilityDelegatemixin is correctly added toUIDriver, replacing the removedWidgetDisplayStateNotifierimplementation. This aligns well with the new capability-based architecture.
47-56: Well-documented initialization method.The
init()method has clear documentation explaining its purpose and return behavior. This provides a proper lifecycle hook for driver initialization.lib/src/capabilities/view_capability.dart (2)
5-12: LGTM on documentation quality.The mixin documentation clearly explains the purpose, usage context, and expected implementation behavior. This provides excellent guidance for implementers.
13-14: Ensuredriverfield is properly initialized in all implementations of capability delegate mixins.All capability delegate mixins declare
late final UIDriver driver;without an initialization mechanism. Classes mixing inViewModelCapabilityDelegate,FocusCapabilityDelegate,UIControllerCapabilityDelegate, orServerActionExecutionCapabilityDelegatemust explicitly initialize this field (typically via constructor or setter) before any methods depending on it are invoked. Document the initialization contract to prevent runtime errors in concrete implementations.
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
♻️ Duplicate comments (1)
lib/src/capabilities/view_capability.dart (1)
93-112: Consider using an enum for display state instead of magic integers.The
stateparameter accepts0(not visible) or1(visible). Using magic numbers reduces type safety and code clarity. An enum would make the API more discoverable and prevent invalid values.Based on past review comments, this suggestion has been previously made but not yet addressed.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
lib/src/action_api/action_executor.dartlib/src/action_api/event_resolver.dartlib/src/action_api/external_event_handler.dartlib/src/capabilities/scripting_capability.dartlib/src/capabilities/view_capability.dartlib/src/driver_api/ui_driver.dart
💤 Files with no reviewable changes (3)
- lib/src/action_api/external_event_handler.dart
- lib/src/action_api/action_executor.dart
- lib/src/action_api/event_resolver.dart
🔇 Additional comments (4)
lib/src/driver_api/ui_driver.dart (3)
7-12: LGTM: Successful integration of ViewModelCapabilityDelegate.The addition of
ViewModelCapabilityDelegateto the mixin list successfully migrates UIDriver to the new capability API. The removal ofWidgetDisplayStateNotifier(per the summary) indicates a clean transition away from the deprecated interface.
44-53: Good addition: Explicit initialization method.The new
init()method provides a clear, documented initialization point for UIDriver implementations. The async signature appropriately accommodates setup operations that may involve I/O or other asynchronous work.
55-63: LGTM: Clear deprecation with migration timeline.The deprecation of the
build()method with the message "Will be removed in the next major release" provides users with a clear signal and timeline for migration. This follows good API evolution practices.lib/src/capabilities/scripting_capability.dart (1)
1-1: Remove the unused empty mixin or implement it with documentation.
ScriptingCapabilityis not used anywhere in the codebase. Either remove it if it's not needed, or add a clear doc comment and implementation if it's intended for future use.Likely an incorrect or invalid review 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/src/capabilities/focus_capability.dart (1)
20-41: Update documentation to match the simplified signature.The documentation still references the removed
targetNodeIdparameter (lines 22-24), describing redirect behavior that no longer exists in the method signature. This creates confusion for implementers and API consumers.🔎 Proposed documentation fix
/// Requests that the focus be moved to the [FocusNode] associated with the given [nodeId]. /// - /// If [targetNodeId] is provided, the focus will move to the node represented by [targetNodeId] - /// rather than [nodeId] itself. This allows redirecting focus requests to other elements, such as - /// when managing focus scopes or proxy nodes. - /// /// Implementations must ensure the correct [FocusNode] is resolved and the focus request is /// performed, triggering any side effects or UI updates required. Typical use cases include: /// - Moving the input cursor to a text field, /// - Focusing a button or other interactive widget, /// - Coordinating focus changes in response to user actions or server commands. /// /// Throws [MissingCapabilityMethodImplementation] if not overridden. /// /// See also: /// - [FocusNode.requestFocus], for the underlying Flutter mechanism. @mustBeOverridden void requestFocus(String nodeId) => throw const MissingCapabilityMethodImplementation( "requestFocus", "FocusCapabilityDelegate", );
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
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
lib/src/capabilities/index.dartlib/src/capabilities/viewmodel_capability.dartpubspec.yamltest/viewmodel_capability_test.dart
🔇 Additional comments (4)
lib/src/capabilities/index.dart (1)
4-4: LGTM!The export correctly exposes the new ViewModelCapabilityDelegate mixin introduced in this PR.
test/viewmodel_capability_test.dart (1)
22-111: Comprehensive test coverage for all delegate methods.The test suite thoroughly verifies that all unimplemented methods throw
MissingMethodImplementation, establishing clear behavior expectations for implementers.lib/src/capabilities/viewmodel_capability.dart (1)
13-164: Well-structured capability interface with comprehensive documentation.The mixin establishes a clear contract for view model capabilities with thorough documentation for each member. The use of
@mustBeOverriddenand explicit exception throwing provides clear feedback to implementers.pubspec.yaml (1)
23-23: mocktail version is current with no security advisories.The ^1.0.4 constraint is already pinned to the latest stable version (1.0.4), and pub.dev reports no security advisories for mocktail. No action required.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/src/capabilities/focus_capability.dart (1)
28-49: Update documentation to match the simplified signature.The
requestFocusmethod signature was updated to remove the optionaltargetNodeIdparameter, but the documentation still references it. Lines 30-32 describetargetNodeIdbehavior that no longer exists.🔎 Proposed documentation fix
/// Requests that the focus be moved to the [FocusNode] associated with the given [nodeId]. /// - /// If [targetNodeId] is provided, the focus will move to the node represented by [targetNodeId] - /// rather than [nodeId] itself. This allows redirecting focus requests to other elements, such as - /// when managing focus scopes or proxy nodes. - /// /// Implementations must ensure the correct [FocusNode] is resolved and the focus request is /// performed, triggering any side effects or UI updates required. Typical use cases include: /// - Moving the input cursor to a text field, /// - Focusing a button or other interactive widget, /// - Coordinating focus changes in response to user actions or server commands. /// /// Throws [MissingCapabilityMethodImplementation] if not overridden. /// /// See also: /// - [FocusNode.requestFocus], for the underlying Flutter mechanism. @mustBeOverridden void requestFocus(String nodeId) =>
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
lib/src/capabilities/actions_capability.dartlib/src/capabilities/driver_ref.dartlib/src/capabilities/focus_capability.dartlib/src/capabilities/ui_controller_capability.dartlib/src/capabilities/viewmodel_capability.dartlib/src/driver_api/ui_driver.dart
🔇 Additional comments (19)
lib/src/capabilities/driver_ref.dart (1)
1-23: LGTM! Clean interface design for driver linkage.The
DriverRefHolderinterface provides a clear contract for capability delegates to maintain references to theirUIDriver. The documentation is comprehensive and the minimal API surface is appropriate for this coordination mechanism.lib/src/capabilities/ui_controller_capability.dart (1)
2-2: LGTM! Consistent DriverRefHolder integration.The capability delegate correctly implements the
DriverRefHolderpattern with a private_driverfield, protected getter, andlinkDrivermethod. Thelate finalmodifier is appropriate since the driver will be linked during initialization.Also applies to: 20-28
lib/src/capabilities/actions_capability.dart (1)
4-4: LGTM! Consistent DriverRefHolder implementation.The
ServerActionExecutionCapabilityDelegatefollows the same driver linkage pattern as other capability delegates, ensuring consistency across the codebase.Also applies to: 48-56
lib/src/capabilities/focus_capability.dart (1)
2-2: LGTM! Consistent DriverRefHolder integration.The
FocusCapabilityDelegatecorrectly implements theDriverRefHolderpattern, maintaining consistency with other capability delegates.Also applies to: 18-26
lib/src/driver_api/ui_driver.dart (3)
28-28: LGTM! Deprecation annotations updated consistently.The deprecation messages have been cleaned up to remove trailing periods, and the
build()method is now properly marked as deprecated. The messages clearly indicate removal in the next major release.Also applies to: 34-34, 38-38, 65-65
47-56: LGTM! Clear initialization hook added.The
init()method provides a well-documented initialization hook for concreteUIDriverimplementations. This is the appropriate place to calllinkDriverfor all capability delegates.
7-12: Ensure each capability delegate'slinkDriveris called separately during initialization.The four capability delegates—
FocusCapabilityDelegate,ServerActionExecutionCapabilityDelegate,UIControllerCapabilityDelegate, andViewModelCapabilityDelegate—each maintain their ownlate finaldriver reference. Due to Dart's mixin method resolution, callinglinkDriver(this)once will only initialize one delegate's driver reference. The other three will remain uninitialized, causingLateInitializationErrorwhen their methods access thedriverproperty.When implementing concrete UIDriver subclasses, ensure that
linkDriver(this)is called on each capability delegate individually, or provide a helper method that initializes all four.lib/src/capabilities/viewmodel_capability.dart (12)
1-5: LGTM: Clean imports.All imports are necessary and properly organized.
6-14: LGTM: Well-documented mixin declaration.The documentation clearly explains the mixin's purpose and responsibilities. Implementing
DriverRefHolderprovides a clean mechanism for driver linkage.
15-22: LGTM: Solid driver management implementation.The use of a private
_driverfield with a protected getter andlinkDrivermethod provides clean encapsulation and addresses concerns about initialization timing. TheDriverRefHolderinterface ensures a consistent pattern across capability delegates.
23-38: LGTM: Well-documented eventStream interface.Clear documentation explaining the expected behavior and usage. The
@mustBeOverriddenannotation with default throw ensures implementers provide concrete implementations.
40-46: LGTM: Clean buildContext interface.Consistent pattern with appropriate documentation.
48-64: LGTM: Comprehensive context setter documentation.Excellent documentation explaining when the setter is called and what implementers should do with the provided value. The examples (theme resolution, localization, widget tree interaction) help clarify usage.
66-82: LGTM: Clear event handling interface.Good documentation explaining how implementers should incorporate events into their handling pipeline, with helpful examples of different approaches.
84-98: LGTM: Appropriate error handling interface.Clear documentation of parameters and expected handling behaviors. The optional
stackTraceparameter is well-explained.
100-119: LGTM: Clear display state change notification.The method is well-documented with explicit explanation of the state parameter values (0 for not visible, 1 for visible).
121-137: LGTM: Clear widget readiness check interface.Straightforward method with clear documentation of parameters and return value.
139-157: LGTM: Comprehensive layout preparation interface.Excellent documentation explaining the parsing and construction responsibilities. The note about
nullindicating invalid/unrecognized layouts is helpful for implementers.
159-170: LGTM: Proper resource cleanup interface.Clear documentation about cleanup responsibilities, ensuring implementers close streams and event sources appropriately.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/src/driver_api/ui_driver.dart (2)
62-62: Update build() documentation to address nullable return type.The documentation for
build()does not align with its signature. The method returnsWidget?, but the documentation states "Returns: The widget representing the UI" without mentioning the nullable return or explaining whennullmight be returned.Update the documentation to clarify when
nullis returned and how callers should handle it:Suggested documentation improvement
/// Builds the UI. /// /// This method is responsible for building the user interface (UI) based on the /// current state of the UI driver. It creates and returns a widget that represents /// the UI to be rendered on the screen. /// - /// Returns: The widget representing the UI. + /// Returns: The widget representing the UI, or null if the driver is not yet + /// initialized or if there is no UI to display. Callers must handle null returns. Widget? build();
44-53: Clarify the lifecycle order and initialization contract for UIDriver implementations.The abstract
init()method lacks documentation about the expected calling sequence relative to other lifecycle methods. Add explicit guidance for implementers regarding:
- Whether
init()must be called beforebuild()- When driver linkage (capability setup) occurs relative to
init()- The expected order:
init()→build()→dispose()Example documentation clarification:
- "Must be called before
build()to ensure the driver is fully initialized."- "Implementers should perform capability setup and resource initialization in this method."
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/src/driver_api/ui_driver.dart
🔇 Additional comments (1)
lib/src/driver_api/ui_driver.dart (1)
28-36: No deprecation annotations were removed—this file was newly created.The git history shows that
lib/src/driver_api/ui_driver.dartwas just added in this commit. The memberseventResolver,actionExecutor, andexternalEventHandlerwere never deprecated in the first place. The concern raised appears to stem from inaccurate information in the PR summary and does not apply to this change.
| UIControllerCapabilityDelegate, | ||
| ViewModelCapabilityDelegate { |
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.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the UIDriver file to understand the structure
head -100 lib/src/driver_api/ui_driver.dartRepository: Duit-Foundation/duit_kernel
Length of output: 2368
🏁 Script executed:
# Search for ViewModelCapabilityDelegate definition
rg -n 'class ViewModelCapabilityDelegate|mixin ViewModelCapabilityDelegate' --type=dart -A 10Repository: Duit-Foundation/duit_kernel
Length of output: 924
🏁 Script executed:
# Search for UIDriver implementations
rg -n 'class.*extends UIDriver' --type=dartRepository: Duit-Foundation/duit_kernel
Length of output: 259
🏁 Script executed:
# Search for linkDriver calls in the codebase
rg -n 'linkDriver' --type=dart -B 2 -A 2Repository: Duit-Foundation/duit_kernel
Length of output: 2363
🏁 Script executed:
# Search for actual calls to linkDriver (not definitions)
rg -n '\.linkDriver\(' --type=dart -B 2 -A 2Repository: Duit-Foundation/duit_kernel
Length of output: 53
🏁 Script executed:
# Search for files containing "UIDriver" to find implementations
fd -e dart -x grep -l "UIDriver" {} \; | head -20Repository: Duit-Foundation/duit_kernel
Length of output: 663
🏁 Script executed:
# Look specifically for linkDriver(this) pattern
rg -n 'linkDriver\s*\(\s*this\s*\)' --type=dart -B 2 -A 2Repository: Duit-Foundation/duit_kernel
Length of output: 53
🏁 Script executed:
# Search for patterns where init() might call linkDriver
rg -n 'Future<void>\s+init\(' --type=dart -A 20Repository: Duit-Foundation/duit_kernel
Length of output: 4569
🏁 Script executed:
# Check testutil.dart for UIDriver implementation patterns
cat lib/testutil.dart 2>/dev/null || cat test/testutil.dart 2>/dev/null | head -150Repository: Duit-Foundation/duit_kernel
Length of output: 3233
🏁 Script executed:
# Check viewmodel_capability_test.dart
cat test/viewmodel_capability_test.dart | head -150Repository: Duit-Foundation/duit_kernel
Length of output: 3152
🏁 Script executed:
# Search more broadly for driver setup or initialization
rg -n 'init\(\)' --type=dart -A 10 | grep -A 10 'UIDriver\|DriverRef'Repository: Duit-Foundation/duit_kernel
Length of output: 53
🏁 Script executed:
# Check if this is a public package with external implementations
cat pubspec.yaml 2>/dev/nullRepository: Duit-Foundation/duit_kernel
Length of output: 568
Document the driver linkage requirement in the new init() method.
The addition of capability mixins (particularly ViewModelCapabilityDelegate) introduces a dependency on the DriverRefHolder pattern, where linkDriver(this) must be called to establish the connection. This requirement is implicit but not documented in the abstract interface.
Since all four capability mixins follow this pattern, implementations should call linkDriver(this) during initialization. Add a documentation note to the init() method or provide a comment explaining that all capability mixins require driver linkage to be established during initialization.
🤖 Prompt for AI Agents
In lib/src/driver_api/ui_driver.dart around lines 11-12, the new capability
mixins (including ViewModelCapabilityDelegate) require the DriverRefHolder
pattern and expect implementations to call linkDriver(this) during
initialization; update the init() method documentation to state this requirement
and add a short comment explaining that all capability mixins must have driver
linkage established via linkDriver(this) in init() so implementations know to
perform that call.
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
Fix all issues with AI Agents 🤖
In @lib/src/capabilities/actions_capability.dart:
- Around line 48-55: The thrown MissingCapabilityMethodImplementation in
linkDriver of ServerActionExecutionCapabilityDelegate has the wrong message;
update the exception construction in the linkDriver override to reference
"linkDriver" and "ServerActionExecutionCapabilityDelegate" (i.e., replace the
current "eventStream" and "ViewModelCapabilityDelegate" strings with the correct
method and delegate names) so the error message correctly identifies the missing
implementation.
In @lib/src/capabilities/focus_capability.dart:
- Around line 18-25: The thrown MissingCapabilityMethodImplementation in the
mixin FocusCapabilityDelegate has the wrong method and class names; update the
linkDriver implementation so the exception references "linkDriver" and
"FocusCapabilityDelegate" instead of "eventStream" and
"ViewModelCapabilityDelegate" (i.e., in the mixin method linkDriver(UIDriver)
replace the two string args passed to MissingCapabilityMethodImplementation with
the correct identifiers).
In @lib/src/capabilities/ui_controller_capability.dart:
- Around line 20-27: The error thrown in the mixin method linkDriver uses the
wrong identifiers ("eventStream" and "ViewModelCapabilityDelegate"), so update
the MissingCapabilityMethodImplementation invocation inside
UIControllerCapabilityDelegate::linkDriver to reference the correct method name
"linkDriver" and the correct capability/mixin name
"UIControllerCapabilityDelegate" (i.e., replace the two string arguments passed
to MissingCapabilityMethodImplementation accordingly).
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/src/capabilities/focus_capability.dart (1)
27-48: Update documentation to match simplified API.The documentation (lines 29-31) references a
targetNodeIdparameter that was removed from therequestFocussignature at line 44. This creates confusion about the method's actual behavior.🔎 Proposed fix
/// Requests that the focus be moved to the [FocusNode] associated with the given [nodeId]. /// - /// If [targetNodeId] is provided, the focus will move to the node represented by [targetNodeId] - /// rather than [nodeId] itself. This allows redirecting focus requests to other elements, such as - /// when managing focus scopes or proxy nodes. - /// /// Implementations must ensure the correct [FocusNode] is resolved and the focus request is /// performed, triggering any side effects or UI updates required. Typical use cases include:
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
lib/src/capabilities/actions_capability.dartlib/src/capabilities/driver_ref.dartlib/src/capabilities/focus_capability.dartlib/src/capabilities/ui_controller_capability.dartlib/src/capabilities/viewmodel_capability.dart
🔇 Additional comments (4)
lib/src/capabilities/driver_ref.dart (1)
1-19: Clean interface design.The
DriverRefHolderinterface provides a clear contract for capability delegates to establish their driver reference. The documentation is thorough and the single-method design keeps the interface focused.lib/src/capabilities/viewmodel_capability.dart (2)
14-21: Correct linkDriver implementation for this delegate.Unlike the other capability delegates in this PR, the
linkDrivererror message here correctly references"eventStream"and"ViewModelCapabilityDelegate", which matches the actual delegate name. This is the intended pattern.
23-157: Well-structured capability delegate interface.The
ViewModelCapabilityDelegateprovides a comprehensive contract for view model interactions. All methods are properly annotated with@mustBeOverriddenand have clear documentation explaining their purpose and usage. The error messages consistently reference the correct delegate name.Note: The past review comment regarding using an enum instead of magic integers (0, 1) for display state at lines 105-108 remains valid, but this is a minor improvement that can be addressed separately.
lib/src/capabilities/focus_capability.dart (1)
44-48: Update documentation to reflect the simplifiedrequestFocussignature.The method documentation (lines 27-31) still references the removed optional
{String? targetNodeId}parameter, describing behavior that no longer exists in the simplified signature. All callers have been properly updated to use the new signaturerequestFocus(String nodeId), but the documentation must be revised to remove references to thetargetNodeIdfunctionality and clarify the current behavior.The documentation should explain that the method now only accepts a
nodeIdand focus is moved directly to that node, without the option for redirection.
| mixin ServerActionExecutionCapabilityDelegate implements DriverRefHolder { | ||
| @override | ||
| @mustBeOverridden | ||
| void linkDriver(UIDriver driver) => | ||
| throw const MissingCapabilityMethodImplementation( | ||
| "eventStream", | ||
| "ViewModelCapabilityDelegate", | ||
| ); |
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.
Fix incorrect error message in linkDriver.
The MissingCapabilityMethodImplementation thrown by linkDriver references "eventStream" and "ViewModelCapabilityDelegate", but this method is linkDriver in ServerActionExecutionCapabilityDelegate. This will confuse implementers who see an error mentioning the wrong method and delegate.
🔎 Proposed fix
@override
@mustBeOverridden
void linkDriver(UIDriver driver) =>
throw const MissingCapabilityMethodImplementation(
- "eventStream",
- "ViewModelCapabilityDelegate",
+ "linkDriver",
+ "ServerActionExecutionCapabilityDelegate",
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mixin ServerActionExecutionCapabilityDelegate implements DriverRefHolder { | |
| @override | |
| @mustBeOverridden | |
| void linkDriver(UIDriver driver) => | |
| throw const MissingCapabilityMethodImplementation( | |
| "eventStream", | |
| "ViewModelCapabilityDelegate", | |
| ); | |
| mixin ServerActionExecutionCapabilityDelegate implements DriverRefHolder { | |
| @override | |
| @mustBeOverridden | |
| void linkDriver(UIDriver driver) => | |
| throw const MissingCapabilityMethodImplementation( | |
| "linkDriver", | |
| "ServerActionExecutionCapabilityDelegate", | |
| ); |
🤖 Prompt for AI Agents
In @lib/src/capabilities/actions_capability.dart around lines 48-55, The thrown
MissingCapabilityMethodImplementation in linkDriver of
ServerActionExecutionCapabilityDelegate has the wrong message; update the
exception construction in the linkDriver override to reference "linkDriver" and
"ServerActionExecutionCapabilityDelegate" (i.e., replace the current
"eventStream" and "ViewModelCapabilityDelegate" strings with the correct method
and delegate names) so the error message correctly identifies the missing
implementation.
| mixin FocusCapabilityDelegate implements DriverRefHolder { | ||
| @override | ||
| @mustBeOverridden | ||
| void linkDriver(UIDriver driver) => | ||
| throw const MissingCapabilityMethodImplementation( | ||
| "eventStream", | ||
| "ViewModelCapabilityDelegate", | ||
| ); |
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.
Fix incorrect error message in linkDriver.
The MissingCapabilityMethodImplementation references "eventStream" and "ViewModelCapabilityDelegate", but this is the linkDriver method in FocusCapabilityDelegate.
🔎 Proposed fix
@override
@mustBeOverridden
void linkDriver(UIDriver driver) =>
throw const MissingCapabilityMethodImplementation(
- "eventStream",
- "ViewModelCapabilityDelegate",
+ "linkDriver",
+ "FocusCapabilityDelegate",
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mixin FocusCapabilityDelegate implements DriverRefHolder { | |
| @override | |
| @mustBeOverridden | |
| void linkDriver(UIDriver driver) => | |
| throw const MissingCapabilityMethodImplementation( | |
| "eventStream", | |
| "ViewModelCapabilityDelegate", | |
| ); | |
| mixin FocusCapabilityDelegate implements DriverRefHolder { | |
| @override | |
| @mustBeOverridden | |
| void linkDriver(UIDriver driver) => | |
| throw const MissingCapabilityMethodImplementation( | |
| "linkDriver", | |
| "FocusCapabilityDelegate", | |
| ); |
🤖 Prompt for AI Agents
In @lib/src/capabilities/focus_capability.dart around lines 18-25, The thrown
MissingCapabilityMethodImplementation in the mixin FocusCapabilityDelegate has
the wrong method and class names; update the linkDriver implementation so the
exception references "linkDriver" and "FocusCapabilityDelegate" instead of
"eventStream" and "ViewModelCapabilityDelegate" (i.e., in the mixin method
linkDriver(UIDriver) replace the two string args passed to
MissingCapabilityMethodImplementation with the correct identifiers).
| mixin UIControllerCapabilityDelegate implements DriverRefHolder { | ||
| @override | ||
| @mustBeOverridden | ||
| void linkDriver(UIDriver driver) => | ||
| throw const MissingCapabilityMethodImplementation( | ||
| "eventStream", | ||
| "ViewModelCapabilityDelegate", | ||
| ); |
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.
Fix incorrect error message in linkDriver.
The MissingCapabilityMethodImplementation references "eventStream" and "ViewModelCapabilityDelegate", but this is the linkDriver method in UIControllerCapabilityDelegate. Implementers will receive a misleading error message pointing to the wrong capability.
🔎 Proposed fix
@override
@mustBeOverridden
void linkDriver(UIDriver driver) =>
throw const MissingCapabilityMethodImplementation(
- "eventStream",
- "ViewModelCapabilityDelegate",
+ "linkDriver",
+ "UIControllerCapabilityDelegate",
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mixin UIControllerCapabilityDelegate implements DriverRefHolder { | |
| @override | |
| @mustBeOverridden | |
| void linkDriver(UIDriver driver) => | |
| throw const MissingCapabilityMethodImplementation( | |
| "eventStream", | |
| "ViewModelCapabilityDelegate", | |
| ); | |
| mixin UIControllerCapabilityDelegate implements DriverRefHolder { | |
| @override | |
| @mustBeOverridden | |
| void linkDriver(UIDriver driver) => | |
| throw const MissingCapabilityMethodImplementation( | |
| "linkDriver", | |
| "UIControllerCapabilityDelegate", | |
| ); |
🤖 Prompt for AI Agents
In @lib/src/capabilities/ui_controller_capability.dart around lines 20-27, The
error thrown in the mixin method linkDriver uses the wrong identifiers
("eventStream" and "ViewModelCapabilityDelegate"), so update the
MissingCapabilityMethodImplementation invocation inside
UIControllerCapabilityDelegate::linkDriver to reference the correct method name
"linkDriver" and the correct capability/mixin name
"UIControllerCapabilityDelegate" (i.e., replace the two string arguments passed
to MissingCapabilityMethodImplementation accordingly).
Summary by CodeRabbit
New Features
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.