Skip to content

refactor(tree): move effect code back to updateContextMenuItem#1804

Merged
chintankavathia merged 1 commit intomainfrom
refactor/tree-update-context-menu
Apr 2, 2026
Merged

refactor(tree): move effect code back to updateContextMenuItem#1804
chintankavathia merged 1 commit intomainfrom
refactor/tree-update-context-menu

Conversation

@spliffone
Copy link
Copy Markdown
Member

@spliffone spliffone commented Apr 2, 2026

@spliffone spliffone requested review from a team as code owners April 2, 2026 08:23
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the initialization of context menu items by moving an effect block from the constructor into a new private method. Feedback indicates that using effect() for state propagation violates the repository style guide (Rule 38), which recommends using computed() or a reactive approach instead. Additionally, the reviewer noted a risk of creating redundant effects if the method is called multiple times and suggested correcting a naming inconsistency between the method and the signal it updates.

Copy link
Copy Markdown
Member

@chintankavathia chintankavathia left a comment

Choose a reason for hiding this comment

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

👍

@chintankavathia chintankavathia added this pull request to the merge queue Apr 2, 2026
Merged via the queue into main with commit ac662f1 Apr 2, 2026
11 checks passed
@chintankavathia chintankavathia deleted the refactor/tree-update-context-menu branch April 2, 2026 09:18
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