[PR #1819] [MERGED] fix(staffml): reset QuestionVisual failed state on visual.path change #34609

Closed
opened 2026-07-14 19:22:19 -05:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/harvard-edge/cs249r_book/pull/1819
Author: @farhan523
Created: 5/29/2026
Status: Merged
Merged: 6/2/2026
Merged by: @profvjreddi

Base: devHead: fix/question-visual-reset-on-prop-change


📝 Commits (1)

  • c19d9fa fix(staffml): reset QuestionVisual failed state on visual.path change

📊 Changes

3 files changed (+58 additions, -0 deletions)

View changed files

interviews/staffml/src/__tests__/question-visual.test.tsx (+45 -0)
📝 interviews/staffml/src/__tests__/setup.ts (+8 -0)
📝 interviews/staffml/src/components/QuestionVisual.tsx (+5 -0)

📄 Description

Summary

  • A question whose diagram failed to load left QuestionVisual in its error state across navigation. failed=true short-circuited before the next question's <img> could mount, so its onError could never reset things. Users saw "Diagram failed to load" on questions whose images were fine.
  • Fix uses the same useState + compare pattern as vault/TopicDetail.tsx, which React's docs recommend for resetting state when a prop changes.
  • Adds a regression test (renders, errors, re-renders with new path, asserts the error UI is cleared).
  • Adds a ResizeObserver stub in the vitest setup.ts so react-medium-image-zoom mounts cleanly under jsdom — without it the new test crashes.

Test plan

  • npx vitest run → 39/39 passing (was 37/37)
  • npx tsc --noEmit → clean
  • Manual verification: open /practice, block a diagram URL in DevTools → red error shows, navigate to next question → new diagram loads (instead of staying in error state)

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/harvard-edge/cs249r_book/pull/1819 **Author:** [@farhan523](https://github.com/farhan523) **Created:** 5/29/2026 **Status:** ✅ Merged **Merged:** 6/2/2026 **Merged by:** [@profvjreddi](https://github.com/profvjreddi) **Base:** `dev` ← **Head:** `fix/question-visual-reset-on-prop-change` --- ### 📝 Commits (1) - [`c19d9fa`](https://github.com/harvard-edge/cs249r_book/commit/c19d9faf5fc400f2b2df72bb13f3c2cbff1ea025) fix(staffml): reset QuestionVisual failed state on visual.path change ### 📊 Changes **3 files changed** (+58 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `interviews/staffml/src/__tests__/question-visual.test.tsx` (+45 -0) 📝 `interviews/staffml/src/__tests__/setup.ts` (+8 -0) 📝 `interviews/staffml/src/components/QuestionVisual.tsx` (+5 -0) </details> ### 📄 Description ## Summary - A question whose diagram failed to load left `QuestionVisual` in its error state across navigation. `failed=true` short-circuited before the next question's `<img>` could mount, so its `onError` could never reset things. Users saw "Diagram failed to load" on questions whose images were fine. - Fix uses the same `useState` + compare pattern as `vault/TopicDetail.tsx`, which React's docs recommend for resetting state when a prop changes. - Adds a regression test (renders, errors, re-renders with new path, asserts the error UI is cleared). - Adds a `ResizeObserver` stub in the vitest `setup.ts` so `react-medium-image-zoom` mounts cleanly under jsdom — without it the new test crashes. ## Test plan - [x] `npx vitest run` → 39/39 passing (was 37/37) - [x] `npx tsc --noEmit` → clean - [ ] Manual verification: open `/practice`, block a diagram URL in DevTools → red error shows, navigate to next question → new diagram loads (instead of staying in error state) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-07-14 19:22:19 -05:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/cs249r_book#34609