Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .changeset/popout-widget-hide-restore.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@itwin/appui-react": patch
---

Fixed a popped-out (child window) widget docking back to its panel instead of remaining popped out after it was hidden (`WidgetState.Hidden`) and then shown again.

Also enforced a minimum popout window size (200x200), matching the existing minimum used for floating widgets, to prevent popout windows from progressively shrinking to an unusably small size across repeated pop-out/dock cycles.
34 changes: 34 additions & 0 deletions docs/storybook/src/widget/PopoutRestore.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import type { Meta, StoryObj } from "@storybook/react-vite";
import { PopoutRestoreStory } from "./PopoutRestore";
import { AppUiDecorator } from "../Decorators";
import { Page } from "../AppUiStory";

const meta = {
title: "Widget/PopoutRestore",
component: PopoutRestoreStory,
tags: ["autodocs"],
decorators: [AppUiDecorator],
parameters: {
docs: {
page: () => <Page />,
},
},
} satisfies Meta<typeof PopoutRestoreStory>;

export default meta;
type Story = StoryObj<typeof meta>;

/** Steps to reproduce AB#2024472:
* 1. Pop the widget out (drag its tab out of the panel, or use the tab's
* "..." menu and click "Pop out active widget tab").
* 2. Click "Hide widget".
* 3. Click "Show widget".
*
* Expected: the widget re-appears in its popout window.
* Before the fix it would instead dock back into the left panel.
*/
export const Default: Story = {};
90 changes: 90 additions & 0 deletions docs/storybook/src/widget/PopoutRestore.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import React from "react";
import {
StagePanelState,
UiItemsProvider,
WidgetState,
useSpecificWidgetDef,
} from "@itwin/appui-react";
import { Button } from "@itwin/itwinui-react";
import { AppUiStory } from "../AppUiStory";
import { createFrontstage, createWidget } from "../Utils";

/** Demonstrates that a popped-out widget remains popped out after being hidden
* (e.g. because it has no content to display) and then shown again.
*
* The hide/show controls live outside of the widget (as `AppUiStory` children)
* since hiding the widget unmounts its content, which would otherwise take the
* "Show widget" button down with it.
*
* To reproduce: pop the widget out (drag its tab out, or use the "..." menu),
* then click "Hide widget" and "Show widget". Before the fix (AB#2024472) the
* widget would dock back into the panel instead of staying popped out.
*/
export function PopoutRestoreStory() {
const provider = {
id: "widgets",
getWidgets: () => [
createWidget(1, {
canPopout: true,
content: <div style={{ padding: "0.5em" }}>Widget content</div>,
}),
],
} satisfies UiItemsProvider;
return (
<AppUiStory
itemProviders={[provider]}
frontstages={[
createFrontstage({
leftPanelProps: {
defaultState: StagePanelState.Open,
},
}),
]}
>
<WidgetVisibilityControls />
</AppUiStory>
);
}

function WidgetVisibilityControls() {
const widgetDef = useSpecificWidgetDef("w1");
const [visible, setVisible] = React.useState(true);

const hide = () => {
setVisible(false);
widgetDef?.setWidgetState(WidgetState.Hidden);
};
const show = () => {
setVisible(true);
widgetDef?.setWidgetState(WidgetState.Open);
};

return (
<div
style={{
position: "absolute",
bottom: 0,
left: 0,
zIndex: 1000,
padding: "0.5em",
display: "flex",
flexDirection: "column",
gap: "0.5em",
}}
>
<div>Widget state: {visible ? "Visible" : "Hidden"}</div>
<div style={{ display: "flex", gap: "0.5em" }}>
<Button onClick={hide} size="small">
Hide widget
</Button>
<Button onClick={show} styleType="cta" size="small">
Show widget
</Button>
</div>
</div>
);
}
11 changes: 7 additions & 4 deletions ui/appui-react/src/appui-react/frontstage/FrontstageDef.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import {
FRONTSTAGE_SETTINGS_NAMESPACE,
getFrontstageStateSettingName,
} from "../widget-panels/Frontstage.js";
import { MIN_POPOUT_WINDOW_SIZE } from "../layout/state/WidgetRestoreState.js";

/** FrontstageDef class provides an API for a Frontstage.
* @public
Expand Down Expand Up @@ -746,8 +747,8 @@ export class FrontstageDef {
const popoutWidget = state.popoutWidgets.byId[location.popoutWidgetId];
const bounds = Rectangle.create(popoutWidget.bounds);
const position: ChildWindowLocationProps = {
width: bounds.getWidth(),
height: bounds.getHeight(),
width: Math.max(bounds.getWidth(), MIN_POPOUT_WINDOW_SIZE.width),
height: Math.max(bounds.getHeight(), MIN_POPOUT_WINDOW_SIZE.height),
left: bounds.left,
top: bounds.top,
};
Expand All @@ -764,9 +765,11 @@ export class FrontstageDef {
// Use outer size if available to avoid inner size + browser zoom issues: https://github.com/iTwin/appui/issues/563
const savedTab = state.savedTabs.byId[tabId];
if (childWindow && savedTab?.popout?.size) {
// Enforce a minimum size so a popout window that shrunk over repeated pop-out cycles
// can never become invisible or unusable.
childWindow.resizeTo(
savedTab.popout.size.width,
savedTab.popout.size.height
Math.max(savedTab.popout.size.width, MIN_POPOUT_WINDOW_SIZE.width),
Math.max(savedTab.popout.size.height, MIN_POPOUT_WINDOW_SIZE.height)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import {
import { getUniqueId } from "../base/NineZone.js";
import {
isPanelWidgetRestoreState,
MIN_POPOUT_WINDOW_SIZE,
type PanelWidgetRestoreState,
} from "./WidgetRestoreState.js";
import {
Expand Down Expand Up @@ -670,6 +671,25 @@ export function NineZoneStateReducer(
if (size) preferredBounds = preferredBounds.setSize(size);
if (position) preferredBounds = preferredBounds.setPosition(position);

// Enforce a minimum size so a popout window can never shrink to an invisible or
// unusable size across repeated pop-out cycles (see AB#2024472). Matches the
// minimum size used for floating widgets (see `minWidth`/`minHeight` in `Widget.tsx`).
if (
preferredBounds.getWidth() < MIN_POPOUT_WINDOW_SIZE.width ||
preferredBounds.getHeight() < MIN_POPOUT_WINDOW_SIZE.height
) {
preferredBounds = preferredBounds.setSize({
width: Math.max(
preferredBounds.getWidth(),
MIN_POPOUT_WINDOW_SIZE.width
),
height: Math.max(
preferredBounds.getHeight(),
MIN_POPOUT_WINDOW_SIZE.height
),
});
}

const popoutWidgetId = getUniqueId();
let home: PopoutWidgetState["home"] | undefined;
if (location && isPanelTabLocation(location)) {
Expand Down Expand Up @@ -1032,6 +1052,15 @@ function hideTab(state: NineZoneState, id: TabState["id"]) {
floatingWidget,
};
});
} else if (isPopoutTabLocation(location)) {
const popoutWidget = state.popoutWidgets.byId[widgetId];
state = updateSavedTabState(state, id, (draft) => {
draft.home = {
widgetId,
tabIndex,
popoutWidget,
};
});
} else if (isPanelTabLocation(location)) {
const side = location.side;
const widgetIndex = state.panels[side].widgets.indexOf(widgetId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@

import type { XAndY } from "@itwin/core-geometry";
import type { TabState } from "./TabState.js";
import type { WidgetRestoreState } from "./WidgetRestoreState.js";
import type { TabRestoreState } from "./WidgetRestoreState.js";
import type { SizeProps } from "../../utils/SizeProps.js";

/** @internal */
export type TabHomeState = WidgetRestoreState & {
export type TabHomeState = TabRestoreState & {
readonly tabIndex: number;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,31 @@
*/

import type { PanelSide } from "../widget-panels/PanelTypes.js";
import type { FloatingWidgetState, WidgetState } from "./WidgetState.js";
import type { SizeProps } from "../../utils/SizeProps.js";
import type {
FloatingWidgetState,
PopoutWidgetState,
WidgetState,
} from "./WidgetState.js";

/** Minimum size enforced for a popped-out widget window. */
export const MIN_POPOUT_WINDOW_SIZE: SizeProps = {
width: 200,
height: 200,
};

/** @internal */
export interface FloatingWidgetRestoreState {
widgetId: WidgetState["id"];
floatingWidget: FloatingWidgetState;
}

/** @internal */
export interface PopoutWidgetRestoreState {
widgetId: WidgetState["id"];
popoutWidget: PopoutWidgetState;
}

/** @internal */
export interface PanelWidgetRestoreState {
widgetId: WidgetState["id"];
Expand All @@ -27,16 +44,29 @@ export type WidgetRestoreState =
| FloatingWidgetRestoreState
| PanelWidgetRestoreState;

/** Restore state for a tab, which (unlike {@link WidgetRestoreState}) can also
* restore into a popout window.
* @internal
*/
export type TabRestoreState = WidgetRestoreState | PopoutWidgetRestoreState;

/** @internal */
export function isFloatingWidgetRestoreState(
state: WidgetRestoreState
state: TabRestoreState
): state is FloatingWidgetRestoreState {
return "floatingWidget" in state;
}
Comment thread
arome marked this conversation as resolved.

/** @internal */
export function isPopoutWidgetRestoreState(
state: TabRestoreState
): state is PopoutWidgetRestoreState {
return "popoutWidget" in state;
}

/** @internal */
export function isPanelWidgetRestoreState(
state: WidgetRestoreState
state: TabRestoreState
): state is PanelWidgetRestoreState {
return "side" in state;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,18 @@ import type { SavedTabState, TabHomeState } from "../SavedTabState.js";
import { getTabLocation } from "../TabLocation.js";
import {
addFloatingWidget,
addPopoutWidget,
assertWidgetState,
getWidgetState,
removeWidget,
setWidgetActiveTabId,
updateWidgetState,
} from "./WidgetStateHelpers.js";
import type { WidgetState } from "../WidgetState.js";
import { isFloatingWidgetRestoreState } from "../WidgetRestoreState.js";
import {
isFloatingWidgetRestoreState,
isPopoutWidgetRestoreState,
} from "../WidgetRestoreState.js";
import { Rectangle } from "@itwin/core-react/internal";
import { getUniqueId } from "../../base/NineZone.js";
import { insertPanelWidget } from "./PanelStateHelpers.js";
Expand Down Expand Up @@ -231,6 +235,13 @@ export function addRemovedTab(
});
}

// Add to a popout widget (i.e. re-open its child window).
if (isPopoutWidgetRestoreState(home)) {
return addPopoutWidget(state, widgetId, [tabId], {
...home.popoutWidget,
});
}

// Add to a panel section.
const panel = state.panels[home.side];

Expand Down
38 changes: 38 additions & 0 deletions ui/appui-react/src/test/frontstage/FrontstageDef.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,44 @@ describe("FrontstageDef", () => {
);
expect(spy).toHaveBeenCalledOnce();
});

it("should enforce a minimum popout window size (AB#2024472)", async () => {
const frontstageDef = new FrontstageDef();
await frontstageDef.initializeFromConfig({
...defaultFrontstageConfig,
leftPanel: {
sections: {
start: [
{
id: "t1",
},
],
},
},
});
initializeNineZoneState(frontstageDef);

const spy = vi.spyOn(window, "open").mockReturnValue({
addEventListener: () => {},
removeEventListener: () => {},
} as unknown as Window);
frontstageDef.popoutWidget("t1", undefined, {
width: 10,
height: 10,
});

spy.mockReset();
const popoutWidgets = frontstageDef.nineZoneState!.popoutWidgets;
const popoutWidget = popoutWidgets.byId[popoutWidgets.allIds[0]];
frontstageDef.openPopoutWidgetContainer(
popoutWidget.id,
frontstageDef.nineZoneState
);
expect(spy).toHaveBeenCalledOnce();
const features = spy.mock.calls[0][2] as string;
expect(features).to.include("width=200");
expect(features).to.include("height=200");
});
});

describe("dockWidgetContainer", () => {
Expand Down
Loading
Loading