Conversation
WalkthroughIntroduces a focus management system to the duit_attributes package. Adds FocusNode configuration type, FocusCommand action abstraction, and FocusCapability wrapper. Embeds FocusCapability into six attribute types (Checkbox, ElevatedButton, Inkwell, Radio, Switch, TextField) with delegating SetFocusNode methods. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
pkg/duit_attributes/checkbox_attributes.go (1)
33-47: Potential nil-pointer dereference in Validate method.The
Validate()method callsr.ThemeConsumer.Validate()andr.ValueReferenceHolder.Validate()on embedded pointer fields without nil-checks. If these embedded fields are nil, this will panic.Per coding guidelines, callers must check for nil before calling methods on potentially nil receivers.
🔎 Proposed fix with nil guards
func (r *CheckboxAttributes) Validate() error { - if err := r.ThemeConsumer.Validate(); err != nil { - return err + if r.ThemeConsumer != nil { + if err := r.ThemeConsumer.Validate(); err != nil { + return err + } } - if err := r.ValueReferenceHolder.Validate(); err != nil { - return err + if r.ValueReferenceHolder != nil { + if err := r.ValueReferenceHolder.Validate(); err != nil { + return err + } } if r.Value == nil { return errors.New("value property is required") } return nil }pkg/duit_attributes/switch_attribytes.go (2)
1-1: Filename typo: "attribytes" should be "attributes".The filename
switch_attribytes.gocontains a spelling error. This should beswitch_attributes.gofor consistency with other attribute files (e.g.,checkbox_attributes.go).
122-136: Potential nil-pointer dereference in Validate method.Same issue as in
CheckboxAttributes: theValidate()method calls methods on embedded pointer fields (ValueReferenceHolder,ThemeConsumer) without nil-checks.🔎 Proposed fix with nil guards
func (r *SwitchAttributes) Validate() error { - if err := r.ValueReferenceHolder.Validate(); err != nil { - return err + if r.ValueReferenceHolder != nil { + if err := r.ValueReferenceHolder.Validate(); err != nil { + return err + } } - if err := r.ThemeConsumer.Validate(); err != nil { - return err + if r.ThemeConsumer != nil { + if err := r.ThemeConsumer.Validate(); err != nil { + return err + } } if r.Value == nil { return errors.New("value property is required") } return nil }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
pkg/duit_attributes/checkbox_attributes.go(1 hunks)pkg/duit_attributes/elevated_buttion_attributes.go(1 hunks)pkg/duit_attributes/focus_capability.go(1 hunks)pkg/duit_attributes/focus_command.go(1 hunks)pkg/duit_attributes/inkwell_attributes.go(1 hunks)pkg/duit_attributes/radio_attributes.go(1 hunks)pkg/duit_attributes/switch_attribytes.go(1 hunks)pkg/duit_attributes/text_field_attributes.go(2 hunks)pkg/duit_attributes/zz_embedded_structs_delegates.go(1 hunks)pkg/duit_props/focus_node.go(1 hunks)pkg/duit_props/traversal_direction.go(1 hunks)pkg/duit_props/unfocus_dispodition.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
pkg/**/*.go
📄 CodeRabbit inference engine (pkg/.cursor/rules/style.mdc)
pkg/**/*.go: When declaring new structure methods, the receiver name must always be "r"
If a caller calls a method on an object that is potentially a nil-receiver, it must explicitly check the object for nil before the call
Files:
pkg/duit_attributes/checkbox_attributes.gopkg/duit_attributes/text_field_attributes.gopkg/duit_props/traversal_direction.gopkg/duit_props/focus_node.gopkg/duit_attributes/radio_attributes.gopkg/duit_props/unfocus_dispodition.gopkg/duit_attributes/focus_capability.gopkg/duit_attributes/zz_embedded_structs_delegates.gopkg/duit_attributes/focus_command.gopkg/duit_attributes/elevated_buttion_attributes.gopkg/duit_attributes/switch_attribytes.gopkg/duit_attributes/inkwell_attributes.go
🧠 Learnings (1)
📚 Learning: 2025-12-08T16:29:08.617Z
Learnt from: CR
Repo: Duit-Foundation/duit_go PR: 0
File: pkg/.cursor/rules/testing.mdc:0-0
Timestamp: 2025-12-08T16:29:08.617Z
Learning: Applies to pkg/**/*_test.go : Always add tests for a struct property if the property is of type duit_utils.Tristate[T], including serialization to JSON
Applied to files:
pkg/duit_props/traversal_direction.go
🧬 Code graph analysis (7)
pkg/duit_attributes/checkbox_attributes.go (1)
pkg/duit_attributes/focus_capability.go (1)
FocusCapability(5-7)
pkg/duit_attributes/text_field_attributes.go (2)
pkg/duit_attributes/focus_capability.go (1)
FocusCapability(5-7)pkg/duit_props/focus_node.go (1)
FocusNode(7-13)
pkg/duit_props/focus_node.go (1)
pkg/duit_utils/tristate.go (3)
TString(33-33)TBool(32-32)TristateFrom(54-68)
pkg/duit_attributes/radio_attributes.go (1)
pkg/duit_attributes/focus_capability.go (1)
FocusCapability(5-7)
pkg/duit_attributes/zz_embedded_structs_delegates.go (7)
pkg/duit_attributes/checkbox_attributes.go (1)
CheckboxAttributes(12-30)pkg/duit_props/focus_node.go (1)
FocusNode(7-13)pkg/duit_attributes/focus_capability.go (1)
FocusCapability(5-7)pkg/duit_attributes/elevated_buttion_attributes.go (1)
ElevatedButtonAttributes(11-19)pkg/duit_attributes/radio_attributes.go (1)
RadioAttributes(12-28)pkg/duit_attributes/switch_attribytes.go (1)
SwitchAttributes(12-30)pkg/duit_attributes/text_field_attributes.go (1)
TextFieldAttributes(10-31)
pkg/duit_attributes/focus_command.go (3)
pkg/duit_utils/tristate.go (4)
TString(33-33)Tristate(30-30)StringValue(70-72)TristateFrom(54-68)pkg/duit_props/unfocus_dispodition.go (1)
UnfocusDisposition(3-3)pkg/duit_props/traversal_direction.go (1)
TraversalDirection(3-3)
pkg/duit_attributes/elevated_buttion_attributes.go (1)
pkg/duit_attributes/focus_capability.go (1)
FocusCapability(5-7)
🔇 Additional comments (13)
pkg/duit_props/focus_node.go (1)
1-48: LGTM!The FocusNode struct and its fluent setter methods are well-implemented. The receiver name "r" follows the coding guidelines, and the use of
TristateFromfor field assignment is consistent with the existing patterns. The docstrings provide helpful context with the Flutter API reference.pkg/duit_attributes/checkbox_attributes.go (1)
15-15: LGTM on FocusCapability embedding.The
*FocusCapabilityembedding follows the established pattern used for*ValueReferenceHolderand*ThemeConsumer. Thegen:"wrap"tag is consistent with other embedded fields.pkg/duit_attributes/focus_capability.go (1)
5-7: LGTM on FocusCapability struct definition.The struct is cleanly defined with appropriate JSON tag including
omitemptyfor optional serialization.pkg/duit_attributes/switch_attribytes.go (1)
15-15: LGTM on FocusCapability embedding.The
*FocusCapabilityembedding is consistent with the pattern used in other attribute structs likeCheckboxAttributes.pkg/duit_attributes/elevated_buttion_attributes.go (1)
14-14: LGTM! Focus capability embedding follows the established pattern.The embedded
*FocusCapabilityintegrates cleanly with the existing struct, and the delegation method inzz_embedded_structs_delegates.goprovides the fluent interface for setting focus nodes.pkg/duit_attributes/radio_attributes.go (1)
16-16: LGTM! Consistent focus capability integration.The embedded
*FocusCapabilityaligns with the pattern used across other attribute structs in this PR.pkg/duit_attributes/text_field_attributes.go (1)
13-13: Embedding looks correct.pkg/duit_props/traversal_direction.go (1)
1-10: LGTM! Clean and straightforward type definition.The
TraversalDirectiontype and its four directional constants are well-defined and integrate cleanly with the focus command constructors.pkg/duit_attributes/inkwell_attributes.go (1)
14-14: LGTM! Focus capability embedding is correct.The embedded
*FocusCapabilityfollows the established pattern. The fields marked as changed (lines 15-37) appear to be a diff artifact from adding the embedding; their semantics remain unchanged.pkg/duit_attributes/focus_command.go (3)
8-16: LGTM! Clear action enumeration.The
FocusActiontype and its five action constants provide a clean API for focus operations.
18-30: LGTM! Well-structured command type.The
FocusCommandstruct appropriately usesTristatefor optional fields and the internalnewFocusCommandhelper ensures consistent initialization.
32-63: LGTM! Consistent constructor pattern.All five constructor functions follow a uniform pattern: initialize via
newFocusCommand, set the action, populate relevant fields, and return. The use ofTristateFromsafely handles nil pointer parameters.pkg/duit_attributes/zz_embedded_structs_delegates.go (1)
4444-4490: LGTM! Consistent delegation pattern across all attribute types.All six
SetFocusNodemethods follow the required pattern:
- Check and initialize
FocusCapabilityif nil (complies with coding guideline on nil-receiver checks)- Delegate to the embedded type's method
- Return receiver for fluent chaining
- Use receiver name "r" (complies with coding guideline)
The generated code maintains consistency across all attribute wrappers.
There was a problem hiding this comment.
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 (1)
pkg/duit_attributes/text_field_attributes.go (1)
134-144: Add nil checks for embedded pointers before calling Validate methods.The Validate method calls
Validate()onValueReferenceHolderandThemeConsumerwithout nil checks. Per coding guidelines, methods must be preceded by explicit nil checks to prevent runtime panics on potentially nil receivers. Update the method to check for nil before calling methods on these embedded pointers:func (r *TextFieldAttributes) Validate() error { if r.ValueReferenceHolder != nil { if err := r.ValueReferenceHolder.Validate(); err != nil { return err } } if r.ThemeConsumer != nil { if err := r.ThemeConsumer.Validate(); err != nil { return err } } return nil }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
pkg/duit_attributes/text_field_attributes.go(1 hunks)pkg/duit_props/unfocus_disposition.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
pkg/**/*.go
📄 CodeRabbit inference engine (pkg/.cursor/rules/style.mdc)
pkg/**/*.go: When declaring new structure methods, the receiver name must always be "r"
If a caller calls a method on an object that is potentially a nil-receiver, it must explicitly check the object for nil before the call
Files:
pkg/duit_attributes/text_field_attributes.gopkg/duit_props/unfocus_disposition.go
🧬 Code graph analysis (1)
pkg/duit_attributes/text_field_attributes.go (1)
pkg/duit_attributes/focus_capability.go (1)
FocusCapability(5-7)
🔇 Additional comments (1)
pkg/duit_attributes/text_field_attributes.go (1)
13-13: LGTM: FocusCapability embedding aligns with established pattern.The embedded
*FocusCapabilityfollows the same structure as other capability embeddings in this struct and across other attribute types. This also resolves the redundant FocusNode field issue flagged in the previous review.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.