Skip to content

FE-778: Fix bounds editing, section stacking, and preview scrolling in the ad-hoc form - #9473

Merged
kube merged 17 commits into
cf/fe-1490-ad-hoc-scenario-user-guidefrom
cf/fe-778-ad-hoc-form-editing-and-layout-fixes
Sep 1, 2026
Merged

FE-778: Fix bounds editing, section stacking, and preview scrolling in the ad-hoc form#9473
kube merged 17 commits into
cf/fe-1490-ad-hoc-scenario-user-guidefrom
cf/fe-778-ad-hoc-form-editing-and-layout-fixes

Conversation

@kube

@kube kube commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

🌟 What is the purpose of this PR?

Three fixes to the ad-hoc scenario form, found while reviewing the stack below it. The optimize bounds popover ignored every mouse press: Min, Max, Step and Scale could not be edited at all, and each press dismissed the popover. A focused section painted over the sticky header of the section hosting it. The experiment drawer's computed initial state grew to whatever height the marking needed, pushing Metrics and the drawer footer out of view. Stacked on #9368; #9371 (optimization constraints) now sits on top of this.

🔗 Related links

🔍 What does this change?

  • The bounds popover takes its own presses. A portal container is commonly a full-bleed layer that lets presses through to the app beneath it (pointer-events: none), and the popover never took them back, so every press aimed at Min, Max, Step or Scale reached the row underneath instead. The dismiss handler read that as a press outside and closed the popover on it. The popover now declares pointer-events: auto.
  • The Scale list opens inside the popover. It portalled to the app container as a sibling of the popover, so choosing an option counted as an outside press: the popover closed and the value was lost, because the list commits on click and the element was already unmounted. The popover now provides itself as the portal container for the layers opened from inside it.
  • Editing one bound no longer overwrites another. The one-shot "select Min when the popover opens" was armed by the Min cell's ref attaching, and that ref is a fresh closure every render, so React re-armed it on every commit. Since each keystroke in a bound dispatches, typing in Max yanked focus to Min mid-edit and the next character replaced Min's bound. The guard is now the element the selection went to, so a re-attach of the same button does nothing and there is no ordering to get wrong.
  • A focused section stays under its host's sticky header. Section lifts itself to z-index: 3 on :focus-within and its sticky header sits at 2, both resolved in the same stacking context once sections nest, so focusing any cell inside the ad-hoc form lifted the nested section's title and rows over the header pinned above them. The lift is capped at 1: still above an unfocused sibling at 0, always below a header.
  • The computed state preview is one bounded panel. Parameters and initial state sit together in a tinted, bordered region (neutral.s20 on neutral.bd.subtle) that scrolls as one and stops at 320px, instead of adding their full height to the drawer.
  • Storybook's portal layers (the preview decorator's and the Simulate view story's) let presses through to the story but never restored them for their children, so every portalled surface in Storybook was inert: selects, menus, and this popover alike. Both now re-enable pointer events on the layer's children, which is what the app already does by portalling into .petrinaut-root.

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:

  • require changes to docs which are made as part of this PR

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

The changes in this PR:

  • do not affect the execution graph

⚠️ Known issues

The popover keeps a second dismissal path, an IntersectionObserver that closes it when its cell scrolls out of the clipped container, rebuilt whenever a bound editor opens or closes. It is untouched here. It did not fire in any case I tested, though a collapse animating around the cell is a plausible trigger and is not covered.

🐾 Next steps

  • The preview's scroll region is plain: the fade-gradient treatment used by the bottom panel's parameter list lives in two local copies (ParametersScrollArea, ScrollableContent) and is worth extracting before a third consumer wants it.
  • Yannis also suggested renaming "No scenario" (it produces a scenario, so the label misleads) on FE-1487: Quick simulation runs an ad-hoc scenario when none is selected #9287. Not touched here.

🛡 What tests cover this?

  • ad-hoc-scenario-form.test.tsx: a new case asserts the popover declares its own pointer events, that the Scale list renders inside the popover rather than beside it, and that editing one bound leaves focus in its editor and the neighbouring bound unchanged. Each assertion fails if its fix is reverted.
  • section.test.tsx (new): a nested Section inside another asserts the focus lift stays below the header tier and above the resting tier.
  • experiment-scenario-run.test.tsx (new): opening "Computed state" renders the places inside the bounded scroll region. It lowers the scenario with the real compiler, so the preview reaches its ready state.
  • jsdom computes no layout, so the CSS assertions read the emitted Panda classes. Paint order and scrolling were verified in the browser (below).

❓ How to test this?

  1. yarn workspace @hashintel/petrinaut dev and open Components / AdHocScenarioForm → Satellites.
  2. Turn Optimize on for altitude, then click its 0 … 1 cell twice to open the bounds popover.
  3. Click Scale. The list opens and the popover stays. Choose Log: the value commits (a validation notice appears, since a log domain needs a positive minimum).
  4. Double-click Max, type a digit. Focus stays in the editor and Min keeps its value. Click outside: the popover closes and the cell shows the new range.
  5. Open Simulate / SimulateView → RunSupplyChainOptimization, localStorage.setItem("petrinaut:user-settings", JSON.stringify({ enableAdHocScenarios: true })), reload, then Create → scenario No scenario. Shrink the viewport, scroll until "Initial state" passes under the pinned "Initial state and parameters" header, and click a place's count: the header stays opaque and legible.
  6. Open Simulate / SimulateView → RunSatellitesLauncherExperimentCreate → expand Computed state: the initial-state list scrolls within its own region and Metrics stays in view.

📹 Demo

The "before" for the sticky header is the screenshot that opened this work: "Initial state", "Initial state and parameters" and the focused place row superimposed. Step 5 above reproduces it on cf/fe-1518-optimization-constraints and shows the header opaque on this branch. Screenshots to attach.

@kube kube self-assigned this Aug 31, 2026
@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated
hash Ready Ready Preview Sep 1, 2026 3:30pm UTC
petrinaut Ready Ready Preview Sep 1, 2026 3:30pm UTC
petrinaut-docs Ready Ready Preview Sep 1, 2026 3:30pm UTC
1 Skipped Deployment
Project Deployment Actions Updated
hashdotdesign-tokens Ignored Ignored Preview Sep 1, 2026 3:30pm UTC

Request Review

@cursor

cursor Bot commented Aug 31, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
UI-only interaction, stacking, and layout fixes in Petrinaut forms and Storybook, with regression tests and no auth or data-path changes.

Overview
Fixes three ad-hoc scenario form UX defects in @hashintel/petrinaut.

The optimize bounds slab is usable again: the portalled overlay sets pointer-events: auto, nested Scale dropdowns portal into the slab via PortalContainerContext so outside-click dismiss does not fire, and the one-shot Min focus guard tracks the selected element instead of a boolean flag so re-renders during bound edits no longer steal focus.

Section caps :focus-within stacking at z-index: 1 (was 3) so nested sections no longer paint over a parent’s sticky header.

The experiment drawer’s Computed state preview is wrapped in a 320px max-height, scrollable, tinted panel so large initial markings do not push the rest of the drawer away.

Storybook portal decorators restore pointer-events: auto on portalled children so menus and selects work in stories. Tests and experiments docs are updated; patch changeset included.

Reviewed by Cursor Bugbot for commit b4cf794. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1990608. Configure here.

Comment thread libs/@hashintel/petrinaut/src/ui/components/ad-hoc-scenario-form/value-editor.tsx Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes interaction and layout defects in Petrinaut’s ad-hoc scenario forms and experiment drawer.

Changes:

  • Restores bounds-editor and portalled-layer interactions.
  • Corrects nested section stacking beneath sticky headers.
  • Bounds and documents computed-state preview scrolling.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.changeset/adhoc-form-editing-and-layout-fixes.md Records the patch release.
libs/@hashintel/petrinaut/.storybook/preview.tsx Makes Storybook portal children interactive.
libs/@hashintel/petrinaut/docs/experiments.md Documents bounded preview scrolling.
libs/@hashintel/petrinaut/src/ui/components/ad-hoc-scenario-form/ad-hoc-scenario-form.test.tsx Tests bounds interaction and focus retention.
libs/@hashintel/petrinaut/src/ui/components/ad-hoc-scenario-form/value-editor.tsx Fixes pointer handling, nested portals, and bound focus.
libs/@hashintel/petrinaut/src/ui/components/section.test.tsx Verifies section stacking tiers.
libs/@hashintel/petrinaut/src/ui/components/section.tsx Keeps focused sections below sticky headers.
libs/@hashintel/petrinaut/src/ui/views/Editor/panels/SimulateView/experiments/experiment-scenario-run.test.tsx Tests the bounded preview panel.
libs/@hashintel/petrinaut/src/ui/views/Editor/panels/SimulateView/experiments/experiment-scenario-run.tsx Adds the unified scrollable preview region.
libs/@hashintel/petrinaut/src/ui/views/Editor/panels/SimulateView/simulate-view.stories.tsx Restores portal interactions in Simulate stories.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@YannisZa

YannisZa commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Referencing of variables inside net parameters and initial states is not supported here. Also other functions inside expressions should not be visible. These will be addressed in a future PR.

@kube
kube added this pull request to the merge queue Sep 1, 2026
Base automatically changed from cf/fe-1517-scenario-creation-uses-the-ad-hoc-form to cf/fe-1490-ad-hoc-scenario-user-guide September 1, 2026 16:49
@github-actions github-actions Bot added the type/eng > backend Owned by the @backend team label Sep 1, 2026
Merged via the queue into main with commit 315cef1 Sep 1, 2026
84 of 110 checks passed
@kube
kube deleted the cf/fe-778-ad-hoc-form-editing-and-layout-fixes branch September 1, 2026 17:07
@hash-release hash-release Bot mentioned this pull request Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/apps > hash.design Affects the `hash.design` design site (app) area/infra Relates to version control, CI, CD or IaC (area) area/libs Relates to first-party libraries/crates/packages (area) type/eng > backend Owned by the @backend team type/eng > frontend Owned by the @frontend team

Development

Successfully merging this pull request may close these issues.

3 participants