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

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

1. In the file ResponseTimeline.tsx, you have changed the import statement for EmptyInAppSurveys from a default import to a named import. Please ensure that EmptyInAppSurveys is indeed exported as a named export from its module.

2. In the file MatrixInput.tsx, you have a condition to check if the length of rows or columns is greater than 2 before rendering the TrashIcon. This could potentially lead to a situation where the TrashIcon is not rendered even when there are items in the rows or columns. Consider revising this condition to ensure the TrashIcon is always rendered when there are items to delete.

3. In the file QuestionsComboBox.tsx, you have added a case for TSurveyQuestionType.Matrix in the getIconType function. This is a good addition, but please ensure that the GridIcon is imported at the top of the file.

4. In the file CTASummary.tsx, PictureChoiceSummary.tsx, HiddenFieldsSummary.tsx, ConsentSummary.tsx, CalSummary.tsx, OpenTextSummary.tsx, you have changed the export from a default export to a named export. This is a good practice as it makes the imports more explicit and easier to refactor. However, please ensure that all imports of these components throughout your codebase are updated to reflect this change.

5. In the file EmptyInAppSurveys.tsx, you have changed the component from a function declaration to a function expression. This is a stylistic choice and doesn't affect the functionality of the component. However, please ensure that the component is exported correctly at the end of the file.

6. In the file HiddenFieldsSummary.tsx, you are using the timeSince function to display the time since the response was updated. This is a good practice as it provides a more human-readable format for the time. However, please ensure that the timeSince function is imported at the top of the file.

7. In DateQuestionSummary.tsx, NPSSummary.tsx, RatingSummary.tsx, FileUploadSummary.tsx, MatrixQuestionSummary.tsx, and MultipleChoiceSummary.tsx, you have converted the default function exports to named constant exports. This is a good practice as it makes the code more readable and easier to debug. However, you could further improve the code by using React.memo for these functional components. This will help to optimize the components by avoiding unnecessary re-renders when the props do not change.

export const DateQuestionSummary = React.memo(({ questionSummary, environmentId }: DateQuestionSummaryProps) => {
  // existing code
});

2. In MatrixQuestionSummary.tsx, you are using inline styles to set the background color of a div. It's generally a good practice to avoid inline styles and use CSS classes instead. This makes the code cleaner and more maintainable.

<div
  className={`hover:outline-brand-dark m-1 flex h-full w-40 items-center justify-center rounded p-4 text-sm hover:outline ${getAlphaValue(percentage)}`}
>
  {percentage}
</div>

3. In MultipleChoiceSummary.tsx, you are using the resultsIdx variable to generate a key for the mapped elements. While this might work in this specific case, it's generally not a good practice to use array indices as keys in React, especially if the order of items can change. This can negatively impact performance and may cause issues with component state. If possible, use a unique identifier from the data itself.

<div key={result.value}>
  {/* existing code */}
</div>

4. In FileUploadSummary.tsx, you are creating a new Date object and converting it to an ISO string in the same line. This is unnecessary as the new Date() constructor already returns a Date object. You can pass the response.updatedAt directly to the timeSince function.

{timeSince(response.updatedAt)}

5. In MatrixQuestionSummary.tsx, you are using a useMemo hook for tooltipContent. While useMemo can be useful for optimizing performance, it's not necessary in this case because the computation is not particularly expensive. Removing useMemo would simplify the code without a significant impact on performance.

const tooltipContent = label ? label : `${Math.round((percentage / 100) * totalResponses)} responses`;

In the MatrixQuestionForm.tsx file, the handleOnChange function checks for duplicate labels after the label has been added or updated. This could lead to a situation where a user adds a duplicate label and only finds out after the fact. It would be better to check for duplicates before adding or updating the label. Here's how you could modify the function:

const handleOnChange = (index: number, type: "row" | "column", e) => {
  const newLabel = e.target.value;
  const labels = type === "row" ? [...question.rows] : [...question.columns];

  // Check for duplicate labels
  const isDuplicate = labels.includes(newLabel);
  if (isDuplicate) {
    toast.error(`Duplicate ${type} labels`);
    return; // Return early to prevent adding or updating the label
  }

  // Update the label at the given index, or add a new label if index is undefined
  if (index !== undefined) {
    labels[index] = newLabel;
  } else {
    labels.push(newLabel);
  }
  if (type === "row") {
    updateQuestion(questionIdx, { rows: labels });
  } else {
    updateQuestion(questionIdx, { columns: labels });
  }
};

In the SummaryList.tsx file, the rendering of different question types is done using a long series of if-else statements. This could be simplified and made more maintainable by using a mapping object:

const questionTypeComponentMap = {
  [TSurveyQuestionType.OpenText]: OpenTextSummary,
  [TSurveyQuestionType.MultipleChoiceSingle]: MultipleChoiceSummary,
  [TSurveyQuestionType.MultipleChoiceMulti]: MultipleChoiceSummary,
  [TSurveyQuestionType.NPS]: NPSSummary,
  [TSurveyQuestionType.CTA]: CTASummary,
  [TSurveyQuestionType.Rating]: RatingSummary,
  [TSurveyQuestionType.Consent]: ConsentSummary,
  [TSurveyQuestionType.PictureSelection]: PictureChoiceSummary,
  [TSurveyQuestionType.Date]: DateQuestionSummary,
  [TSurveyQuestionType.FileUpload]: FileUploadSummary,
  [TSurveyQuestionType.Cal]: CalSummary,
  [TSurveyQuestionType.Matrix]: MatrixQuestionSummary,
  "hiddenField": HiddenFieldsSummary,
};

// ...

summary.map((questionSummary) => {
  const Component = questionTypeComponentMap[questionSummary.type];
  return Component ? <Component key={questionSummary.question.id} questionSummary={questionSummary} /> : null;
})

In the QuestionConditional.tsx file, the same long series of if-else statements is used. This could be simplified in the same way:

const questionTypeComponentMap = {
  [TSurveyQuestionType.OpenText]: OpenTextQuestion,
  [TSurveyQuestionType.MultipleChoiceSingle]: MultipleChoiceSingleQuestion,
  [TSurveyQuestionType.MultipleChoiceMulti]: MultipleChoiceMultiQuestion,
  [TSurveyQuestionType.NPS]: NPSQuestion,
  [TSurveyQuestionType.CTA]: CTAQuestion,
  [TSurveyQuestionType.Rating]: RatingQuestion,
  [TSurveyQuestionType.Consent]: ConsentQuestion,
  [TSurveyQuestionType.Date]: DateQuestion,
  [TSurveyQuestionType.PictureSelection]: PictureSelectionQuestion,
  [TSurveyQuestionType.FileUpload]: FileUploadQuestion,
  [TSurveyQuestionType.Cal]: CalQuestion,
  [TSurveyQuestionType.Matrix]: MatrixQuestion,
};

// ...

const Component = questionTypeComponentMap[question.type];
return Component ? <Component {...props} /> : null;

Where props is an object containing all the props that are common to all question types.

1. In the QuestionCard.tsx file, the formatTextWithSlashes function splits the text and checks if the part was inside slashes. This function can be optimized by using a single reduce function instead of split and map. Here is the optimized version:

const formatTextWithSlashes = (text) => {
  const regex = /\/(.*?)\\/g;
  return text.split(regex).reduce((acc, part, index) => {
    if (index % 2 !== 0) {
      acc.push(
        <span key={index} className="mx-1 rounded-md bg-slate-100 p-1 px-2 text-xs">
          {part}
        </span>
      );
    } else {
      acc.push(part);
    }
    return acc;
  }, []);
};

2. In the QuestionCard.tsx file, the updateEmptyNextButtonLabels function updates the button label for each question in the survey. This function can be optimized by using a for loop instead of forEach to allow early termination when the last question is reached:

const updateEmptyNextButtonLabels = (labelValue: string) => {
  for (let index = 0; index < localSurvey.questions.length - 1; index++) {
    const q = localSurvey.questions[index];
    if (!q.buttonLabel || q.buttonLabel?.trim() === "") {
      updateQuestion(index, { buttonLabel: labelValue });
    }
  }
};

3. In the SurveyMenuBar.tsx file, the validateSurvey function checks for duplicate choices in the question. This can be optimized by using a Set to check for duplicates:

if (
  question.type === TSurveyQuestionType.MultipleChoiceSingle ||
  question.type === TSurveyQuestionType.MultipleChoiceMulti
) {
  const choiceLabels = new Set(question.choices.map((choice) => choice.label.trim().toLowerCase()));
  if (choiceLabels.has("") || choiceLabels.size !== question.choices.length) {
    toast.error("You have empty or duplicate choices.");
    return false;
  }
}

4. In the SurveyMenuBar.tsx file, the saveSurveyAction function validates the survey before saving. This function can be optimized by moving the validation check to the beginning of the function to avoid unnecessary computations:

const saveSurveyAction = async (shouldNavigateBack = false) => {
  if (!validateSurvey(localSurvey)) {
    setIsSurveySaving(false);
    return;
  }
  // rest of the function...
};

5. In the SurveyMenuBar.tsx file, the handleSurveyPublish function validates the survey before publishing. This function can be optimized by moving the validation check to the beginning of the function to avoid unnecessary computations:

const handleSurveyPublish = async () => {
  if (!validateSurvey(localSurvey)) {
    setIsSurveyPublishing(false);
    return;
  }
  // rest of the function...
};

The new code added in the PR seems to be handling a new case for TSurveyQuestionType.Matrix. The code is well-written and follows the existing pattern of the switch-case structure. However, there are a few improvements that can be made:

1. The key prop is missing in the map functions. React uses the key prop to identify which items have changed, are added, or are removed and thus it is a good practice to provide unique keys when rendering dynamic children.

2. The th tag is used in the tbody section, which is semantically incorrect. The th tag is meant for table headers and should be used in the thead section. For table data, the td tag should be used.

Here are the code suggestions:

// Add key prop in map functions
{firstQuestion.columns.map((column, index) => {
  return <th key={index} className="max-w-40 break-words px-4 py-2 text-gray-800">{column}</th>;
})}
{firstQuestion.rows.map((row, rowIndex) => {
  return (
    <tr key={rowIndex} className={`${rowIndex % 2 === 0 ? "bg-gray-100" : ""}`}>
      <td className="max-w-40 break-words px-4 py-2">{row}</td>
      {firstQuestion.columns.map((_, colIndex) => {
        return (
          <td key={colIndex} className="px-4 py-2 text-gray-800">
            <Section className="h-4 w-4 rounded-full bg-white p-2 outline"></Section>
          </td>
        );
      })}
    </tr>
  );
})}

// Use td tag instead of th in tbody
{firstQuestion.columns.map(() => {
  return (
    <td className="px-4 py-2 text-gray-800">
      <Section className="h-4 w-4 rounded-full bg-white p-2 outline"></Section>
    </td>
  );
})}

These changes will improve the semantic correctness of your HTML and help React optimize re-rendering.
In the apps/web/app/lib/questions.ts file, you've added a new question type "Matrix". It's a good addition, but it would be better to validate the rows and columns fields in the preset object to ensure they are not empty. This can be done by adding a simple check like:

if (!preset.rows.length || !preset.columns.length) {
  throw new Error("Rows and columns cannot be empty for Matrix question type");
}

In the packages/lib/responses.ts file, you've added a new function processResponseData to handle different types of response data. This is a good approach, but it would be better to handle the case where responseData is an object but not an array. You can add a check like:

else if (typeof responseData === "object") {
  return JSON.stringify(responseData);
}

In the apps/web/app/api/pipeline/lib/handleIntegrations.ts file, you've replaced the existing response processing logic with the processResponseData function. This is a good refactor, but it would be better to handle the case where responseValue is undefined. You can add a check like:

responses.push(responseValue !== undefined ? processResponseData(responseValue) : "");

In the apps/web/app/lib/surveys/surveys.ts file, you've added a new case for the Matrix question type in the generateQuestionAndFilterOptions function. This is a good addition, but it would be better to validate the filterType.filterValue and filterType.filterComboBoxValue fields to ensure they are not empty. This can be done by adding a simple check like:

if (!filterType.filterValue || !filterType.filterComboBoxValue) {
  throw new Error("Filter value and combo box value cannot be empty for Matrix question type");
}

The code in this PR is generally well-written and follows good practices. However, there are a few areas where improvements can be made:

1. In the buildWhereClause function, there are several instances where you're checking if a property exists on filterCriteria before using it. This is good, but you can make it more concise by destructuring filterCriteria at the start of the function. This way, you only need to check if the property is not undefined before using it.

const { finished, createdAt, tags, personAttributes, data } = filterCriteria || {};

2. In the getResponsesJson function, you're using processResponseData to process the response data. However, the processResponseData function is not defined in this file. Make sure to import it at the top of the file.

import { processResponseData } from './responseProcessing';

3. In the getQuestionWiseSummary function, you're using a switch statement to handle different question types. This is fine, but it can be improved by using a strategy pattern. This way, you can avoid the long switch statement and make the code more maintainable.

const questionHandlers = {
  [TSurveyQuestionType.OpenText]: handleOpenTextQuestion,
  [TSurveyQuestionType.MultipleChoiceSingle]: handleMultipleChoiceQuestion,
  // ...
};

function handleOpenTextQuestion(question, responses) {
  // ...
}

function handleMultipleChoiceQuestion(question, responses) {
  // ...
}

// ...

survey.questions.forEach((question) => {
  const handler = questionHandlers[question.type];
  if (handler) {
    handler(question, responses);
  }
});

4. In the calculateTtcTotal function, you're creating a copy of ttc and then adding a _total property to it. This is fine, but it can be simplified by using the reduce function directly on Object.values(ttc).

export function calculateTtcTotal(ttc: TResponseTtc) {
  return {
    ...ttc,
    _total: Object.values(ttc).reduce((acc: number, val: number) => acc + val, 0),
  };
}

5. In the getQuestionWiseSummary function, you're using convertFloatTo2Decimal to convert a float to two decimal places. However, the convertFloatTo2Decimal function is not defined in this file. Make sure to import it at the top of the file.

import { convertFloatTo2Decimal } from './numberProcessing';

Hey @ShubhamPalriwala , have updated the PR with mentioned changes 😊

Regarding the issue of the UI breaking with lengthy row and column names, we've chosen to implement tooltips exclusively on the summary page. However, for the question page, our approach is to display the entire label and offer users the necessary scrolling options to view these labels in full.

hey! here's what I noticed (if unrelated to this PR pls fix in a separat PR):

1. default NPS quesiton:

2. CTA question doesn't have a text by default

3. Unusual red frame for file upload

4. It's kind of hard to hit the radio button. I know there are a few CSS tricks to make it easier to click (e.g. adding an invisible ring or similar. Pls choose a solution which works stable on all browsers and devices:

5. Make use of new css classes so that the question can be styled with new styling (colors and roundness)

5.b replace color with styled color

6. the spacing is a bit off, reduce a bit

7. contrast is weak, not sure which color this text is but should be slate-950 and reduce the darkest teal so that it does not get as dark and the contrast is better:

8. replace with filled star (not only outline). It's ok to custom SVG component for this this lucide doesnt provide

9. Remove space before the ":"

The rest looks good, thanks! :))

Hey @jobenjada , have fixed the mentioned issues
Issue 1.2,3,5b and 8 seems unrelated to this PR (tested on prod) so will create a separate PR for it 😊

Fixed issue - 4

Fixed issue - 5

Fixed issue - 7

You can also adjust the value for it in function getOpacityLevel in MatrixQuestionSummary.tsx, where you can change value of 15 between 0 to 25, where moving towards 25 makes it darker

Fixed issue - 6

Fixed issue - 9