Skip to content

Java Lab: refactor and migrate Theater to ts#73345

Merged
molly-moen merged 2 commits into
stagingfrom
molly/migrate-theater-ts
Jun 18, 2026
Merged

Java Lab: refactor and migrate Theater to ts#73345
molly-moen merged 2 commits into
stagingfrom
molly/migrate-theater-ts

Conversation

@molly-moen

@molly-moen molly-moen commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Small refactor to prep for lab2 Java Lab theater:

  • Migrate Theater.js to typescript
  • Move Theater.ts to the shared mini apps folder for reuse
  • Also migrate TheaterTest to ts and use jest rather than sinon.

Testing story

Tested there was no impact on existing theater in Java Lab. Theater in Java Lab on lab2 is coming next.

) {
this.canvas = null;
this.context = null;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these were not used

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors Java Lab’s Theater mini-app in preparation for Lab2 Java Lab Theater by migrating it to TypeScript, relocating it under apps/src/miniApps/ for shared use, and updating unit tests to Jest.

Changes:

  • Migrate Theater implementation to TypeScript and move it to apps/src/miniApps/theater/.
  • Update Java Lab to import Theater from the new shared mini-app path.
  • Convert Theater unit tests to TypeScript and Jest mocks/assertions.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
apps/test/unit/miniApps/theater/TheaterTest.ts Migrates the Theater unit tests from sinon/chai style to Jest and updates imports to the new Theater location.
apps/src/miniApps/theater/Theater.ts TypeScript migration of Theater, adds typing, and adjusts implementation details for the new shared mini-app location.
apps/src/javalab/Javalab.js Switches Java Lab’s Theater import to the new @cdo/apps/miniApps/theater/Theater path.
Comments suppressed due to low confidence (3)

apps/src/miniApps/theater/Theater.ts:17

  • handleSignal assumes certain fields exist (e.g. data.detail.url for AUDIO_URL/VISUAL_URL), but the TheaterSignal type currently allows any string value and makes url optional. This weakens the TypeScript migration and can silently produce an invalid src like "undefined?=..." if url is missing. Model the signal as a discriminated union so url is required for the URL signals and detail is optional for NO_AUDIO.
    apps/src/miniApps/theater/Theater.ts:184
  • XMLHttpRequest.onload fires for any completed response (including HTTP 4xx/5xx). Assigning xhr.onload = onSuccess will report upload success even when the server rejected the PUT. Route non-2xx statuses to the error handler (and keep onerror for network failures).
    apps/test/unit/miniApps/theater/TheaterTest.ts:84
  • This test previously asserted the audio play function was called exactly once after both media elements loaded. toHaveBeenCalled() is weaker and would still pass if playback was triggered multiple times; use toHaveBeenCalledTimes(1) to keep the original intent.

@molly-moen molly-moen marked this pull request as ready for review June 18, 2026 20:02
@molly-moen molly-moen requested a review from a team June 18, 2026 20:02
} from './redux/javalabRedux';
import javalabView, {setDisplayTheme} from './redux/viewRedux';
import Theater from './theater/Theater';
import TheaterVisualizationColumn from './theater/TheaterVisualizationColumn';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: consistent relative/absolute import for these two in same dir?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they aren't the same directory anymore, the theater visualization column is still in the javalab directory.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops sorry!

@molly-moen molly-moen merged commit 25941af into staging Jun 18, 2026
8 checks passed
@molly-moen molly-moen deleted the molly/migrate-theater-ts branch June 18, 2026 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants