Fix replay timing, navigation, and sidebar behavior - #1364
Conversation
|
A preview of 8e98d4d is uploaded and can be seen here: ✨ https://revisit.dev/study/PR1364 ✨ Changes may take a few minutes to propagate. |
There was a problem hiding this comment.
Pull request overview
This PR improves analysis replay correctness and UX across task timing, navigation resets, header controls, and sidebar layout, with added unit/E2E regressions to prevent replay-time drift and navigation carryover.
Changes:
- Make the persisted task interval authoritative for replay timing (including when media ends early) and add replay reset support when navigating between task identifiers.
- Preserve original response timing when revisiting/editing completed answers and clear the one-shot “Previous” state.
- Improve replay/analysis UI behavior (disable “Next Participant” in replay; make long sidebars independently scrollable above the replay footer) and add targeted Playwright coverage.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/utils.ts | Adds IndexedDB helper for reading stored component timing in Playwright. |
| tests/demo-trrack.spec.ts | Adds replay navigation reset assertion (timer returns to ~0 after task change). |
| tests/demo-html.spec.ts | Adds regression to ensure revisiting a completed task preserves timing. |
| tests/demo-form-elements.spec.ts | Adds sidebar overflow/layout replay regressions and timing persistence checks. |
| src/store/hooks/useReplay.ts | Reworks replay clocking to use a synthetic task clock, handles shorter media, and adds resetReplay(). |
| src/store/hooks/useNextStep.ts | Preserves original timing/windowEvents when revisiting completed answers; clears clickedPrevious. |
| src/store/hooks/tests/useReplay.spec.tsx | Adds unit coverage for authoritative task clock behavior and resetReplay(). |
| src/store/hooks/tests/useNextStep.spec.tsx | Adds unit test for preserving original timing on revisited completed answers. |
| src/store/hooks/tests/useNextStep.spec.ts | Updates mocks for setClickedPrevious. |
| src/components/tests/StepRenderer.spec.tsx | Updates replay hook mock to include resetReplay; adds identifier mock. |
| src/components/StepRenderer.tsx | Resets replay state on identifier navigation; constrains sidebar via footer height; remounts footer by identifier. |
| src/components/interface/tests/AppNavBar.spec.tsx | Updates AppNavBar signature and asserts constrained/sticky sidebar behavior. |
| src/components/interface/tests/AppHeader.spec.tsx | Adds unit coverage ensuring “Next Participant” is disabled in analysis replay mode. |
| src/components/interface/AppNavBar.tsx | Adds bottom prop and makes sidebar sticky + independently scrollable with max-height constraint. |
| src/components/interface/AppHeader.tsx | Disables “Next Participant” menu item in analysis replay mode. |
| src/analysis/individualStudy/thinkAloud/ThinkAloudFooter.tsx | Adds missing aria-labels to replay control icons for accessibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I can still reproduce #1362 (jumping between components still leaves blank space in the timeline) Here are some findings:
|
Summary
Closes #1360
Closes #1361
Closes #1362
Closes #1363
Validation
yarn unittest --run— 2,038 passed, 1 skippedyarn typecheckyarn lintyarn buildgit diff --checkPlaywright was not run locally, per the repository instruction; the targeted scenarios are included for CI or maintainer execution.