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: confirm modal on back button #2189

Merged
merged 1 commit into from
Mar 6, 2024
Merged

fix: confirm modal on back button #2189

merged 1 commit into from
Mar 6, 2024

Conversation

Dhruwang
Copy link
Contributor

@Dhruwang Dhruwang commented Mar 5, 2024

What does this PR do?

Fixes 1972

When back button is clicked we check the equality of localsurvey and survey

Screenshot 2024-03-05 at 5 19 00 PM

But there occurs a mismatch due to field updatedAt
Screenshot 2024-03-05 at 5 23 12 PM

When changes are made to a survey and then saved, survey is updated, which further runs this useEffect
Screenshot 2024-03-05 at 5 21 16 PM
But due to the check if (localSurvey) return; , localsurvey is not updated with the latest survey
And due to this there occurs a time difference between the time when survey was saved (localsurvey.updatedAt) and the actual time at which it was saved in database (survey.updatedAt)

How should this be tested?

  1. Save a survey
  2. Click on back button on survey menubar

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read How we Code at Formbricks
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran pnpm build
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Formbricks Docs if changes were necessary

Copy link

vercel bot commented Mar 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Updated (UTC)
formbricks-cloud ⬜️ Ignored (Inspect) Mar 5, 2024 11:57am
formbricks-com ⬜️ Ignored (Inspect) Mar 5, 2024 11:57am

Copy link
Contributor

github-actions bot commented Mar 5, 2024

Thank you for following the naming conventions for pull request titles! 🙏

Copy link
Contributor

apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/components/SurveyEditor.tsx

Currently, the useEffect hook that fetches the latest product is triggered every time the localProduct.id changes. This could lead to unnecessary API calls if the id changes frequently. Instead, consider using a ref to store the id and only fetch the latest product when the document visibility changes.
Create Issue
See the diff
Checkout the fix

    const productIdRef = useRef(localProduct.id);

    useEffect(() => {
      const listener = () => {
        if (document.visibilityState === "visible") {
          const fetchLatestProduct = async () => {
            const latestProduct = await refetchProduct(productIdRef.current);
            if (latestProduct) {
              setLocalProduct(latestProduct);
            }
          };
          fetchLatestProduct();
        }
      };
      document.addEventListener("visibilitychange", listener);
      return () => {
        document.removeEventListener("visibilitychange", listener);
      };
    }, []);
git fetch origin && git checkout -b ReviewBot/Impro-e4e9k4u origin/ReviewBot/Impro-e4e9k4u

Currently, if an error occurs while creating a new segment, a generic error message is thrown. It would be more helpful to include the actual error message in the thrown error to aid in debugging.
Create Issue
See the diff
Checkout the fix

    if (!localSurvey.segment?.id) {
      try {
        createSegment();
      } catch (err) {
        throw new Error(`Error creating segment: ${err.message}`);
      }
    }
git fetch origin && git checkout -b ReviewBot/Impro-zv1eiqa origin/ReviewBot/Impro-zv1eiqa

@jobenjada jobenjada added this pull request to the merge queue Mar 6, 2024
Merged via the queue into main with commit f20a0d2 Mar 6, 2024
16 checks passed
@jobenjada jobenjada deleted the confirm-modal-issue branch March 6, 2024 07:48
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.

None yet

2 participants