Skip to content

feat: Recall Information FOR-849 #1673

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.

Sign up for GitHub

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

Jump to bottom
Closed
neilchauhan2 wants to merge 2 commits into from
Closed

feat: Recall Information FOR-849 #1673

neilchauhan2 wants to merge 2 commits into from

Conversation

@neilchauhan2
Copy link
Contributor

@neilchauhan2 neilchauhan2 commented Nov 22, 2023

…ed on dummy options

What does this PR do?

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change adds a new database migration
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

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
  • First PR at Formbricks? Please sign the CLA! Without it we wont be able to merge it 🙏

Appreciated

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

@neilchauhan2 neilchauhan2 self-assigned this Nov 22, 2023
@vercel
Copy link

vercel bot commented Nov 22, 2023 •
edited
Loading

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

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
formbricks-cloud ⬜️ Ignored (Inspect) Visit Preview Nov 23, 2023 1:20pm
formbricks-com ⬜️ Ignored (Inspect) Visit Preview Nov 23, 2023 1:20pm

@github-actions
Copy link
Contributor

github-actions bot commented Nov 22, 2023 •
edited
Loading

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

@review-agent-prime
Copy link
Contributor

review-agent-prime bot commented Nov 22, 2023

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

While the implementation of the MentionsInput component is a great addition, the hardcoded data array for the mentions could be improved. If the data for the mentions is static and won't change, it's fine to leave it as is. However, if the data is dynamic and could change, it would be better to fetch this data from a source (like an API or a state management store) and update it accordingly.
Create Issue
See the diff
Checkout the fix

    // If the data is dynamic, fetch it from the appropriate source
    const data = fetchDataFromSource();
git fetch origin && git checkout -b ReviewBot/The-n-0vzvh6i origin/ReviewBot/The-n-0vzvh6i

The new code introduces inline styles for the MentionsInput component. While this works, it can lead to performance issues in React as the object will be recreated on every render. It's recommended to use CSS classes or styled-components for better performance.
Create Issue
See the diff
Checkout the fix

    // Use CSS classes or styled-components for better performance
    const StyledMentionsInput = styled(MentionsInput)`
      width: 100%;
      border: 1px rgb(203 213 225) solid;
      border-radius: 4px;
      textArea: {
        border: none;
        marginBottom: 1rem;
      };
      suggestions: {
        list: {
          backgroundColor: white;
          fontSize: 14;
        };
        item: {
          padding: 5px 15px;
          "&focused": {
            backgroundColor: #cee4e5;
          };
        };
      };
    `;
git fetch origin && git checkout -b ReviewBot/The-n-55wjm35 origin/ReviewBot/The-n-55wjm35

review-agent-prime[bot]
review-agent-prime bot reviewed Nov 22, 2023
View reviewed changes
.../(app)/environments/[environmentId]/surveys/[surveyId]/edit/components/QuestionFormInput.tsx Outdated
Comment on lines 30 to 36
const data = [
{ id: "question 1", display: "question 1" },
{ id: "question 2", display: "question 2" },
{ id: "question 3", display: "question 3" },
{ id: "question 4", display: "question 4" },
{ id: "question 5", display: "question 5" },
];
Copy link
Contributor

@review-agent-prime review-agent-prime bot Nov 22, 2023

Choose a reason for hiding this comment

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

The hardcoded data array for the mentions is replaced with a function call to fetch data from a source. This is beneficial if the data is dynamic and could change over time.

Suggested change
const data = [
{ id: "question 1", display: "question 1" },
{ id: "question 2", display: "question 2" },
{ id: "question 3", display: "question 3" },
{ id: "question 4", display: "question 4" },
{ id: "question 5", display: "question 5" },
];
const data = fetchDataFromSource();

@jobenjada jobenjada closed this Dec 1, 2023
@Dhruwang Dhruwang mentioned this pull request Dec 18, 2023
Merged
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@review-agent-prime review-agent-prime[bot] review-agent-prime[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@neilchauhan2 neilchauhan2

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@neilchauhan2 @jobenjada