Skip to content

refactor: refactor AccountOverview to avoid prop drilling [PERA-3692]#237

Open
fmsouza wants to merge 1 commit intomainfrom
fmsouza/pera-3692
Open

refactor: refactor AccountOverview to avoid prop drilling [PERA-3692]#237
fmsouza wants to merge 1 commit intomainfrom
fmsouza/pera-3692

Conversation

@fmsouza
Copy link
Contributor

@fmsouza fmsouza commented Mar 24, 2026

Pull Request Template

Description

  • Create a local context for AccounOverview to avoid passing everything as prop
  • Split AccountOverview level state into different levels of responsibility and move some of the logic to the nested components.

Related Issues

Checklist

  • Have you tested your changes locally?
  • Have you reviewed the code for any potential issues?
  • Have you documented any necessary changes in the project's documentation?
  • Have you added any necessary tests for your changes?
  • Have you updated any relevant dependencies?

Additional Notes

  • Add any additional notes or comments that may be helpful for reviewers.

Comment on lines +27 to +33
export type UseAccountOverviewModalResult = {
account: WalletAccount
openSendFunds: () => void
openReceiveFunds: () => void
openAccountOptions: () => void
onScrollEnabledChange: (enabled: boolean) => void
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these are the same

Suggested change
export type UseAccountOverviewModalResult = {
account: WalletAccount
openSendFunds: () => void
openReceiveFunds: () => void
openAccountOptions: () => void
onScrollEnabledChange: (enabled: boolean) => void
}
export type UseAccountOverviewModalResult = AccountOverviewModalContextValue

Comment on lines +74 to +79
const onScrollEnabledChange = useCallback(
(enabled: boolean) => {
setScrollingEnabled(enabled)
},
[setScrollingEnabled],
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason to wrap it with useCallback? State setter should already be stable, right? 🤔

const handleChartSelectionChange = useCallback(
(selected: AccountBalanceHistoryItem | null) => {
setSelectedPoint(selected)
onScrollEnabledChange(selected === null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this work? Isn't it very difficult to understand inverted conditional statements? 😅

Suggested change
onScrollEnabledChange(selected === null)
onScrollEnabledChange(!selected)

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