Key Factors Redesign 2nd iteration#4470
Key Factors Redesign 2nd iteration#4470ncarazon wants to merge 4 commits intofeat/key-factors-new-iterationfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
| useEffect(() => { | ||
| if (keyFactor && !comment) { | ||
| ensureCommentLoaded(keyFactor.comment_id); | ||
| } | ||
| }, [keyFactor, comment, ensureCommentLoaded]); |
There was a problem hiding this comment.
this feels like if comment is null, this can run on every re-render as ensureCommentLoaded function is also not wrapped in useCallback. Are we sure this line only sends api request once?
890b7c6 to
c20a690
Compare
f46cb8c to
e7aa882
Compare
c20a690 to
b46d152
Compare
b076895 to
a0511e6
Compare
This PR implements the 1st iteration of the Key Factors Redesign
Iteration 2: Expanded Overlay