Skip to content

chore: Fixed reset model#271

Open
kinhdev24 wants to merge 1 commit into
mainfrom
hotfix/3d-building-reset-model
Open

chore: Fixed reset model#271
kinhdev24 wants to merge 1 commit into
mainfrom
hotfix/3d-building-reset-model

Conversation

@kinhdev24
Copy link
Copy Markdown
Member

No description provided.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spacedf-web-app Ready Ready Preview, Comment Apr 29, 2026 10:03am

Request Review

Copy link
Copy Markdown

@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 WidgetList component to use useMemo for the dashboard list and introduces a 400ms delay for the run function in the FitOnRefocus component. A review comment highlights the need for a cleanup function in the useEffect hook to clear the setTimeout and avoid potential memory leaks, while also questioning the use of a hardcoded delay.

}
run()

setTimeout(run, 400)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The setTimeout call is missing a cleanup function. When using timers inside useEffect, it is important to clear them to prevent the callback from executing after the component has unmounted or when dependencies change. This avoids potential memory leaks and ensures that the run function doesn't execute with stale closure values.

Additionally, the use of a hardcoded delay (400ms) is a "magic number" that might lead to inconsistent behavior. Consider if there is a more deterministic way to signal that the model is ready for the bounds calculation.

    const timer = setTimeout(run, 400)
    return () => clearTimeout(timer)

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.

1 participant