Skip to content

Disable file editing while running simulation #243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andeplane wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Disable file editing while running simulation #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fde01eb
feat: Make editor read-only when simulation is running
cursoragent Nov 30, 2025
f59e3b9
Refactor: Update store creation in Edit tests
cursoragent Nov 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions src/containers/Edit.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { render, screen } from '@testing-library/react';
import { StoreProvider, createStore } from 'easy-peasy';
import Edit from './Edit';
import { StoreModel } from '../store/model';

// Mock Monaco Editor
vi.mock('@monaco-editor/react', () => ({
default: vi.fn(({ options, value }) => (
<div data-testid="monaco-editor" data-readonly={options.readOnly}>
{value}
</div>
)),
loader: {
init: vi.fn(() => Promise.resolve({
languages: {
register: vi.fn(),
setMonarchTokensProvider: vi.fn(),
},
})),
},
}));

describe('Edit', () => {
let store: any;

beforeEach(() => {
// Create a minimal store with the necessary state
store = createStore<Partial<StoreModel>>({
app: {
selectedFile: {
fileName: 'test.in',
content: 'units lj\natom_style atomic',
url: '',
},
setSelectedFile: vi.fn(),
setStatus: vi.fn(),
},
simulation: {
running: false,
paused: false,
showConsole: false,
files: [],
lammpsOutput: [],
simulation: {
id: 'test-sim',
files: [
{
fileName: 'test.in',
content: 'units lj\natom_style atomic',
url: '',
},
],
inputScript: 'test.in',
start: false,
},
resetLammpsOutput: vi.fn(),
addLammpsOutput: vi.fn(),
setShowConsole: vi.fn(),
setPaused: vi.fn(),
setCameraPosition: vi.fn(),
setCameraTarget: vi.fn(),
setSimulation: vi.fn(),
setRunning: vi.fn(),
setFiles: vi.fn(),
setLammps: vi.fn(),
extractAndApplyAtomifyCommands: vi.fn(),
syncFilesWasm: vi.fn(),
syncFilesJupyterLite: vi.fn(),
run: vi.fn(),
newSimulation: vi.fn(),
reset: vi.fn(),
},
} as any);
});

it('should render editor when file is selected', () => {
// Arrange & Act
render(
<StoreProvider store={store}>
<Edit />
</StoreProvider>
);

// Assert
expect(screen.getByTestId('monaco-editor')).toBeInTheDocument();
expect(screen.getByTestId('monaco-editor')).toHaveTextContent('units lj');
});

it('should render "No file selected" when no file is selected', () => {
// Arrange
store = createStore({
app: {
selectedFile: undefined,
setSelectedFile: vi.fn(),
setStatus: vi.fn(),
},
simulation: {
running: false,
paused: false,
showConsole: false,
files: [],
lammpsOutput: [],
resetLammpsOutput: vi.fn(),
addLammpsOutput: vi.fn(),
setShowConsole: vi.fn(),
setPaused: vi.fn(),
setCameraPosition: vi.fn(),
setCameraTarget: vi.fn(),
setSimulation: vi.fn(),
setRunning: vi.fn(),
setFiles: vi.fn(),
setLammps: vi.fn(),
extractAndApplyAtomifyCommands: vi.fn(),
syncFilesWasm: vi.fn(),
syncFilesJupyterLite: vi.fn(),
run: vi.fn(),
newSimulation: vi.fn(),
reset: vi.fn(),
},
} as any);

// Act
render(
<StoreProvider store={store}>
<Edit />
</StoreProvider>
);

// Assert
expect(screen.getByText('No file selected')).toBeInTheDocument();
expect(screen.queryByTestId('monaco-editor')).not.toBeInTheDocument();
});
Comment on lines +90 to +133
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 30, 2025

Choose a reason for hiding this comment

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

medium

This test case re-creates the entire store, which duplicates a lot of setup from the beforeEach block. To simplify this and avoid duplication, you can mutate the state of the existing store instance, similar to how it's done in other tests in this file. This makes the test suite cleaner and easier to maintain.

Note: Directly mutating the store's state via getState() is generally an anti-pattern, but since it's already used in this test file, this suggestion aims for consistency and reduction of duplicated code. A more robust long-term solution would be to use a test store factory function.

  it('should render "No file selected" when no file is selected', () => {
    // Arrange
    store.getState().app.selectedFile = undefined;

    // Act
    render(
      <StoreProvider store={store}>
        <Edit />
      </StoreProvider>
    );

    // Assert
    expect(screen.getByText('No file selected')).toBeInTheDocument();
    expect(screen.queryByTestId('monaco-editor')).not.toBeInTheDocument();
  });


it('should be editable when simulation is not running', () => {
// Arrange
store.getState().simulation.running = false;

// Act
render(
<StoreProvider store={store}>
<Edit />
</StoreProvider>
);

// Assert
const editor = screen.getByTestId('monaco-editor');
expect(editor).toHaveAttribute('data-readonly', 'false');
});

it('should be read-only when simulation is running', () => {
// Arrange
store.getState().simulation.running = true;

// Act
render(
<StoreProvider store={store}>
<Edit />
</StoreProvider>
);

// Assert
const editor = screen.getByTestId('monaco-editor');
expect(editor).toHaveAttribute('data-readonly', 'true');
});

it('should show notification banner when simulation is running', () => {
// Arrange
store.getState().simulation.running = true;

// Act
render(
<StoreProvider store={store}>
<Edit />
</StoreProvider>
);

// Assert
expect(screen.getByText(/File editing is disabled while simulation is running/i)).toBeInTheDocument();
});

it('should not show notification banner when simulation is not running', () => {
// Arrange
store.getState().simulation.running = false;

// Act
render(
<StoreProvider store={store}>
<Edit />
</StoreProvider>
);

// Assert
expect(screen.queryByText(/File editing is disabled while simulation is running/i)).not.toBeInTheDocument();
});

it('should update content when file content changes', () => {
// Arrange
const { rerender } = render(
<StoreProvider store={store}>
<Edit />
</StoreProvider>
);

// Act - Update the selected file content
store.getState().app.selectedFile = {
fileName: 'test.in',
content: 'units metal\natom_style full',
url: '',
};

rerender(
<StoreProvider store={store}>
<Edit />
</StoreProvider>
);

// Assert
expect(screen.getByTestId('monaco-editor')).toHaveTextContent('units metal');
});
});
36 changes: 27 additions & 9 deletions src/containers/Edit.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1415,8 +1415,10 @@ loader.init().then((monaco) => {
const Edit = () => {
const selectedFile = useStoreState((state) => state.app.selectedFile);
const simulation = useStoreState((state) => state.simulation.simulation);
const isRunning = useStoreState((state) => state.simulation.running);
const options = {
selectOnLineNumbers: true,
readOnly: isRunning,
};

const handleEditorDidMount = useCallback(
Expand Down Expand Up @@ -1444,15 +1446,31 @@ const Edit = () => {
}

return (
<Editor
height="100vh"
language="lammps"
theme="vs-dark"
value={selectedFile.content}
options={options}
onChange={onEditorChange}
onMount={handleEditorDidMount}
/>
<>
{isRunning && (
<div
style={{
backgroundColor: "#3a3a3a",
color: "#ffa500",
padding: "8px 16px",
fontSize: "12px",
borderBottom: "1px solid #555",
fontFamily: "monospace",
}}
Comment on lines +1452 to +1459
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 30, 2025

Choose a reason for hiding this comment

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

medium

For better readability and maintainability, consider extracting these inline styles into a constant object. This separates styling concerns from the component's structure. If your project uses a CSS-in-JS library or CSS modules, that would be an even better approach.

Example:

const bannerStyle: React.CSSProperties = {
  backgroundColor: "#3a3a3a",
  color: "#ffa500",
  padding: "8px 16px",
  fontSize: "12px",
  borderBottom: "1px solid #555",
  fontFamily: "monospace",
};

const Edit = () => {
  // ...
  return (
    // ...
      {isRunning && (
        <div style={bannerStyle}>
          ⓘ File editing is disabled while simulation is running
        </div>
      )}
    // ...
  );
}

>
ⓘ File editing is disabled while simulation is running
</div>
)}
<Editor
height={isRunning ? "calc(100vh - 33px)" : "100vh"}
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 30, 2025

Choose a reason for hiding this comment

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

medium

The hardcoded value 33px is brittle. If the banner's styling (e.g., padding, font-size) changes, this value will need to be manually updated, which is error-prone.

It's better to either define it as a constant with an explanatory comment, or refactor the layout to use Flexbox to avoid calculating heights manually.

Example with a constant:

const BANNER_HEIGHT_PX = 33; // Corresponds to banner padding, font size, and border.
// ...
<Editor
  height={isRunning ? `calc(100vh - ${BANNER_HEIGHT_PX}px)` : "100vh"}
  //...
/>

Example with Flexbox (more robust):

// Wrap the component's return in a flex container
<div style={{ display: 'flex', flexDirection: 'column', height: '100vh' }}>
  {isRunning && (
    <div style={bannerStyle}>
      ⓘ File editing is disabled while simulation is running
    </div>
  )}
  <div style={{ flex: 1, position: 'relative' }}>
    <Editor
      height="100%" // The editor will fill the remaining space
      // ... other props
    />
  </div>
</div>

The flexbox approach is recommended as it's more resilient to style changes.

language="lammps"
theme="vs-dark"
value={selectedFile.content}
options={options}
onChange={onEditorChange}
onMount={handleEditorDidMount}
/>
</>
);
};
export default Edit;