Add dark mode support#32
Open
dqnykamp wants to merge 7 commits into
Open
Conversation
- Add ThemeSetting type ("light" | "dark" | "system") and useResolvedTheme
hook in src/utils/theme.ts, mirroring DoenetML's implementation
- Import assignment-viewer.css with CSS variables for light/dark themes
(--canvas, --canvasText, --buttonSurface, --buttonSurfaceAlt) consistent
with DoenetML's [data-theme] variables
- Wrap ActivityViewer output in a div with class assignment-viewer-root and
data-theme attribute; resolve darkMode prop (including "system") before
passing the concrete "light"/"dark" value down to child components
- Thread standaloneUrl and cssUrl props through the full component chain
(ActivityViewer → Viewer → Activity → Sequence/Select/SingleDocActivity)
so callers can supply a local standalone bundle instead of the CDN
- Replace all hardcoded button background colors with CSS variables
- Add vite dev plugin to serve local DoenetML standalone build
- Use local standalone in dev/App.tsx; add dark mode selector to controls
- Export ThemeSetting from src/index.ts
- Upgrade @doenet/doenetml-iframe to 0.7.20-dev.20260627181531.cafd571
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Set up Cypress component testing (matching DoenetML's pattern) and add dark mode tests for ActivityViewer: - cypress.config.ts with Vite bundler and React framework - test/cypress/support/component.ts — registers cy.mount - test/cypress/support/component-index.html - test/cypress/component/helpers.ts — derives CDN URLs for the standalone bundle from the installed @doenet/doenetml-iframe package version - test/cypress/component/ActivityViewer.darkMode.cy.tsx — tests: - data-theme attribute and root background in light/dark - iframe element background (CSS rule) - iframe body background (loaded from CDN) - prop change after mount (dark→light→dark) - test/tsconfig.json with cypress types - npm scripts: test-cypress (open) and test-cypress-all (headless) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix tsconfig extends path (../../ → ../) - Use ESM import for package.json version in helpers.ts instead of require() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merged
- Remove serveDoenetStandalone vite plugin and standaloneUrl/cssUrl from dev/App.tsx — dev server now uses the CDN version via the installed @doenet/doenetml-iframe package (no sibling-directory dependency) - Add cypress.config.ts to tsconfig.node.json include so ESLint can parse it - Add test/tsconfig.json to ESLint parserOptions.project so cypress test files are linted correctly - Fix no-confusing-void-expression in theme.ts and darkMode test - Add eslint-disable comments for Cypress's required namespace/interface pattern in test/cypress/support/component.ts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- selectState.ts, sequenceState.ts, singleDocState.ts: remove redundant .toString() on string id fields (numbers keep .toString()) - Viewer.tsx: remove unused ActivityAndDoenetState/isActivityAndDoenetState imports; eslint-disable setState-in-useMemo (intentional error handling) - activity-viewer.tsx: eslint-disable ref access during render (intentional pattern for tracking prop changes across renders) - dev/App.tsx: eslint-disable unsafe-assignment for message_id passthrough - activityStateReducer.test.ts: file-level disable for unsafe-assignment, unsafe-member-access, no-non-null-assertion (test spy access patterns) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
These were added to support loading from a local sibling ../DoenetML directory, which we've since removed. The iframe component handles CDN URL resolution internally from the installed package version. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds dark mode support to the assignment viewer by introducing a ThemeSetting ("light" | "dark" | "system") with a resolver hook, applying theme-driven CSS variables, and wiring theme + standalone bundle URLs through the component stack so consumers can control theming and asset sources.
Changes:
- Introduces
ThemeSettinganduseResolvedTheme, and applies resolved theme via a root wrapperdata-themeattribute. - Adds theme CSS variables and updates button styles to use variables instead of hardcoded colors.
- Adds Cypress component test scaffolding and a dark mode test suite; threads
standaloneUrl/cssUrlthrough components and updates dev controls.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.node.json | Includes cypress.config.ts in node TS project. |
| test/tsconfig.json | Adds a dedicated TS project for Cypress/component tests. |
| test/cypress/support/component.ts | Adds cy.mount() command typing and registration. |
| test/cypress/support/component-index.html | Provides component-test index HTML with Cypress root mount point. |
| test/cypress/component/helpers.ts | Builds CDN URLs for standalone bundle/CSS and exports timeouts. |
| test/cypress/component/ActivityViewer.darkMode.cy.tsx | Adds Cypress component tests validating dark/light theming and live updates. |
| src/Viewer/Viewer.tsx | Threads standaloneUrl/cssUrl and switches button colors to CSS variables. |
| src/utils/theme.ts | Adds ThemeSetting and useResolvedTheme hook to resolve "system" to a concrete theme. |
| src/test/activityStateReducer.test.ts | Adjusts linting disables in an existing unit test file. |
| src/index.ts | Exports ThemeSetting type publicly. |
| src/assignment-viewer.css | Adds theme CSS variables and theme-scoped styling for root + iframes. |
| src/Activity/singleDocState.ts | Simplifies RNG seed construction (removes redundant .toString()). |
| src/Activity/SingleDocActivity.tsx | Threads standaloneUrl/cssUrl and updates button colors to CSS variables. |
| src/Activity/sequenceState.ts | Simplifies RNG seed construction (removes redundant .toString()). |
| src/Activity/SequenceActivity.tsx | Threads standaloneUrl/cssUrl through sequence activity path. |
| src/Activity/selectState.ts | Simplifies RNG seed construction (removes redundant .toString()). |
| src/Activity/SelectActivity.tsx | Threads standaloneUrl/cssUrl through select activity path. |
| src/Activity/Activity.tsx | Threads standaloneUrl/cssUrl through activity dispatcher. |
| src/activity-viewer.tsx | Wraps output in themed root div, resolves "system" theme, and passes resolved theme down. |
| package.json | Adds Cypress scripts/dependencies and bumps @doenet/doenetml-iframe peer version. |
| package-lock.json | Locks Cypress-related dependencies and updated @doenet/doenetml-iframe version graph. |
| eslint.config.js | Adds test/tsconfig.json to typed-eslint project list. |
| dev/App.tsx | Adds dark mode selector in dev controls and passes darkMode into ActivityViewer. |
| cypress.config.ts | Adds Cypress component testing configuration using Vite. |
Comments suppressed due to low confidence (1)
src/Viewer/Viewer.tsx:98
- Calling
setErrMsg(...)inside theuseMemocallback is a state update during render (hence the eslint disable) and can trigger React warnings or unexpected behavior in concurrent rendering. Instead, compute asourceErrorMessagein the memo and update state from an effect.
return {
numActivityVariants: {},
sourceHash: "",
numItems: 0,
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+16
to
+20
| function getSystemTheme(): ResolvedTheme { | ||
| return window.matchMedia("(prefers-color-scheme: dark)").matches | ||
| ? "dark" | ||
| : "light"; | ||
| } |
Comment on lines
+42
to
+55
| export function useResolvedTheme(setting: ThemeSetting): ResolvedTheme { | ||
| const subscribe = | ||
| setting === "system" ? subscribeToSystemTheme : subscribeToPinnedTheme; | ||
| const systemTheme = useSyncExternalStore( | ||
| subscribe, | ||
| getSystemTheme, | ||
| () => "light" as ResolvedTheme, | ||
| ); | ||
|
|
||
| if (setting === "system") { | ||
| return systemTheme; | ||
| } | ||
| return setting; | ||
| } |
Comment on lines
+3
to
+6
| "compilerOptions": { | ||
| "noEmit": true, | ||
| "types": ["cypress", "node"] | ||
| }, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements dark mode for the assignment viewer, consistent with how DoenetML handles theming.
Changes
ThemeSettingtype ("light" | "dark" | "system") anduseResolvedThemehook insrc/utils/theme.ts, mirroring DoenetML's implementation."system"tracksprefers-color-schemelive.src/assignment-viewer.cssfor light/dark themes (--canvas,--canvasText,--buttonSurface,--buttonSurfaceAlt), consistent with DoenetML's[data-theme]variables.ActivityViewerwraps its output in adivwithclass="assignment-viewer-root"anddata-themeattribute. ThedarkModeprop now acceptsThemeSetting(default"system") and resolves it before passing the concrete"light"/"dark"value to child components.standaloneUrlandcssUrlprops threaded through the full component chain so callers can supply a local standalone bundle instead of the CDN version embedded in the activity source.lightgray/rgb(237, 242, 247)withvar(--buttonSurface)/var(--buttonSurfaceAlt)andvar(--canvasText).@doenet/doenetml-iframeto0.7.20-dev.20260627181531.cafd571.