Conversation
|
🪓 PR closed, deleted preview. |
JackWilb
requested review from
JackWilb and
Copilot
and removed request for
Copilot
There was a problem hiding this comment.
Pull request overview
Adds option-level “info text” support across response components, including matrix question/answer options, by allowing richer StringOption objects (with optional value) and updating UI rendering/normalization accordingly.
Changes:
- Extend matrix
answerOptions/questionOptionsto acceptStringOptionentries and renderinfoTextviaOptionLabel. - Make
StringOption.valueoptional (defaulting tolabel) and normalize option values across inputs and randomization. - Update JSON schemas and demo survey config to reflect the new option shapes.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/handleResponseRandomization.ts | Normalizes option/question values when randomizing to support StringOption without explicit value. |
| src/parser/types.ts | Makes StringOption.value optional; extends matrix option types to allow StringOption. |
| src/parser/StudyConfigSchema.json | Updates schema to allow matrix options as string or StringOption; makes StringOption.value optional. |
| src/parser/LibraryConfigSchema.json | Mirrors schema updates (matrix options + StringOption changes) and adds clickToRecord fields. |
| src/components/response/utils.ts | Adjusts matrix init-field generation for updated question option shapes (currently with a keying issue). |
| src/components/response/ResponseSwitcher.tsx | Initializes matrix fields using value ?? label for question keys. |
| src/components/response/RankingInput.tsx | Normalizes ranking option ids/values to handle missing value. |
| src/components/response/RadioInput.tsx | Normalizes radio option values and renders infoText in labels. |
| src/components/response/MatrixInput.tsx | Renders infoText for matrix rows/columns and normalizes question/answer values. |
| src/components/response/DropdownInput.tsx | Adds renderOption with OptionLabel to show option infoText in dropdowns. |
| src/components/response/CheckBoxInput.tsx | Normalizes checkbox option values and keys to handle missing value. |
| src/components/response/ButtonsInput.tsx | Renders option labels with OptionLabel and normalizes option values. |
| src/analysis/individualStudy/summary/utils.tsx | Formats matrix response option summaries when options are StringOption objects. |
| public/demo-survey/config.json | Updates demo config to use label/infoText objects (omitting value) for several responses, including matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Box | ||
| key={`choice-${idx}-label`} | ||
| style={{ | ||
| fontWeight: 'bold', | ||
| textAlign: 'center', | ||
| fontSize: '0.8em', | ||
| }} | ||
| mb="sm" | ||
| ml="xs" | ||
| mr="xs" | ||
| > | ||
| {entry.label} | ||
| </Text> | ||
| <OptionLabel label={entry.label} infoText={entry.infoText} /> | ||
| </Box> |
There was a problem hiding this comment.
The removal of fontWeight: 'bold' from the column header styling changes the visual appearance of matrix response column headers. Previously, column headers were displayed in bold text, but now they will appear in regular weight. If this visual change is intentional and aligns with the design in the screenshot, this comment can be disregarded. Otherwise, consider adding fontWeight: 'bold' to the style prop of the Box component or applying it through the OptionLabel component to maintain consistency with the previous design.
| const questions = parseStringOptions(response.questionOptions); | ||
| const questionsByValue = Object.fromEntries(questions.map((question) => [question.value, question])); |
There was a problem hiding this comment.
The questions variable is created on every render without useMemo, and it's used as a dependency in the orderedQuestions useMemo on line 146. This means orderedQuestions will be recalculated on every render even if response.questionOptions hasn't changed, because questions is a new array reference each time. Consider wrapping the parseStringOptions call in useMemo with response.questionOptions as the dependency to avoid unnecessary recalculations.
| const _choices: ParsedStringOption[] = typeof answerOptions === 'string' | ||
| ? parseStringOptions(_choiceStringToColumns[answerOptions]) | ||
| : parseStringOptions(answerOptions); |
There was a problem hiding this comment.
The _choices variable is created on every render without useMemo. Since it's used in the onChangeCheckbox function which is passed to child components, this could cause unnecessary re-renders of CheckboxComponent. Consider wrapping the _choices calculation in useMemo with answerOptions as a dependency to improve performance, especially for large matrices.
| const _choices: ParsedStringOption[] = typeof answerOptions === 'string' | |
| ? parseStringOptions(_choiceStringToColumns[answerOptions]) | |
| : parseStringOptions(answerOptions); | |
| const _choices: ParsedStringOption[] = useMemo( | |
| () => (typeof answerOptions === 'string' | |
| ? parseStringOptions(_choiceStringToColumns[answerOptions]) | |
| : parseStringOptions(answerOptions)), | |
| [answerOptions], | |
| ); |
Does this PR close any open issues?
Closes #1035
Give a longer description of what this PR addresses and why it's needed
Add info text to matrix responses (question, option)
Provide pictures/videos of the behavior before and after these changes (optional)
Are there any additional TODOs before this PR is ready to go?
TODOs: