Fix/organize state#212
Open
meganrm wants to merge 16 commits into
Open
Conversation
…into fix/organize-state
meganrm
requested review from
ShrimpCryptid,
Copilot and
frasercl
and removed request for
a team
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors application state management by splitting the previously monolithic Simularium React context into three domain-focused contexts (UI, simulation, analysis), and updates components to consume typed hooks instead of useContext directly. This reduces provider churn and makes state ownership clearer across the app.
Changes:
- Split the context into
SimulariumUiContext,SimulariumSimulationContext, andSimulariumAnalysisContext, and added typed hooks (useSimulariumUi,useSimulariumSimulation,useSimulariumAnalysis). - Updated affected components to use the new hooks and adjusted state wiring in
App.tsxto provide three context values. - Added documentation (
CONTEXT_ORGANIZATION.md,CLAUDE.md), added a reusableResetButton, and updated.gitignoreto exclude.claude.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/simulation/context.tsx | Splits the single context type/provider into three contexts (UI/simulation/analysis). |
| src/hooks/useSimulationContext.ts | Adds typed hooks to access each new context. |
| src/components/ViewSwitch.tsx | Switches to using the new UI/simulation hooks. |
| src/components/Viewer.tsx | Switches to using the new UI/simulation hooks for viewer wiring. |
| src/components/StartExperiment.tsx | Switches to simulation hook for experiment start handler. |
| src/components/shared/VisibilityControl.tsx | Switches to UI hook for page/section/module visibility logic. |
| src/components/shared/ResetButton.tsx | Adds a new reset button component to reset analysis or all state. |
| src/components/shared/ProgressionControl.tsx | Switches to UI hook for page/progression state. |
| src/components/shared/NextButton.tsx | Switches to UI hook for page/module navigation. |
| src/components/shared/BackButton.tsx | Switches to UI hook for page navigation. |
| src/components/ScaleBar.tsx | Switches to simulation hook for max concentration. |
| src/components/quiz-questions/KdQuestion.tsx | Switches to UI hook for module completion tracking. |
| src/components/quiz-questions/EquilibriumQuestion.tsx | Switches to UI hook for quiz state. |
| src/components/plots/ProductConcentrationPlot.tsx | Switches to simulation hook for simulation/plot inputs. |
| src/components/plots/EventsOverTimePlot.tsx | Splits UI vs simulation reads via new hooks. |
| src/components/plots/EquilibriumPlot.tsx | Splits UI vs simulation reads via new hooks; modifies axis tick handling. |
| src/components/PlayButton.tsx | Switches to simulation hook for play/pause state. |
| src/components/PageIndicator.tsx | Switches to UI hook for module navigation/completion display. |
| src/components/MixButton.tsx | Switches to simulation hook for mix handler. |
| src/components/main-layout/RightPanel.tsx | Switches to simulation hook for productName. |
| src/components/main-layout/NavPanel.tsx | Switches to UI hook for setPage. |
| src/components/main-layout/LeftPanel.tsx | Uses UI hook to conditionally hide events plot in the competitive module. |
| src/components/main-layout/ContentPanelTimer.tsx | Switches to UI hook for page state. |
| src/components/LabView.tsx | Splits UI vs simulation reads via new hooks. |
| src/components/concentration-display/LiveConcentrationDisplay.tsx | Switches to simulation hook for color/max concentration. |
| src/components/concentration-display/ConcentrationSlider.tsx | Switches to analysis hook for recorded concentrations. |
| src/components/concentration-display/Concentration.tsx | Splits UI vs simulation reads via new hooks. |
| src/components/AdminUi.tsx | Switches to UI hook for page/module navigation. |
| src/App.tsx | Provides three context providers and memoized context values; refactors handlers accordingly. |
| CONTEXT_ORGANIZATION.md | Documents the new context split and hook usage. |
| CLAUDE.md | Adds repository-level development/guidelines documentation. |
| .gitignore | Ignores .claude directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+748
to
+749
| progressionElement: (pageContent.progressionElement ?? | ||
| "") as ProgressionElement, |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
ShrimpCryptid
approved these changes
May 22, 2026
Comment on lines
+11
to
+30
| /** | ||
| * Hook to access UI-related context (page, module, view type, etc.) | ||
| */ | ||
| export const useSimulariumUi = (): SimulariumUiContextType => { | ||
| return useContext(SimulariumUiContext); | ||
| }; | ||
|
|
||
| /** | ||
| * Hook to access simulation-related context (playback, viewport, trajectory, etc.) | ||
| */ | ||
| export const useSimulariumSimulation = (): SimulariumSimulationContextType => { | ||
| return useContext(SimulariumSimulationContext); | ||
| }; | ||
|
|
||
| /** | ||
| * Hook to access analysis-related context (recorded data, reset functions, etc.) | ||
| */ | ||
| export const useSimulariumAnalysis = (): SimulariumAnalysisContextType => { | ||
| return useContext(SimulariumAnalysisContext); | ||
| }; |
Contributor
There was a problem hiding this comment.
I like that the monolithic state is being split up! It still has some of the problems with useContext, namely that components will rerender on any change to the subscribed context, even if they only need a subset of the properties.
Would you consider using another state management library (like Zustand) in the future?
Contributor
Author
There was a problem hiding this comment.
yes, I was thinking of this as a step towards that
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.
Problem
Estimated review size: medium (it's a lot of files, but it should be easy to follow)
Tech debt: the context had gotten too large to manage as one set
Solution
separated out the context into 3 sections using claude code.
Type of change
Please delete options that are not relevant.
Steps to Verify: