Skip to content

Conversation

@lesleysin
Copy link
Member

@lesleysin lesleysin commented Jan 3, 2026

  • 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.

Summary by CodeRabbit

  • New Features

    • Added build commands: test, lint, and format for local development
    • Introduced ViewModel and Scripting capabilities; added init() to driver lifecycle
  • Refactor

    • Simplified focus request API to single-target form
    • Capability delegates now use explicit driver references; several deprecation annotations removed
  • Tests

    • Added unit tests for default ViewModel capability behaviors
  • Chores

    • Added a mocking library to dev dependencies

✏️ Tip: You can customize this high-level summary in your review settings.

* 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.
@lesleysin lesleysin self-assigned this Jan 3, 2026
@lesleysin lesleysin changed the title feat: Enhance Makefile and add ViewModel capability feat: Add ViewModel capability Jan 3, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 3, 2026

Walkthrough

Adds 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

Cohort / File(s) Summary
New capability & export
lib/src/capabilities/viewmodel_capability.dart, lib/src/capabilities/index.dart
Introduces ViewModelCapabilityDelegate mixin with @mustBeOverridden members (linkDriver, eventStream, buildContext/context setter, addUIDriverEvent/error, notifyWidgetDisplayStateChanged, isWidgetReady, prepareLayout, releaseResources); exports it.
Driver ref interface
lib/src/capabilities/driver_ref.dart
Adds DriverRefHolder interface declaring void linkDriver(UIDriver driver) to standardize driver linkage for capability delegates.
Capabilities adopt DriverRefHolder
lib/src/capabilities/*
lib/src/capabilities/actions_capability.dart, .../ui_controller_capability.dart, .../focus_capability.dart
Several capability delegates now implements DriverRefHolder, remove in-mixin driver fields, and add linkDriver(UIDriver) (default throws MissingCapabilityMethodImplementation).
Focus capability change
lib/src/capabilities/focus_capability.dart
requestFocus signature changed to void requestFocus(String nodeId) (removed optional targetNodeId); integrates DriverRefHolder (adds linkDriver).
New empty capability
lib/src/capabilities/scripting_capability.dart
Adds an empty ScriptingCapability mixin placeholder.
UIDriver API changes
lib/src/driver_api/ui_driver.dart
UIDriver now mixes in/uses ViewModelCapabilityDelegate via driver linkage, removes buildContext/eventStream properties and related setters, adds Future<void> init(), and removes WidgetDisplayStateNotifier from implements; per-property deprecation annotations removed.
Undeprecation of action APIs
lib/src/action_api/action_executor.dart, lib/src/action_api/event_resolver.dart, lib/src/action_api/external_event_handler.dart
Removed @Deprecated(...) annotations from ActionExecutor/DefaultActionExecutor, EventResolver/DefaultEventResolver, and ExternalEventHandler.
Tests
test/viewmodel_capability_test.dart
New unit tests asserting default ViewModelCapabilityDelegate members throw MissingCapabilityMethodImplementation.
Dev deps & Makefile
pubspec.yaml, Makefile
Added mocktail: ^1.0.4 to dev_dependencies; added Makefile targets: test (fvm flutter test -j=10), lint (fvm flutter analyze .), format (fvm dart format .).

Sequence Diagram(s)

mermaid
sequenceDiagram
actor External
participant UIDriver
participant ViewModelCap as ViewModelCapabilityDelegate
participant Capability as CapabilityDelegate
Note over UIDriver,ViewModelCap: Initialization (new linkDriver flow)
UIDriver->>ViewModelCap: linkDriver(driver)
UIDriver->>Capability: linkDriver(driver)
Note over External,ViewModelCap: Runtime event flow
External->>ViewModelCap: send UIDriverEvent
ViewModelCap->>UIDriver: addUIDriverEvent(event)
ViewModelCap->>UIDriver: addUIDriverError(error)
ViewModelCap->>UIDriver: notifyWidgetDisplayStateChanged(viewTag,state)
Note right of UIDriver: event handling / routing

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: Add ViewModel capability' directly and clearly summarizes the main change in the changeset - introducing ViewModelCapabilityDelegate and related capability infrastructure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9c33175 and 424eecc.

📒 Files selected for processing (5)
  • Makefile
  • lib/src/capabilities/index.dart
  • lib/src/capabilities/view_capability.dart
  • lib/src/driver_api/display_state_notifier.dart
  • lib/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, and format targets are well-structured with proper .PHONY declarations. These will help standardize the development workflow.

Regarding the static analysis hints about missing all and clean targets: 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 ViewModelCapabilityDelegate capability to consumers.

lib/src/driver_api/ui_driver.dart (2)

11-12: LGTM!

The ViewModelCapabilityDelegate mixin is correctly added to UIDriver, replacing the removed WidgetDisplayStateNotifier implementation. 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: Ensure driver field 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 in ViewModelCapabilityDelegate, FocusCapabilityDelegate, UIControllerCapabilityDelegate, or ServerActionExecutionCapabilityDelegate must 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.

Copy link

@coderabbitai coderabbitai bot left a 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 state parameter accepts 0 (not visible) or 1 (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

📥 Commits

Reviewing files that changed from the base of the PR and between 424eecc and 9a1fe33.

📒 Files selected for processing (6)
  • lib/src/action_api/action_executor.dart
  • lib/src/action_api/event_resolver.dart
  • lib/src/action_api/external_event_handler.dart
  • lib/src/capabilities/scripting_capability.dart
  • lib/src/capabilities/view_capability.dart
  • lib/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 ViewModelCapabilityDelegate to the mixin list successfully migrates UIDriver to the new capability API. The removal of WidgetDisplayStateNotifier (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.

ScriptingCapability is 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.

Copy link

@coderabbitai coderabbitai bot left a 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 targetNodeId parameter (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",
      );
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a1fe33 and dc6a79d.

📒 Files selected for processing (1)
  • lib/src/capabilities/focus_capability.dart

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between dc6a79d and 64a135e.

📒 Files selected for processing (4)
  • lib/src/capabilities/index.dart
  • lib/src/capabilities/viewmodel_capability.dart
  • pubspec.yaml
  • test/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 @mustBeOverridden and 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.

Copy link

@coderabbitai coderabbitai bot left a 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 requestFocus method signature was updated to remove the optional targetNodeId parameter, but the documentation still references it. Lines 30-32 describe targetNodeId behavior 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

📥 Commits

Reviewing files that changed from the base of the PR and between 64a135e and a5c3112.

📒 Files selected for processing (6)
  • lib/src/capabilities/actions_capability.dart
  • lib/src/capabilities/driver_ref.dart
  • lib/src/capabilities/focus_capability.dart
  • lib/src/capabilities/ui_controller_capability.dart
  • lib/src/capabilities/viewmodel_capability.dart
  • lib/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 DriverRefHolder interface provides a clear contract for capability delegates to maintain references to their UIDriver. 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 DriverRefHolder pattern with a private _driver field, protected getter, and linkDriver method. The late final modifier 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 ServerActionExecutionCapabilityDelegate follows 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 FocusCapabilityDelegate correctly implements the DriverRefHolder pattern, 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 concrete UIDriver implementations. This is the appropriate place to call linkDriver for all capability delegates.


7-12: Ensure each capability delegate's linkDriver is called separately during initialization.

The four capability delegates—FocusCapabilityDelegate, ServerActionExecutionCapabilityDelegate, UIControllerCapabilityDelegate, and ViewModelCapabilityDelegate—each maintain their own late final driver reference. Due to Dart's mixin method resolution, calling linkDriver(this) once will only initialize one delegate's driver reference. The other three will remain uninitialized, causing LateInitializationError when their methods access the driver property.

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 DriverRefHolder provides a clean mechanism for driver linkage.


15-22: LGTM: Solid driver management implementation.

The use of a private _driver field with a protected getter and linkDriver method provides clean encapsulation and addresses concerns about initialization timing. The DriverRefHolder interface ensures a consistent pattern across capability delegates.


23-38: LGTM: Well-documented eventStream interface.

Clear documentation explaining the expected behavior and usage. The @mustBeOverridden annotation 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 stackTrace parameter 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 null indicating 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.

Copy link

@coderabbitai coderabbitai bot left a 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 returns Widget?, but the documentation states "Returns: The widget representing the UI" without mentioning the nullable return or explaining when null might be returned.

Update the documentation to clarify when null is 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:

  1. Whether init() must be called before build()
  2. When driver linkage (capability setup) occurs relative to init()
  3. 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

📥 Commits

Reviewing files that changed from the base of the PR and between a5c3112 and 9c1841e.

📒 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.dart was just added in this commit. The members eventResolver, actionExecutor, and externalEventHandler were 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.

Comment on lines +11 to +12
UIControllerCapabilityDelegate,
ViewModelCapabilityDelegate {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's read the UIDriver file to understand the structure
head -100 lib/src/driver_api/ui_driver.dart

Repository: Duit-Foundation/duit_kernel

Length of output: 2368


🏁 Script executed:

# Search for ViewModelCapabilityDelegate definition
rg -n 'class ViewModelCapabilityDelegate|mixin ViewModelCapabilityDelegate' --type=dart -A 10

Repository: Duit-Foundation/duit_kernel

Length of output: 924


🏁 Script executed:

# Search for UIDriver implementations
rg -n 'class.*extends UIDriver' --type=dart

Repository: 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 2

Repository: 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 2

Repository: 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 -20

Repository: 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 2

Repository: 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 20

Repository: 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 -150

Repository: Duit-Foundation/duit_kernel

Length of output: 3233


🏁 Script executed:

# Check viewmodel_capability_test.dart
cat test/viewmodel_capability_test.dart | head -150

Repository: 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/null

Repository: 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.

Copy link

@coderabbitai coderabbitai bot left a 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 targetNodeId parameter that was removed from the requestFocus signature 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9c1841e and 8770e57.

📒 Files selected for processing (5)
  • lib/src/capabilities/actions_capability.dart
  • lib/src/capabilities/driver_ref.dart
  • lib/src/capabilities/focus_capability.dart
  • lib/src/capabilities/ui_controller_capability.dart
  • lib/src/capabilities/viewmodel_capability.dart
🔇 Additional comments (4)
lib/src/capabilities/driver_ref.dart (1)

1-19: Clean interface design.

The DriverRefHolder interface 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 linkDriver error 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 ViewModelCapabilityDelegate provides a comprehensive contract for view model interactions. All methods are properly annotated with @mustBeOverridden and 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 simplified requestFocus signature.

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 signature requestFocus(String nodeId), but the documentation must be revised to remove references to the targetNodeId functionality and clarify the current behavior.

The documentation should explain that the method now only accepts a nodeId and focus is moved directly to that node, without the option for redirection.

Comment on lines +48 to +55
mixin ServerActionExecutionCapabilityDelegate implements DriverRefHolder {
@override
@mustBeOverridden
void linkDriver(UIDriver driver) =>
throw const MissingCapabilityMethodImplementation(
"eventStream",
"ViewModelCapabilityDelegate",
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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.

Comment on lines +18 to +25
mixin FocusCapabilityDelegate implements DriverRefHolder {
@override
@mustBeOverridden
void linkDriver(UIDriver driver) =>
throw const MissingCapabilityMethodImplementation(
"eventStream",
"ViewModelCapabilityDelegate",
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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).

Comment on lines +20 to +27
mixin UIControllerCapabilityDelegate implements DriverRefHolder {
@override
@mustBeOverridden
void linkDriver(UIDriver driver) =>
throw const MissingCapabilityMethodImplementation(
"eventStream",
"ViewModelCapabilityDelegate",
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
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).

@lesleysin lesleysin merged commit b1348d3 into main Jan 5, 2026
11 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants