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

are you sure modal missing fix:#1419 #1485

Merged
merged 2 commits into from
Nov 6, 2023
Merged

are you sure modal missing fix:#1419 #1485

merged 2 commits into from
Nov 6, 2023

Conversation

RajuGangitla
Copy link
Contributor

What does this PR do?

Now, if we add or delete question or we made any changes in survey it will show the "are you sure" popup

Fixes # (issue)
#1419

Your.Surveys._.Formbricks.-.Google.Chrome.2023-10-28.10-02-07.mp4

Type of change

  • [x ] Bug fix (non-breaking change which fixes an issue)

@vercel
Copy link

vercel bot commented Oct 28, 2023

@RajuGangitla is attempting to deploy a commit to the formbricks Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2023

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "are you sure modal missing fix:#1419". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@review-agent-prime
Copy link
Contributor

The change you made in the useEffect hook, where you deep clone the survey object using JSON.parse(JSON.stringify(survey)), can be a potential performance issue if the survey object is large. This is because JSON.stringify will convert the entire object into a string, and JSON.parse will parse the string back into an object. This operation can be costly for large objects.

If you need to create a deep copy of the survey object to avoid mutating the original object, consider using a library like lodash's _.cloneDeep method which is more efficient for this purpose.

Here's how you can do it:

import _ from 'lodash';

// ...

useEffect(() => {
  if (survey) {
    setLocalSurvey(_.cloneDeep(survey));
    if (survey.questions.length > 0) {
      setActiveQuestionId(survey.questions[0].id);
    }
  }
}, [survey]);

This will create a deep copy of the survey object without the potential performance issue of JSON.parse(JSON.stringify(survey)).

@jobenjada
Copy link
Member

@RajuGangitla Please explain how your solution is supposed to work 🤔

@jobenjada jobenjada marked this pull request as draft October 28, 2023 15:51
@RajuGangitla
Copy link
Contributor Author

@jobenjada previously when we were trying to edit the survey there if we add or delete some questions and click on back button it's not showing are you sure modal popup is not showing.
Now we can overcome this by creating a deep copy of surveyb object from db and assign it to local survey state this can be done by two ways mentioned below

  1. using lodash
  2. using json stringify
    @mattinannt suggested that its better if we do without any package and he suggested the 2nd approach and made a pull request with that approach

@mattinannt mattinannt self-assigned this Nov 6, 2023
@mattinannt mattinannt marked this pull request as ready for review November 6, 2023 09:41
Copy link
Member

@mattinannt mattinannt left a comment

Choose a reason for hiding this comment

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

💪🙏

@mattinannt mattinannt added this pull request to the merge queue Nov 6, 2023
Merged via the queue into formbricks:main with commit 5ea1588 Nov 6, 2023
6 of 9 checks passed
kevinkong91 added a commit to kevinkong91/formbricks that referenced this pull request Nov 7, 2023
* main: (42 commits)
  chore: add revert to oss-friends (formbricks#745)
  fix: pass authOptions to getServerSession in authLayout (formbricks#1584)
  fix: fixed URL example for "Create Survey" API endpoint is wrong formbricks#1555 (formbricks#1586)
  fix(docs): default account info docs changes (formbricks#1583)
  chore: Add docker packages to Github Packages on release (formbricks#1585)
  feat: avatar upload (formbricks#1546)
  fix: editor crashing (formbricks#1582)
  fix: Add scroll to the setting navbar  (formbricks#1398)
  are you sure modal missing fix:formbricks#1419 (formbricks#1485)
  fix: github linting issues (formbricks#1510)
  refactor: added authorization to airtable integration and create a common actions file (formbricks#1538)
  docs: add docs for airtable (formbricks#1541)
  fix: missing static generation store in revalidation due to pages dir (formbricks#1581)
  feat: Pagination for other values in multi choice (formbricks#1560)
  feat:  $199 pricing model for unlimited plans (formbricks#1564)
  fix: make pricing pages consistent (formbricks#1567)
  fix: openText issue (formbricks#1579)
  fix: avoid blocking the request if it does not have an associated ip (formbricks#1540)
  feat: Add Unkey to OSS Friends (formbricks#1574)
  feat: FOR-683 Role Switch (formbricks#1450)
  ...
kevinkong91 added a commit to kevinkong91/formbricks that referenced this pull request Nov 7, 2023
* main: (70 commits)
  chore: add revert to oss-friends (formbricks#745)
  fix: pass authOptions to getServerSession in authLayout (formbricks#1584)
  fix: fixed URL example for "Create Survey" API endpoint is wrong formbricks#1555 (formbricks#1586)
  fix(docs): default account info docs changes (formbricks#1583)
  chore: Add docker packages to Github Packages on release (formbricks#1585)
  feat: avatar upload (formbricks#1546)
  fix: editor crashing (formbricks#1582)
  fix: Add scroll to the setting navbar  (formbricks#1398)
  are you sure modal missing fix:formbricks#1419 (formbricks#1485)
  fix: github linting issues (formbricks#1510)
  refactor: added authorization to airtable integration and create a common actions file (formbricks#1538)
  docs: add docs for airtable (formbricks#1541)
  fix: missing static generation store in revalidation due to pages dir (formbricks#1581)
  feat: Pagination for other values in multi choice (formbricks#1560)
  feat:  $199 pricing model for unlimited plans (formbricks#1564)
  fix: make pricing pages consistent (formbricks#1567)
  fix: openText issue (formbricks#1579)
  fix: avoid blocking the request if it does not have an associated ip (formbricks#1540)
  feat: Add Unkey to OSS Friends (formbricks#1574)
  feat: FOR-683 Role Switch (formbricks#1450)
  ...
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

3 participants