-
Notifications
You must be signed in to change notification settings - Fork 6
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Do we need to to anything to make sure to not undo this fix? #2267
@eniskraasniqi1 maybe you remember when the menu opened as an empty square?
Not really 😅, but I'm going to test those changes locally (bring back some lines), and see if I added this line for a good reason. EDIT: Can't link UI locally, will try with someone tmr. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't seem like a good solution to me. The dependency that you now use is a ref, so it will never update, so the effect will never run again.
@lowiebenoot The item should not be rerendered |
75707b7
9765840
to
75707b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldnt rerender so 🤞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry didn't see this, could've probably been added in the same release 🙈
Group MCed with @eniskraasniqi1 and @stefaandevylder ✅ |
Description
Fixes the possibility to have an infinite loop. This was introduced when converting from JS to Typescript. Bug was found with Sentry.
This is a shot in the dark, but most likely the fix.
Sentry: https://sentry.io/organizations/teamleader/issues/3788415931/?project=1211390&referrer=slack