feat: Recall functionality #1789
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/components/OpenQuestionForm.tsxConsider using the useCallback hook for the event handlers to avoid unnecessary re-rendering of the components. This will improve the performance of your application. const handleInputChange = useCallback((inputType: TSurveyOpenTextQuestionInputType) => {
const updatedAttributes = {
inputType: inputType,
placeholder: getPlaceholderByInputType(inputType),
longAnswer: inputType === "text" ? question.longAnswer : false,
};
updateQuestion(questionIdx, updatedAttributes);
}, [question.longAnswer, questionIdx, updateQuestion]);apps/web/app/(app)/environments/[environmentId]/surveys/[surveyId]/edit/components/RecallQuestionSelect.tsxConsider destructuring the properties of the objects to improve the readability of your code. const { environmentId, questions } = localSurvey;packages/surveys/src/components/questions/CTAQuestion.tsxConsider validating the URL before opening it in a new tab to prevent any potential security risks. if (question.buttonExternal && question.buttonUrl && isValidURL(question.buttonUrl)) {
window?.open(question.buttonUrl, "_blank")?.focus();
}packages/surveys/src/components/general/Survey.tsxConsider breaking down the getNextQuestionId function into smaller, more manageable functions. This will make the code easier to read and maintain. const getNextQuestionId = (data: TResponseData, isFromPrefilling: Boolean = false): string => {
const questions = survey.questions;
const responseValue = data[questionId];
if (questionId === "start") {
if (!isFromPrefilling) {
return questions[0]?.id || "end";
} else {
currIdx = 0;
currQues = questions[0];
}
}
if (currIdx === -1) throw new Error("Question not found");
if (currQues?.logic && currQues?.logic.length > 0) {
for (let logic of currQues.logic) {
if (!logic.destination) continue;
if (evaluateCondition(logic, responseValue)) {
return logic.destination;
}
}
}
return questions[currIdx + 1]?.id || "end";
}Consider using the useCallback hook to memoize the onSubmit function. This will prevent unnecessary re-rendering of the component when the function is passed as a prop to child components. const onSubmit = useCallback((responseData: TResponseData, ttc: TResponseTtc, isFromPrefilling: Boolean = false) => {
const questionId = Object.keys(responseData)[0];
setLoadingElement(true);
const nextQuestionId = getNextQuestionId(responseData, isFromPrefilling);
const finished = nextQuestionId === "end";
onResponse({ data: responseData, ttc, finished });
if (finished) {
onFinished();
}
setQuestionId(nextQuestionId);
// add to history
setHistory([...history, questionId]);
setLoadingElement(false);
onActiveQuestionChange(nextQuestionId);
}, [getNextQuestionId, onResponse, onFinished, setQuestionId, setHistory, setLoadingElement, onActiveQuestionChange]);Consider renaming the variable 'ttc' to a more descriptive name. This will make the code easier to understand for other developers. const [responseTime, setResponseTime] = useState<TResponseTtc>({});packages/lib/utils/recall.tsThe same regular expression pattern is used in multiple places. It would be more readable and maintainable to define it once and reuse it. const RECALL_PATTERN = /recall:([A-Za-z0-9]+)/;
const FALLBACK_PATTERN = /fallback:(\S*)/;
const RECALL_INFO_PATTERN = /recall:([A-Za-z0-9]+)\/fallback:(\S*)/;
... |
![review-agent-prime[bot]](https://avatars.githubusercontent.com/in/404610?s=60&v=4){kind=small}
| }; | ||
|
|
||
| export default function RecallQuestionSelect({ | ||
| currentQuestionIdx, |
There was a problem hiding this comment.
Destructuring the properties of the 'localSurvey' object to improve the readability of the code.
| currentQuestionIdx, | |
| const { environmentId, questions } = localSurvey; |
packages/lib/utils/recall.ts
Outdated
| import { TSurvey } from "@formbricks/types/surveys"; | ||
|
|
||
| export function extractId(text: string) { | ||
| const pattern = /recall:([A-Za-z0-9]+)/; | ||
| const match = text.match(pattern); | ||
| if (match && match[1]) { | ||
| return match[1]; | ||
| } else { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| export function extractIds(text: string) { | ||
| const pattern = /recall:([A-Za-z0-9]+)/g; | ||
| const matches = text.match(pattern); | ||
| if (!matches) { | ||
| return []; | ||
| } | ||
| return matches | ||
| .map((match) => { | ||
| const matchPattern = /recall:([A-Za-z0-9]+)/; | ||
| const idMatch = match.match(matchPattern); | ||
| return idMatch ? idMatch[1] : null; | ||
| }) | ||
| .filter((id) => id !== null); | ||
| } | ||
|
|
||
| export function extractFallbackValue(text: string): string { | ||
| const pattern = /fallback:(\S*)/; | ||
| const match = text.match(pattern); | ||
| if (match && match[1]) { | ||
| return match[1]; | ||
| } else { | ||
| return ""; | ||
| } | ||
| } | ||
|
|
||
| export function extractRecallInfo(headline: string): string | null { | ||
| const pattern = /recall:([A-Za-z0-9]+)\/fallback:(\S*)/; | ||
| const match = headline.match(pattern); | ||
| return match ? match[0] : null; | ||
| } | ||
|
|
||
| export const checkForRecall = (headline: string, survey: TSurvey) => { | ||
| let newHeadline = headline; | ||
| if (!headline.includes("recall:")) return headline; | ||
| while (newHeadline.includes("recall:")) { | ||
| const recallInfo = extractRecallInfo(newHeadline); | ||
| if (recallInfo) { | ||
| const questionId = extractId(recallInfo); | ||
| newHeadline = newHeadline.replace( | ||
| recallInfo, | ||
| `@${survey.questions.find((question) => question.id === questionId)?.headline}` | ||
| ); | ||
| } | ||
| } | ||
| return newHeadline; | ||
| }; | ||
|
|
||
| export function findRecallInfoById(text: string, id: string): string | null { | ||
| const pattern = new RegExp(`recall:${id}\\/fallback:(\\S*)`, "g"); | ||
| const match = text.match(pattern); | ||
| return match ? match[0] : null; | ||
| } |
There was a problem hiding this comment.
Defining the regular expression patterns once and reusing them in the code to improve readability and maintainability.
| import { TSurvey } from "@formbricks/types/surveys"; | |
| export function extractId(text: string) { | |
| const pattern = /recall:([A-Za-z0-9]+)/; | |
| const match = text.match(pattern); | |
| if (match && match[1]) { | |
| return match[1]; | |
| } else { | |
| return null; | |
| } | |
| } | |
| export function extractIds(text: string) { | |
| const pattern = /recall:([A-Za-z0-9]+)/g; | |
| const matches = text.match(pattern); | |
| if (!matches) { | |
| return []; | |
| } | |
| return matches | |
| .map((match) => { | |
| const matchPattern = /recall:([A-Za-z0-9]+)/; | |
| const idMatch = match.match(matchPattern); | |
| return idMatch ? idMatch[1] : null; | |
| }) | |
| .filter((id) => id !== null); | |
| } | |
| export function extractFallbackValue(text: string): string { | |
| const pattern = /fallback:(\S*)/; | |
| const match = text.match(pattern); | |
| if (match && match[1]) { | |
| return match[1]; | |
| } else { | |
| return ""; | |
| } | |
| } | |
| export function extractRecallInfo(headline: string): string | null { | |
| const pattern = /recall:([A-Za-z0-9]+)\/fallback:(\S*)/; | |
| const match = headline.match(pattern); | |
| return match ? match[0] : null; | |
| } | |
| export const checkForRecall = (headline: string, survey: TSurvey) => { | |
| let newHeadline = headline; | |
| if (!headline.includes("recall:")) return headline; | |
| while (newHeadline.includes("recall:")) { | |
| const recallInfo = extractRecallInfo(newHeadline); | |
| if (recallInfo) { | |
| const questionId = extractId(recallInfo); | |
| newHeadline = newHeadline.replace( | |
| recallInfo, | |
| `@${survey.questions.find((question) => question.id === questionId)?.headline}` | |
| ); | |
| } | |
| } | |
| return newHeadline; | |
| }; | |
| export function findRecallInfoById(text: string, id: string): string | null { | |
| const pattern = new RegExp(`recall:${id}\\/fallback:(\\S*)`, "g"); | |
| const match = text.match(pattern); | |
| return match ? match[0] : null; | |
| } | |
| const RECALL_PATTERN = /recall:([A-Za-z0-9]+)/; | |
| const RECALL_PATTERN_GLOBAL = /recall:([A-Za-z0-9]+)/g; | |
| const FALLBACK_PATTERN = /fallback:(\S*)/; | |
| const RECALL_INFO_PATTERN = /recall:([A-Za-z0-9]+)\/fallback:(\S*)/; | |
| export function extractId(text: string) { | |
| const match = text.match(RECALL_PATTERN); | |
| ... | |
| } | |
| export function extractIds(text: string) { | |
| const matches = text.match(RECALL_PATTERN_GLOBAL); | |
| ... | |
| const idMatch = match.match(RECALL_PATTERN); | |
| ... | |
| } | |
| export function extractFallbackValue(text: string): string { | |
| const match = text.match(FALLBACK_PATTERN); | |
| ... | |
| } | |
| export function extractRecallInfo(headline: string): string | null { | |
| const match = headline.match(RECALL_INFO_PATTERN); | |
| ... | |
| } | |
| export function findRecallInfoById(text: string, id: string): string | null { | |
| const pattern = new RegExp(`recall:${id}\/fallback:(\S*)`, "g"); | |
| ... | |
| } |
|
Hey Dru! Super cool what you've built with so few lines of code 😍 💪 Here is my feedback:
Also, add a bit of margin-x here: 
Can we move it into this bubble and display it on bubble hover? And then open the fallback editor on bubble click? That would be much more intuitive and solve the hover issues :) When there are two recalled questions, only display the fallback settings for the one where the user clicked on the bubble:
So if you hit backspace, just remove the bubble.
Recalling questions which are recalling questions leads to tiny UI mess: 
Any ideas on how to solve that? 🤔 I think it doesnt make sense to have it in the bubble itself
and the icons are getting smaller and smaller: 
This is a recalled date: 
Can we reformat that to "12th of December, 2023" (or similar) ? Just to increase readability. Seems like a lot but mostly tweaking! Great work Dru, really impressed with this 😍 💪 |
|
@Dhruwang I'm currently making the final testing. The whole feature looks and works pretty great! 😊💪 The recall information in the thank you card isn't working: Can you please check and fix that? If this is fixed we are ready to merge this 😊💪 |
|
Added little hint as placeholder: 
|
|
@mattinannt , have fixed the issue with thank you card 😊 |
There was a problem hiding this comment.
@Dhruwang thanks for the changes; works great! 😊
So cool that we finally finished this feature after so many months and different attempts. great job! 👏👏
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
@Dhruwang there seems to be one issue left with the e2e tests. Can you please check what's going on there? 😊 |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
What does this PR do?
Fixes #1673
recall.mp4
Type of change
How should this be tested?
Type @ into question headline box and select the recall question
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated