Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(app): Removing infinite loop on modified dropdown render #1697

Merged
merged 2 commits into from
May 30, 2024

Conversation

jtulk
Copy link
Contributor

@jtulk jtulk commented May 30, 2024

https://wandb.atlassian.net/browse/WB-19077

Effect updating state causing infinite render. Unsure as to the deeper root cause but this fixes the error.

@jtulk jtulk requested a review from a team as a code owner May 30, 2024 17:28
@circle-job-mirror
Copy link

circle-job-mirror bot commented May 30, 2024

}
const isOverflow =
optionRef.current.scrollWidth > optionRef.current.clientWidth;
if (optionRef.current && isOverflow !== showTooltip) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is optionRef.current && required here now that you are checking above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope

Comment on lines +490 to +491
if (isOverflow !== showTooltip) {
setShowTooltip(isOverflow);
Copy link
Contributor

Choose a reason for hiding this comment

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

could this just be setShowTooltip(isOverflow);? React shouldn't cause an update if it ends up the same boolean value 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You would think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a great answer here. Why this is infinite looping is not exactly known to me. I just know this fixes it as experienced.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh wait duh 😂 , that was the previous code. I wonder if this is because of the ref

@jtulk jtulk enabled auto-merge (squash) May 30, 2024 18:30
@jtulk jtulk merged commit c843c75 into master May 30, 2024
24 checks passed
@jtulk jtulk deleted the jtulk/fix-report-render-error branch May 30, 2024 18:34
@github-actions github-actions bot locked and limited conversation to collaborators May 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants