fix: toast Message while creating the survey#3240
Conversation
|
@smritidoneria is attempting to deploy a commit to the formbricks Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes involve updating the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
github-actions
bot
added
bug
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SuccessMessage.tsx (1)
Line range hint
26-38: Effective solution: Preventing duplicate messages inuseEffect.The changes in the
useEffecthook successfully address the issue of duplicate toast messages:
- The added check
!messageShownRef.currentensures the message is only shown once.- Setting
messageShownRef.current = trueafter showing the message prevents future displays.These modifications align perfectly with the PR objective and the issue described in #3204.
For improved clarity, consider adding a comment explaining the purpose of
messageShownRef:const messageShownRef = useRef(false); +// Ref to track if the success message has been shown, preventing duplicates
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SuccessMessage.tsx (3 hunks)
🔇 Additional comments (3)
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/(analysis)/summary/components/SuccessMessage.tsx (3)
4-4: LGTM: Import statement updated correctly.The addition of
useRefto the import statement is consistent with its usage in the component and aligns with the PR objective of fixing duplicate toast messages.
18-18: Good addition:messageShownRefprevents duplicate messages.The introduction of
messageShownRefusinguseRefis an effective solution to track whether the success message has been shown. This directly addresses the issue of duplicate toast messages mentioned in the PR objectives (#3204). The initial value offalseis appropriate, ensuring the message is shown at least once.
Line range hint
1-53: Summary: Effective solution for preventing duplicate toast messages.The changes implemented in this file successfully address the issue of duplicate toast messages when creating and publishing a survey, as described in the PR objectives and issue #3204. The introduction of
messageShownRefand its usage in theuseEffecthook provide a clean and efficient solution to the problem.To finalize this PR:
- Add the suggested comment for
messageShownRefto improve code clarity.- Ensure thorough testing of the survey creation and publishing process to verify that the toast message appears only once.
- Consider adding a unit test for the
SuccessMessagecomponent to prevent regression of this issue in the future.To verify the changes, please run the following test script:
This script will help ensure that the changes are implemented correctly and consistently throughout the component.
|
hey @smritidoneria your solution doesn't solve the problem but the symptom. Please find out why the Toast is issued twice (probably related to the component being rendered twice). Consult with GPT to find out why and learn more on how React works 🤓 |
|
Hey @jobenjada ,this issue is likely to occur due to the due to use effect hook being triggered various times due to its dependency array change, causing it to re-run. Specifically searchPrams is causing this effect to re-run. So the solution of this problem is to use useRef hook. The useRef hook allows you to persist a mutable value across renders without causing a re-render when the value changes. This will be useful for keeping track of whether the toast message has already been shown.( this pr uses this solution only) |
|
@smritidoneria, you can try adding a unique ID to this toast. This should fix the issue without using ref https://arc.net/l/quote/pyoztdxy |
|
Hey @gupta-piyush19 , Updated the code by adding a toastId. kindly have a look |
gupta-piyush19
left a comment
gupta-piyush19
left a comment
There was a problem hiding this comment.
Thanks @smritidoneria, fix works great 🙌
LGTM 🚀
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
hey @gupta-piyush19 , can you please tell why this pr has been removed from merged Queue, are there any changes required? |
4 participants
What does this PR do?
This pr fixes the duplication of the toast message after creating the survey.
Fixes #3204
Screen.Recording.2024-10-02.at.10.57.12.AM.mov
How should this be tested?
2.Publish it.
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes