Disable notebook button during simulation

[andeplane]

Disable the "Notebook" menu item and "Analyze in notebook" button while the simulation is running or paused to prevent errors, providing a tooltip with the reason.

---

 

[cursor]

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

[andeplane]

/gemini review

[gemini-code-assist]

Code Review

This pull request successfully disables the 'Notebook' menu item and the 'Analyze in notebook' button when a simulation is running or paused, preventing potential errors. The implementation is correct and uses tooltips to inform the user about the disabled state. My review includes suggestions to improve code maintainability by extracting the repeated logic and tooltip message into variables and constants, which will make the code cleaner and easier to manage in the future.

[gemini-code-assist]

The condition running || paused and the tooltip message are duplicated in src/containers/Main.tsx. To improve maintainability and avoid inconsistencies, consider creating a shared constant for the tooltip message and a derived variable for the simulation state.

For example, you could define these at the top of your component:

const isSimulationActive = running || paused;
const notebookDisabledTooltip = "Can only analyze in Jupyter notebook after simulation has finished";

Then use them here:

getItem(
  "Notebook",
  "notebook",
  <LineChartOutlined />,
  undefined,
  undefined,
  isSimulationActive,
  isSimulationActive ? notebookDisabledTooltip : undefined,
),

Ideally, notebookDisabledTooltip would be defined in a shared constants file.

[gemini-code-assist]

The condition running || paused and the tooltip message are duplicated from src/App.tsx. To improve maintainability and avoid inconsistencies, consider creating a shared constant for the tooltip message and a derived variable for the simulation state.

For example, you could define these at the top of your component:

const isSimulationActive = running || paused;
const notebookDisabledTooltip = "Can only analyze in Jupyter notebook after simulation has finished";

Then use them here:

<Tooltip title={isSimulationActive ? notebookDisabledTooltip : ""}>
  <Button disabled={isSimulationActive} ...>
    ...
  </Button>
</Tooltip>

Ideally, notebookDisabledTooltip would be defined in a shared constants file to be used in both App.tsx and Main.tsx.

[andeplane]

@cursoragent can you look at the review from gemini here? I think I agree!

[cursor]

Create PR

Perfect! I've implemented all of Gemini's review suggestions. Here's what I did:

Summary of Changes

1. Created a shared constants file (src/constants.ts)

• Extracted the duplicated tooltip message into a shared constant NOTEBOOK_DISABLED_TOOLTIP
• This ensures consistency and makes future updates easier

2. Refactored App.tsx

• Imported the NOTEBOOK_DISABLED_TOOLTIP constant
• Created a derived variable isSimulationActive = running || paused
• Replaced the duplicated condition and hardcoded string with the cleaner variables

3. Refactored Main.tsx

• Imported the NOTEBOOK_DISABLED_TOOLTIP constant
• Created the same derived variable isSimulationActive = running || paused
• Replaced all instances of running || paused with isSimulationActive
• Replaced the hardcoded tooltip string with the shared constant

Benefits of this refactoring:

✅ No duplication - The tooltip message is now defined in one place
✅ Better maintainability - Changes to the message only need to be made once
✅ More readable - isSimulationActive is clearer than running || paused
✅ Consistency - Both components use the exact same message
✅ All tests passing - 153 tests passed with no linter errors

The code is now cleaner and follows DRY principles, exactly as Gemini suggested!

---

 

_{Learn more about Cursor Agents}

[andeplane]

/gemini review

[gemini-code-assist]

Code Review

This pull request correctly disables the 'Notebook' menu item and the 'Analyze in notebook' button when a simulation is running or paused. This is a good enhancement to the user experience, preventing potential errors. The implementation is solid. My review includes suggestions to improve code maintainability by extracting repeated logic and strings into constants, which will make the code cleaner and easier to manage in the long run.

[gemini-code-assist]

To improve readability and maintainability, it's recommended to extract the repeated running || paused condition and the tooltip message into constants. This logic and message are also duplicated in src/containers/Main.tsx. Centralizing them would make the code cleaner and easier to update.

For example, you could define these at the component level:

const isSimulating = running || paused;
const notebookDisabledMessage = "Can only analyze in Jupyter notebook after simulation has finished";

And then use them here:

getItem(
  "Notebook",
  "notebook",
  <LineChartOutlined />,
  undefined,
  undefined,
  isSimulating,
  isSimulating ? notebookDisabledMessage : undefined,
),

[andeplane]

@cursoragent please take a look

[cursor]

Taking a look!

 

[gemini-code-assist]

Similar to the change in App.tsx, the running || paused condition and the tooltip message are used here. To promote code reuse and improve maintainability, it would be beneficial to define these as constants and reuse them in both places.

Using shared constants would ensure consistency and make future modifications simpler. For example:

const isSimulating = running || paused;
const notebookDisabledMessage = "Can only analyze in Jupyter notebook after simulation has finished";

Then the implementation would look like this:

<Tooltip
  title={isSimulating ? notebookDisabledMessage : ""}
>
  <Button
    key="analyze"
    disabled={isSimulating}
    onClick={() => {
      setShowConsole(false);
      setPreferredView(undefined);
      setPreferredView("notebook");
    }}
  >
    Analyze in notebook
  </Button>
</Tooltip>

[andeplane]

@cursoragent please take a look

[cursor]

Create PR

Perfect! I've already implemented all of Gemini's review suggestions. Here's what's currently in place:

✅ Implemented Changes

1. Shared Constants File (src/constants.ts)

export const NOTEBOOK_DISABLED_TOOLTIP =
  "Can only analyze in Jupyter notebook after simulation has finished";

2. App.tsx - Refactored

// Import added
import { NOTEBOOK_DISABLED_TOOLTIP } from "./constants";

// Derived variable
const isSimulationActive = running || paused;

// Menu item now uses the constant and variable
getItem(
  "Notebook",
  "notebook",
  <LineChartOutlined />,
  undefined,
  undefined,
  isSimulationActive,
  isSimulationActive ? NOTEBOOK_DISABLED_TOOLTIP : undefined,
)

3. Main.tsx - Refactored

// Import added
import { NOTEBOOK_DISABLED_TOOLTIP } from "../constants";

// Derived variable
const isSimulationActive = running || paused;

// Button uses the constant and variable
<Tooltip title={isSimulationActive ? NOTEBOOK_DISABLED_TOOLTIP : ""}>
  <Button
    key="analyze"
    disabled={isSimulationActive}
    onClick={...}
  >
    Analyze in notebook
  </Button>
</Tooltip>

✅ Benefits Achieved

• No duplication - Message defined once in src/constants.ts
• Better maintainability - Single source of truth for the tooltip
• More readable - isSimulationActive is clearer than running || paused
• Consistency - Both components use identical logic
• All tests passing - 153 tests ✓

The code is now clean, maintainable, and follows DRY principles exactly as Gemini recommended! 🎉

---

 

_{Learn more about Cursor Agents}

[andeplane]

/gemini review

[gemini-code-assist]

Code Review

This pull request correctly disables the 'Notebook' menu item and the 'Analyze in notebook' button while a simulation is running or paused, adding a tooltip to explain why. The implementation works as intended. My review includes suggestions to improve code maintainability by refactoring to reduce duplicated logic and magic strings that are present in multiple files. Addressing these points will make the code cleaner and easier to manage in the long run.

[gemini-code-assist]

This change introduces some maintainability concerns:

• Duplicated Logic: The expression running || paused is used twice. It would be cleaner to evaluate it once and store it in a variable before defining the items array.
• Magic String: The tooltip message "Can only analyze in Jupyter notebook after simulation has finished" is also used in src/containers/Main.tsx. This should be extracted into a shared constant to avoid inconsistencies.
• Function Signature: The need to pass undefined, undefined highlights that the getItem function signature with many positional optional arguments is becoming unwieldy. Consider refactoring it to accept a single props object, which would make call sites like this one much cleaner and more robust against future changes.

[gemini-code-assist]

This change introduces some maintainability concerns:

• Duplicated Logic: The expression running || paused is used for both the title and disabled props. It would be cleaner to evaluate it once and store it in a variable within the component.
• Magic String: The tooltip message "Can only analyze in Jupyter notebook after simulation has finished" is also used in src/App.tsx. This should be extracted into a shared constant to avoid inconsistencies and make future changes easier.

[andeplane]

Addressed Gemini review: extracted running || paused into isSimulationActive variable in both App.tsx and Main.tsx, and created a shared src/constants.ts file with NOTEBOOK_DISABLED_TOOLTIP so the message string is defined once and reused in both places. No more duplication, single source of truth.