Flow Graph Editor: Port to ModularTool + Fluent

.azure-pipelines/ci-graph-tools.yml

@@ -144,7 +144,7 @@ jobs:
 
           - script: |
                 npx playwright install --with-deps
-                npx playwright test --project=graphTools
+                npx playwright test --project=graphTools --project=flowGraphEditor
             displayName: "Test tools"
             env:
                 CI: "true"

.github/skills/porting-tools-to-fluent/SKILL.md

@@ -100,7 +100,15 @@ MakeModularTool({
 - `SettingsStore` (persisted user preferences)
 - `ThemeService` + optional `ThemeSelectorService`
 - `ShellService` (layout: central content, side panes, toolbars)
-- `ToastProvider` for toast notifications
+- `ToastProvider` + `IToastService` for toast notifications (consume `ToastServiceIdentity` — do not roll your own container)
+- `IDialogService` (consume `DialogServiceIdentity`) for modal alert/confirm dialogs — replaces ad-hoc `MessageDialog`
+
+### Cross-window / popup hosting
+
+`MakeModularTool` derives `targetDocument` from `containerElement.ownerDocument`. If your tool's entry function (e.g. `Show(options)`) hosts the editor in a popup window, just pass the popup body as `containerElement` — Fluent/Griffel/`Theme` plumb cross-window automatically.
+
+- For a fully-Fluent popup, use `OpenPopupWindow` from `shared-ui-components/fluent/hoc/popupWindow`.
+- If part of your tool still ships traditional CSS/SCSS (e.g. `shared-ui-components/nodeGraphSystem/`'s graph canvas), keep the legacy `CreatePopup` from `shared-ui-components/popupHelper` — it copies stylesheets into the popup. Fluent and `CreatePopup` coexist fine.
 
 ---
 
@@ -477,3 +485,97 @@ For `satisfies` clauses in const option arrays, use `satisfies DropdownOption<st
 8. **Switch hooks** — use `useObservableState` from `shared-ui-components/modularTool/` instead of local hooks
 9. **Delete obsolete files** — remove SCSS, FA wrappers, legacy components
 10. **Build & verify** — ensure clean build and correct rendering in both themes
+
+---
+
+## 11. Patterns for Large Editor Ports
+
+Distilled from the Flow Graph Editor port (`packages/tools/flowGraphEditor/`). See its `port-to-fluent.md` for the full plan.
+
+### Phased execution
+
+For tools too large to port in one go (NME-class), use phases that each leave the build green:
+
+1. **Bootstrap & shell** — add Fluent deps, create a `GlobalState` service, wrap the existing class component as a single `addCentralContent` passthrough, switch entry point to `MakeModularTool`.
+2. **Decompose layout** — extract each pane into its own service that still wraps the _legacy_ component. Delete `SplitContainer`/`Splitter` once the shell owns the layout.
+3. **Port surrounding components** — rewrite each pane/dialog/toolbar to Fluent + `makeStyles`. One component at a time.
+4. **Port property panels** — replace `shared-ui-components/lines/*` with Fluent property-line HOCs.
+5. **Cleanup** — delete local `sharedComponents/`, all `.scss`, obsolete `package.json` devDeps; run lint/format/build/e2e.
+
+### `MakeXService(options)` factory pattern
+
+When a service needs instance-specific inputs from the tool's entry function (e.g. `Show(options)`), export a factory rather than a static `ServiceDefinition`:
+
+```ts
+export function MakeGlobalStateService(options: IMyToolOptions, hostElement: HTMLElement): ServiceDefinition<[IGlobalStateService], []> {
+    return {
+        friendlyName: "Global State Service",
+        produces: [GlobalStateServiceIdentity],
+        factory: () => {
+            const globalState = new GlobalState(options.scene);
+            // wire options into globalState...
+            return { globalState, dispose: () => { /* cleanup */ } };
+        },
+    };
+}
+
+// In Show(options):
+MakeModularTool({
+    serviceDefinitions: [MakeGlobalStateService(options, hostElement), CentralServiceDefinition, ...],
+    /* ... */
+});
+```
+
+Prefer this over a `parentContainer` for instance-scoped data.
+
+### Bridge services for legacy observables
+
+When the existing codebase uses `globalState.on*Observable` to trigger UI (toasts, dialogs, etc.), don't rewrite all call sites. Add a small "bridge" service that consumes the framework service and forwards observable events:
+
+```ts
+export const ToastBridgeServiceDefinition: ServiceDefinition<[], [IGlobalStateService, IToastService]> = {
+    friendlyName: "Toast Bridge Service",
+    consumes: [GlobalStateServiceIdentity, ToastServiceIdentity],
+    factory: (gs, toast) => {
+        const observer = gs.globalState.onToastNotification.add((d) => toast.showToast(d.message, { intent: d.severity }));
+        return { dispose: () => observer?.remove() };
+    },
+};
+```
+
+Same pattern works for `DialogBridge` → `IDialogService`, etc. Delete the legacy renderer (`ToastContainerComponent`, `MessageDialog`) once bridged.
+
+### `ToolContext` override per pane
+
+Dense property panels often want `size: "small"` independent of the user's tool-wide setting. Override `ToolContext` _inside_ the pane's content function (spread the parent first so other fields are inherited):
+
+```tsx
+content: () => {
+    const parent = useContext(ToolContext);
+    const ctx = useMemo(() => ({ ...parent, size: "small" as const }), [parent]);
+    return <ToolContext.Provider value={ctx}><PropertyTab .../></ToolContext.Provider>;
+},
+```
+
+### Toolbar hosts global actions
+
+Convert global buttons (Help, How-to-use, documentation links) to `shellService.addToolbarItem({ horizontalLocation: "right", verticalLocation: "bottom" })`. Existing top-of-canvas control bars (play/pause/undo/redo) fit naturally in `{ horizontalLocation: "left", verticalLocation: "top" }`, removing the need for a custom bar above the canvas.
+
+Buttons typically just notify an existing `globalState.on*Requested` observable so the dialog overlay logic stays where it is.
+
+### Side panes — `ExtensibleAccordion`, title + icon
+
+- Use `ExtensibleAccordion` (from `shared-ui-components/modularTool/components/extensibleAccordion`) for node lists and property tabs — gives filtering and pinning for free.
+- Set `title` and `icon` on `addSidePane` so the shell renders the tool name/logo in the pane header (mirrors `viewer-configurator/configuratorService.tsx`). Use Fluent icons first; `createFluentIcon` only for Babylon-specific glyphs (logo, port markers).
+
+### `FileUploadLine` for file inputs
+
+Replace local `FileButtonLineComponent`-style components with `FileUploadLine` from `shared-ui-components/fluent/hoc/fileUploadLine`. Its callback receives a `FileList` — read `files[0]` if you previously took a single `File`.
+
+### Shared `Dialog` primitive
+
+Use `Dialog` from `shared-ui-components/fluent/primitives/dialog` (open + title + children + actions) for ad-hoc dialogs instead of composing `FluentDialog` + `DialogSurface` + `DialogBody` directly.
+
+### Keep `GraphCanvasComponent` out of scope
+
+The shared `shared-ui-components/nodeGraphSystem/` graph canvas is consumed by every node-graph editor and still ships SCSS. Don't try to port it during a tool-level Fluent migration — only port the surrounding shell, panes, dialogs, and property panels.

packages/dev/sharedUiComponents/src/fluent/hoc/childWindow.tsx

@@ -1,30 +1,8 @@
 import { type GriffelRenderer, createDOMRenderer, FluentProvider, Portal, RendererProvider } from "@fluentui/react-components";
 import { type FunctionComponent, type PropsWithChildren, type Ref, useCallback, useEffect, useImperativeHandle, useState } from "react";
 
-import { Logger } from "core/Misc/logger";
 import { ToastProvider } from "../primitives/toast";
-
-function ToFeaturesString(options: ChildWindowOptions) {
-    const { defaultWidth, defaultHeight, defaultLeft, defaultTop } = options;
-
-    const features: { key: string; value: string }[] = [];
-
-    if (defaultWidth !== undefined) {
-        features.push({ key: "width", value: defaultWidth.toString() });
-    }
-    if (defaultHeight !== undefined) {
-        features.push({ key: "height", value: defaultHeight.toString() });
-    }
-    if (defaultLeft !== undefined) {
-        features.push({ key: "left", value: defaultLeft.toString() });
-    }
-    if (defaultTop !== undefined) {
-        features.push({ key: "top", value: defaultTop.toString() });
-    }
-    features.push({ key: "location", value: "no" });
-
-    return features.map((feature) => `${feature.key}=${feature.value}`).join(",");
-}
+import { OpenPopupWindow, type PopupWindowHandle } from "./popupWindow";
 
 export type ChildWindowOptions = {
     /**
@@ -99,150 +77,76 @@ export const ChildWindow: FunctionComponent<PropsWithChildren<ChildWindowProps>>
     const { id, children, onOpenChange, imperativeRef: imperativeRef } = props;
 
     const [windowState, setWindowState] = useState<{ mountNode: HTMLElement; renderer: GriffelRenderer }>();
-    const [childWindow, setChildWindow] = useState<Window>();
-
-    const storageKey = id ? `Babylon/Settings/ChildWindow/${id}/Bounds` : null;
+    const [popupHandle, setPopupHandle] = useState<PopupWindowHandle>();
 
     // This function is just for creating the child window itself. It is a function because
     // it must be called synchronously in response to a user interaction (e.g. button click),
     // otherwise the browser will block it as a scripted popup.
     const createWindow = useCallback(
         (options: ChildWindowOptions = {}) => {
-            if (storageKey) {
-                // If we are persisting window bounds, but the window is already open, just use the existing bounds.
-                // Otherwise, try to load bounds from storage.
-                if (childWindow) {
-                    options.defaultLeft = childWindow.screenX;
-                    options.defaultTop = childWindow.screenY;
-                    options.defaultWidth = childWindow.innerWidth;
-                    options.defaultHeight = childWindow.innerHeight;
-                } else {
-                    const savedBounds = localStorage.getItem(storageKey);
-                    if (savedBounds) {
-                        try {
-                            const bounds = JSON.parse(savedBounds);
-                            options.defaultLeft = bounds.left;
-                            options.defaultTop = bounds.top;
-                            options.defaultWidth = bounds.width;
-                            options.defaultHeight = bounds.height;
-                        } catch {
-                            Logger.Warn(`Could not parse saved bounds for child window with key ${storageKey}`);
-                        }
-                    }
-                }
-            }
-
-            // Half width by default.
-            if (!options.defaultWidth) {
-                options.defaultWidth = window.innerWidth * (2 / 3);
-            }
-            // Half height by default.
-            if (!options.defaultHeight) {
-                options.defaultHeight = window.innerHeight * (2 / 3);
-            }
-            // Horizontally centered by default.
-            if (!options.defaultLeft) {
-                options.defaultLeft = window.screenX + (window.innerWidth - options.defaultWidth) * (2 / 3);
-            }
-            // Vertically centered by default.
-            if (!options.defaultTop) {
-                options.defaultTop = window.screenY + (window.innerHeight - options.defaultHeight) * (2 / 3);
-            }
-
-            // Try to create the child window (can be null if popups are blocked).
-            const newChildWindow = window.open("", "", ToFeaturesString(options));
-            if (newChildWindow) {
-                // Set the title if provided.
-                newChildWindow.document.title = options.title ?? id ?? "";
+            const handle = OpenPopupWindow({
+                id,
+                title: options.title ?? id,
+                defaultWidth: options.defaultWidth,
+                defaultHeight: options.defaultHeight,
+                defaultLeft: options.defaultLeft,
+                defaultTop: options.defaultTop,
+                onClose: () => {
+                    // Triggered when the popup is closed for any reason (user dismissal, parent unload,
+                    // or programmatic dispose). Clear the popup handle so the effect cleanup runs and
+                    // `onOpenChange(false)` is propagated up the tree (which lets a parent shell re-dock
+                    // a previously-undocked pane). teardown() is idempotent so calling dispose() again
+                    // from the effect cleanup is a safe no-op.
+                    setPopupHandle(undefined);
+                },
+            });
 
-                // Set the child window state.
-                setChildWindow((current) => {
-                    // But first close any existing child window.
-                    current?.close();
-                    return newChildWindow;
+            if (handle) {
+                setPopupHandle((current) => {
+                    // Close any existing child window before adopting the new one.
+                    current?.dispose();
+                    return handle;
                 });
             }
         },
-        [childWindow, storageKey]
+        [id]
     );
 
     useImperativeHandle(imperativeRef, () => {
         return {
             open: createWindow,
-            close: () => setChildWindow(undefined),
+            close: () => {
+                setPopupHandle((current) => {
+                    current?.dispose();
+                    return undefined;
+                });
+            },
         };
     }, [createWindow]);
 
-    // This side effect runs any time the child window instance changes. It does the rest of the child window
+    // This side effect runs any time the popup handle changes. It does the rest of the child window
     // setup work, including creating resources and state needed to properly render the content of the child window.
     useEffect(() => {
-        const disposeActions: (() => void)[] = [];
-
-        if (childWindow) {
-            const body = childWindow.document.body;
-            body.style.width = "100%";
-            body.style.height = "100%";
-            body.style.margin = "0";
-            body.style.padding = "0";
-            body.style.display = "flex";
-            body.style.overflow = "hidden";
-
-            // Setup the window state, including creating a Fluent/Griffel "renderer" for managing runtime styles/classes in the child window.
-            setWindowState({ mountNode: body, renderer: createDOMRenderer(childWindow.document) });
-            onOpenChange?.(true);
-
-            // Track the most recently observed window bounds. In some browsers (e.g. Firefox), accessing
-            // properties like screenX on a closed window throws, so we cache the last known good values
-            // to use as a fallback when the dispose runs after the window has already been closed.
-            const getBounds = () => ({
-                left: childWindow.screenX,
-                top: childWindow.screenY,
-                width: childWindow.innerWidth,
-                height: childWindow.innerHeight,
-            });
-            let lastBounds = getBounds();
-
-            // When the child window is closed for any reason, transition back to a closed state.
-            const onChildWindowUnload = () => {
-                setWindowState(undefined);
-                setChildWindow(undefined);
-                onOpenChange?.(false);
-            };
-            childWindow.addEventListener("unload", onChildWindowUnload, { once: true });
-            disposeActions.push(() => childWindow.removeEventListener("unload", onChildWindowUnload));
-
-            // Capture bounds before the window is unloaded, while its properties are still safe to read.
-            const onChildWindowBeforeUnload = () => {
-                lastBounds = getBounds();
-            };
-            childWindow.addEventListener("beforeunload", onChildWindowBeforeUnload);
-            disposeActions.push(() => childWindow.removeEventListener("beforeunload", onChildWindowBeforeUnload));
-
-            // If the main window closes, close any open child windows as well (don't leave them orphaned).
-            const onParentWindowUnload = () => {
-                childWindow.close();
-            };
-            window.addEventListener("unload", onParentWindowUnload, { once: true });
-            disposeActions.push(() => window.removeEventListener("unload", onParentWindowUnload));
+        if (!popupHandle) {
+            return undefined;
+        }
 
-            // On dispose, close the child window.
-            disposeActions.push(() => childWindow.close());
+        const popupDocument = popupHandle.popupWindow.document;
 
-            // On dispose, save the window bounds.
-            disposeActions.push(() => {
-                if (storageKey) {
-                    if (!childWindow.closed) {
-                        lastBounds = getBounds();
-                    }
-                    localStorage.setItem(storageKey, JSON.stringify(lastBounds));
-                }
-            });
-        }
+        // Use the popup's hostElement as the React mount point. OpenPopupWindow configures it as a
+        // flex column that fills the popup body, so the FluentProvider (also a flex column with
+        // flex-grow: 1) inside it can fill the available space.
+        setWindowState({ mountNode: popupHandle.hostElement, renderer: createDOMRenderer(popupDocument) });
+        onOpenChange?.(true);
 
         return () => {
-            disposeActions.reverse().forEach((dispose) => dispose());
+            // Tear down the popup. The cached handle's dispose() handles bounds saving and
+            // listener cleanup; React state is reset so the Portal/Provider tree unmounts.
+            popupHandle.dispose();
+            setWindowState(undefined);
+            onOpenChange?.(false);
         };
-    }, [childWindow]);
+    }, [popupHandle]);
 
     if (!windowState) {
         return null;