Conversation
|
🪓 PR closed, deleted preview. |
There was a problem hiding this comment.
Pull request overview
This PR implements study conditions in URL query parameters, allowing researchers to define multiple experimental conditions within a single study configuration and select which condition to run via a URL parameter (e.g., ?condition=color). This addresses issue #799 by enabling condition-based filtering of study sequences without requiring duplicate study configurations.
Changes:
- Added
conditionfield toComponentBlockandDynamicBlocktypes and schemas to mark conditional blocks - Implemented condition filtering utilities (
parseConditionParam,filterSequenceByCondition,getSequenceConditions,getConditionParticipantCounts) - Modified sequence generation and storage to support condition-based filtering
- Updated UI components to display and select conditions (ConfigSwitcher landing page with dropdown)
- Preserved search parameters across navigation to maintain condition context
- Added demo-condition study showcasing color, size, and shape condition variants
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/handleSequenceConditions.ts | New utility module for parsing, filtering, and analyzing conditions in sequences |
| src/utils/handleSequenceConditions.spec.ts | Comprehensive test coverage for condition parsing and filtering functions |
| src/storage/engines/types.ts | Modified sequence generation logic to handle conditions; added updateParticipantSearchParams method |
| src/utils/handleRandomSequences.tsx | Updated to preserve condition field when converting ComponentBlocks to Sequences |
| src/utils/getSequenceFlatMap.ts | Added condition field preservation in path mapping |
| src/store/types.ts | Added optional condition field to Sequence interface |
| src/parser/types.ts | Added condition field to ComponentBlock and DynamicBlock interfaces with documentation |
| src/parser/StudyConfigSchema.json | Added condition property to ComponentBlock and DynamicBlock schemas |
| src/parser/LibraryConfigSchema.json | Added condition property to library schemas |
| src/components/Shell.tsx | Implemented condition filtering and handling for participant sessions |
| src/components/ConfigSwitcher.tsx | Added condition selector dropdown with participant counts per condition |
| src/components/interface/StepsPanel.tsx | Preserved search params in navigation |
| src/controllers/ComponentController.tsx | Preserved search params when auto-forwarding participants |
| src/utils/useNavigateToTrial.tsx | Added searchParams parameter to preserve condition when opening trials |
| src/analysis/individualStudy/thinkAloud/ThinkAloudFooter.tsx | Preserved participant search params in replay links |
| src/analysis/individualStudy/table/TableView.tsx | Added conditional Condition column when participants have conditions |
| src/analysis/individualStudy/replay/SingleTask.tsx | Added searchParams to task replay navigation |
| src/analysis/individualStudy/replay/AllTasksTimeline.tsx | Passed participant searchParams to SingleTask components |
| public/global.json | Registered demo-condition study |
| public/demo-condition/config.json | Demo study configuration with three conditions (color, size, shape) |
| public/demo-condition/assets/* | HTML and markdown assets for demo trials |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (hasConditions && config) { | ||
| // Generate fresh sequences and filter for this participant's condition | ||
| const fullSequenceArray = generateSequenceArray(config); | ||
| sequenceArray = fullSequenceArray.map((seq) => filterSequenceByCondition(seq, activeCondition)); |
There was a problem hiding this comment.
Is it possible that we don't generate a sequence with the requested condition if the config is large enough?
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Added reminder for participants to proceed correctly.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const matchingSequence = studyCondition | ||
| ? generatedSequences.find( | ||
| (sequence) => getSequenceConditions(sequence).includes(studyCondition), | ||
| ) | ||
| : generatedSequences[0]; |
There was a problem hiding this comment.
The fallback logic for finding a matching sequence when studyCondition is specified is incorrect. The code looks for a sequence that contains the specified condition using getSequenceConditions, but it should instead filter the selected sequence using filterSequenceByCondition.
Currently, if a condition is specified in the URL but an error occurs, the fallback will try to find a pre-generated sequence that happens to include that condition. However, this doesn't apply the condition filtering logic and may include all conditional blocks instead of just the matching ones.
The correct approach would be to select a sequence (e.g., generatedSequences[0]) and then apply filterSequenceByCondition to it, similar to how it's done in the success path at line 148.
There was a problem hiding this comment.
Updated it to filter using filterSequenceByCondition
const matchingSequence = generatedSequences[0];
const fallbackSequence = filterSequenceByCondition(
matchingSequence,
studyCondition,
);| > | ||
| <g> | ||
| <SingleTask incomplete={answer.startTime === 0} isCorrect={isCorrect} hasCorrect={hasCorrect} key={name} labelHeight={currentHeight * LABEL_GAP} height={maxHeight} name={name} xScale={scale} scaleStart={scaleStart} scaleEnd={scaleEnd} trialOrder={answer.trialOrder} participantId={participantData.participantId} studyId={studyId} /> | ||
| <SingleTask incomplete={answer.startTime === 0} isCorrect={isCorrect} hasCorrect={hasCorrect} key={name} labelHeight={currentHeight * LABEL_GAP} height={maxHeight} name={name} xScale={scale} scaleStart={scaleStart} scaleEnd={scaleEnd} trialOrder={answer.trialOrder} participantId={participantData.participantId} studyId={studyId} condition={participantData.searchParams?.condition} /> |
There was a problem hiding this comment.
Are there any issues with using participantData.searchParams here? Does this work on refresh? What if I switch to a different condition? How are the searchParams updated for a participant? They shouldn't be able to change conditions in a study, but we should be able to in dev.
There was a problem hiding this comment.
- Add a
studyConditionwith theParticipantDatain storagetypes.tsso it also works when it's refreshed since it's saved in the storage - Add if the mode is
developmentModeEnabled, allow updating condition
| {!isComponent && conditional && ( | ||
| <Badge size="xs" ml={5} variant="transparent">{label}</Badge> | ||
| )} |
There was a problem hiding this comment.
Do we need this with the new logic in the sequence?
There was a problem hiding this comment.
Use icon and tooltip instead of a badge
| ...participantSession.searchParams, | ||
| condition: studyCondition, | ||
| }; | ||
| await storageEngine.updateParticipantSearchParams(updatedSearchParams); |
There was a problem hiding this comment.
Does this let a participant change conditions mid way through a study?
There was a problem hiding this comment.
It will not update the condition unless in devMode
types.ts
async updateStudyCondition(condition: string | string[]) {
...
const modes = await this.getModes(this.studyId);
if (!modes.developmentModeEnabled) {
throw new Error('Cannot update study condition when development mode is disabled');
}
...
}
|
# Conflicts: # src/components/ConfigSwitcher.tsx # src/components/downloader/DownloadTidy.tsx
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
💡 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 38 out of 38 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Does this PR close any open issues?
Closes #799
Give a longer description of what this PR addresses and why it's needed
/path/to/study?condition=color,size
These would then be accessed via a query param:
/path/to/study?condition=size
/path/to/study?condition=color
/path/to/study?condition=size,color
The effect would be that blocks that don't match the condition specified in the query param would be ignored. So all blocks with no condition will be included, all blocks with the matching condition as well.
Provide pictures/videos of the behavior before and after these changes (optional)
Live demo:
https://revisit.dev/study/PR1023
https://revisit.dev/study/PR1023/demo-condition?condition=color
Landing page
Study
Analysis
Are there any additional TODOs before this PR is ready to go?
TODOs: