Skip to content

Conversation

@kube
Copy link
Collaborator

@kube kube commented Jan 22, 2026

🌟 What is the purpose of this PR?

  • Improve UX in simulation/read-only mode by ensuring disabled/readonly controls display explanatory tooltips when hovered.

  • Define previously native components, as custom components in src/components to prepare future migration to the Design System.

  • Define withTooltip HoC as discussed with @lunelson to have a "standardized" way to compose Tooltips with other components. (Components from the Design System won't be aware of tooltips)

    This does not work for CodeEditor which is a special case.

🔗 Related links

🚫 Blocked by

Nothing

🔍 What does this change?

Tooltip Component Refactoring

  • Added display prop with two variants:
    • "block" (default): For full-width elements like inputs and selects
    • "inline": For buttons and inline elements in flex containers
  • Introduced withTooltip HoC to reduce boilerplate when adding tooltip support to components

New Reusable Form Components

All components use the withTooltip HoC for automatic tooltip support:

  • Input: Text input with monospace and hasError variants
  • NumberInput: Numeric input with monospace font
  • Slider: Range input slider
  • Button: Button with default and ghost variants
  • IconButton: Icon-only button with size (sm/md/lg) and variant (default/danger) options
  • Select: Dropdown select

CodeEditor Component

  • New wrapper around Monaco Editor with standardized options and read-only tooltip support
  • Shows tooltip on hover in read-only mode AND when user attempts to edit via onDidAttemptReadOnlyEdit listener
  • Uses isEditorMounted state to properly register listeners after Monaco mounts
  • Properly cleans up timeout on unmount to prevent memory leaks

Properties Panel Refactoring

Replaced native HTML form elements with new reusable components across all properties panels:

  • parameter-properties.tsx
  • transition-properties.tsx
  • place-properties.tsx
  • sortable-arc-item.tsx - Added tooltip support for arc weight inputs
  • type-properties.tsx
  • differential-equation-properties.tsx
  • place-initial-state.tsx

This removes redundant inline style definitions and ensures consistent tooltip behavior.

Component Updates

  • SegmentGroup: Now uses withTooltip HoC with disabled state styling
  • Switch: Now uses withTooltip HoC for consistent tooltip behavior

Read-Only Mode Enforcement

  • useIsReadOnly hook now treats Complete simulation state as read-only
  • Disabled add/delete buttons in sidebar lists (parameters, types, differential equations)
  • Disabled delete buttons in properties panels during simulation
  • Prevented Delete/Backspace keyboard deletion on canvas in read-only mode
  • Added tooltip explanations for all disabled controls

New UI Messages

  • Added DYNAMICS_REQUIRES_TYPE message for the Dynamics toggle when no type is selected

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • modifies an npm-publishable library and I have added a changeset file(s)

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

⚠️ Known issues

None

🐾 Next steps

disabled vs aria-disabled

This PR adds a span inside Tooltip component.

Not the best solution, but works with disabled: true on the child component.

One possible next step is to remove usage of disabled and rely instead on aria-disabled, which will remove the need for a wrapper.

🛡 What tests cover this?

  • No automated tests yet

❓ How to test this?

  1. Checkout the branch
  2. Run the petrinaut editor with yarn dev
  3. Start a simulation
  4. Verify that all disabled controls (buttons, inputs, selects, switches) show explanatory tooltips on hover
  5. Verify that hovering over the CodeEditor in read-only mode shows a tooltip
  6. Verify that attempting to edit the CodeEditor in read-only mode briefly shows a tooltip
  7. Verify that Input/Output Arc weight inputs show tooltips in read-only mode
  8. Verify that Delete/Backspace keys don't delete elements when in read-only mode

📹 Demo

N/A

@vercel
Copy link

vercel bot commented Jan 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments
Project Deployment Review Updated (UTC)
ds-theme Ignored Ignored Preview Jan 28, 2026 6:02pm
hashdotdesign Ignored Ignored Preview Jan 28, 2026 6:02pm
hashdotdesign-tokens Ignored Ignored Preview Jan 28, 2026 6:02pm

@github-actions github-actions bot added area/libs Relates to first-party libraries/crates/packages (area) type/eng > frontend Owned by the @frontend team labels Jan 22, 2026
Copy link
Collaborator Author

kube commented Jan 22, 2026

@cursor
Copy link

cursor bot commented Jan 23, 2026

PR Summary

Medium Risk
Broad UI refactor touching many editor controls and read-only gating; while mostly UX, it can subtly change interaction/disable behavior across the editor and should be smoke-tested in edit vs simulation states.

Overview
Fixes read-only/simulation UX by making tooltips reliably appear on disabled controls (via a Tooltip trigger wrapper and standardized display modes) and by propagating tooltips through a new withTooltip HOC.

Introduces reusable form controls (Button, IconButton, Input, NumberInput, Select, Slider) plus a CodeEditor wrapper, and replaces ad-hoc HTML controls/DisabledTooltip usage throughout editor properties panels and sidebar lists to ensure consistent disabling + explanatory tooltips.

Tightens read-only enforcement by extending useIsReadOnly to include simulation Complete, disabling add/delete actions in lists and properties, and blocking Delete/Backspace canvas deletion when read-only; adds UI_MESSAGES.DYNAMICS_REQUIRES_TYPE for clearer Dynamics toggle feedback.

Written by Cursor Bugbot for commit 99fdc4d. This will update automatically on new commits. Configure here.

cursor[bot]

This comment was marked as outdated.

@augmentcode
Copy link

augmentcode bot commented Jan 23, 2026

🤖 Augment PR Summary

Summary: This PR improves read-only UX during Petrinaut simulation by ensuring disabled/readonly controls can still display explanatory tooltips.

Changes:

  • Removed the <DisabledTooltip> wrapper pattern and made Tooltip accept optional content.
  • Introduced a CodeEditor wrapper around Monaco with tooltip support for read-only explanations.
  • Extended Switch with a tooltip prop and event-forwarding via ark.span to support tooltips on disabled states.
  • Extended SegmentGroup with disabled/tooltip props using ArkTooltip.Trigger (asChild) for disabled-state explanations.
  • Added a new UI message for “dynamics requires type” and applied it to the Dynamics switch when no type is selected.
  • Disabled delete/add actions across places, transitions, and sidebar lists while in read-only mode, and added tooltips explaining why.
  • Updated useIsReadOnly to treat the simulation Complete state as read-only.
  • Prevented Delete/Backspace keyboard deletion when the canvas is read-only.

Technical Notes: Read-only state is now centralized via useIsReadOnly and reused across editor panels, lists, and the SDCPN canvas.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

kube and others added 11 commits January 27, 2026 12:27
Using inline-flex caused block-level children like CodeEditor to lose
their width. Using flex ensures the wrapper takes full width while
still properly wrapping both block and inline elements.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Ensure the flex wrapper takes full width of its parent container
so block-level children like CodeEditor don't collapse.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Block-level div elements properly contain block-level children like
CodeEditor. The span element wasn't correctly handling the width
inheritance for Monaco Editor containers.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
The previous fix using flex with width: 100% was breaking the dimensions
of input elements. This change uses inline-flex instead, which preserves
the natural dimensions of input elements while still enabling tooltips
on disabled elements.

The inline-flex display mode ensures that:
1. Input elements maintain their intended width
2. Tooltips still work on disabled elements
3. The wrapper doesn't force unwanted width constraints
4. Alignment is preserved with align-items: center

Fixes issue with Tooltip wrapper breaking dimensions of inputs.
- Tooltip now accepts a `display` prop ("block" | "inline") to control
  wrapper element behavior, preventing layout issues
- Block mode (default): For full-width elements like inputs/selects
- Inline mode: For buttons and inline elements in flex containers
- Simplified SegmentGroup and Switch components to use shared Tooltip
- CodeEditor now shows tooltip on edit attempts in readonly mode via
  Monaco's onDidAttemptReadOnlyEdit event
- Added missing `display="inline"` to button tooltips throughout the app

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Introduces a reusable `withTooltip` higher-order component that adds `tooltip` and
`tooltipDisplay` props to any component. Refactors SegmentGroup and Switch to use
this HoC instead of inline tooltip handling.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Create Input, NumberInput, Slider, Button, IconButton, and Select components
- All components use withTooltip HoC for automatic tooltip support
- Replace native form elements with new components across:
  - parameter-properties.tsx
  - transition-properties.tsx
  - place-properties.tsx
  - sortable-arc-item.tsx
  - type-properties.tsx
  - differential-equation-properties.tsx
  - place-initial-state.tsx
- Remove redundant inline style definitions (inputStyle, deleteButtonStyle, etc.)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Use type aliases instead of interfaces for component props to follow
project conventions.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Fix memory leak by adding cleanup useEffect for timeout
- Fix edit listener not registering when readOnly changes dynamically
  by storing editor ref and using useEffect to manage listener lifecycle
- Fix tooltip not showing on hover in read-only mode by combining
  isHovering and showReadOnlyTooltip states
- Rename 'editor' variable to 'editorElement' to avoid shadowing
- Add display="inline" to AI menu tooltip in transition-properties

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Fix CodeEditor duplicate listener by using isEditorMounted state
  to trigger useEffect after Monaco mounts, removing registration
  from handleMount callback
- Add tooltip prop to SortableArcItem for read-only mode feedback
- Pass read-only tooltip to Input/Output Arc weight inputs

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copy link
Contributor

@lunelson lunelson left a comment

Choose a reason for hiding this comment

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

The parts related to Tooltip seem okay to me 👍🏼

Copy link
Member

@CiaranMn CiaranMn left a comment

Choose a reason for hiding this comment

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

Almost everything looks good and tooltips work well – a couple of issues in the DE section:

  1. If a type is not selected, the dropdown doesn't appear correctly (in readonly mode it's the same but also has no width in addition to no height)
Image
  1. In readonly mode, the code editor does not show
Image
  1. If populated, the dropdown appearance (with type selected) has changed in a different way in readonly and non-readonly mode

a. in readonly, it is content width only

Image

b. in not readonly it is full width and centre justified.

Image

Previously, it was always full width and left justified.

Image

- Add placeholder text and empty state for type dropdown in differential equation properties
- Simplify CodeEditor tooltip by using standard Tooltip component with className support
- Add className prop to Tooltip component using cx helper
- Update button styles with disabled variant styling for type properties
- Add read-only tooltip to place initial state number input

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

/>
);

export const Slider = withTooltip(SliderBase, "inline");
Copy link

Choose a reason for hiding this comment

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

Unused Slider component is dead code

Low Severity

The entire Slider component file is dead code. While the PR description lists it as a "New Reusable Form Component," it is never imported or used anywhere in the codebase. There's also an existing sliderStyle in simulation-controls.tsx with nearly identical styling that doesn't use this new component. This unused component adds maintenance burden without providing value.

Fix in Cursor Fix in Web

@kube kube added this pull request to the merge queue Jan 28, 2026
Merged via the queue into main with commit 673ece2 Jan 28, 2026
44 checks passed
@kube kube deleted the cf/h-5976-fix-readonly-tooltips branch January 28, 2026 18:32
@hashdotai hashdotai mentioned this pull request Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/infra Relates to version control, CI, CD or IaC (area) area/libs Relates to first-party libraries/crates/packages (area) type/eng > frontend Owned by the @frontend team

Development

Successfully merging this pull request may close these issues.

4 participants