Skip to content

Commit 5207dd0

Browse files
committed
(fix) Handle non-leaf parent in openChildWorkspace
When launchChildWorkspace is called from a non-leaf parent (e.g. a double-click race where the store updates synchronously but React hasn't re-rendered), openChildWorkspace now trims descendants above the parent and opens the child, instead of throwing. launchChildWorkspace is now async (Promise<void>) and prompts before trimming only when affected descendants have unsaved changes. Also adds a guard for stale parent references and updates tests/docs.
1 parent 0fe30d3 commit 5207dd0

7 files changed

Lines changed: 243 additions & 10 deletions

File tree

packages/framework/esm-framework/docs/functions/getRegisteredWorkspace2Names.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
> **getRegisteredWorkspace2Names**(): `string`[]
66
7-
Defined in: [packages/framework/esm-styleguide/src/workspaces2/workspace2.ts:600](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/workspaces2/workspace2.ts#L600)
7+
Defined in: [packages/framework/esm-styleguide/src/workspaces2/workspace2.ts:604](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/workspaces2/workspace2.ts#L604)
88

99
## Returns
1010

packages/framework/esm-framework/docs/functions/useWorkspace2Context.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
> **useWorkspace2Context**(): [`Workspace2DefinitionProps`](../interfaces/Workspace2DefinitionProps.md)\<`object`, `object`, `object`\>
66
7-
Defined in: [packages/framework/esm-styleguide/src/workspaces2/workspace2.ts:594](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/workspaces2/workspace2.ts#L594)
7+
Defined in: [packages/framework/esm-styleguide/src/workspaces2/workspace2.ts:598](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/workspaces2/workspace2.ts#L598)
88

99
Returns the react Context containing props passed into a workspace.
1010
This hook MUST be called inside a child of <Workspace2>

packages/framework/esm-framework/docs/interfaces/Workspace2DefinitionProps.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ a Promise that resolves to true if the workspace is closed, false otherwise.
130130

131131
### launchChildWorkspace()
132132

133-
> **launchChildWorkspace**\<`Props`\>(`workspaceName`, `workspaceProps?`): `void`
133+
> **launchChildWorkspace**\<`Props`\>(`workspaceName`, `workspaceProps?`): `Promise`\<`void`\>
134134
135135
Defined in: [packages/framework/esm-styleguide/src/workspaces2/workspace2.component.tsx:33](https://github.com/openmrs/openmrs-esm-core/blob/main/packages/framework/esm-styleguide/src/workspaces2/workspace2.component.tsx#L33)
136136

@@ -158,4 +158,4 @@ the window props or group props
158158

159159
#### Returns
160160

161-
`void`
161+
`Promise`\<`void`\>

packages/framework/esm-styleguide/src/workspaces2/active-workspace-window.component.tsx

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,32 @@ const ActiveWorkspace: React.FC<ActiveWorkspaceProps> = ({
101101
return false;
102102
}
103103
},
104-
launchChildWorkspace: (childWorkspaceName, childWorkspaceProps) => {
104+
launchChildWorkspace: async (childWorkspaceName, childWorkspaceProps) => {
105105
const parentWorkspaceName = openedWorkspace.workspaceName;
106+
const { openedWorkspaces } = openedWindow;
107+
const parentIndex = openedWorkspaces.findIndex((w) => w.workspaceName === parentWorkspaceName);
108+
if (parentIndex === -1) {
109+
return;
110+
}
111+
const isLeaf = parentIndex === openedWorkspaces.length - 1;
112+
113+
if (!isLeaf) {
114+
// There are workspaces above the parent that will be closed.
115+
// Prompt if any of them have unsaved changes.
116+
const workspacesAboveParent = openedWorkspaces.slice(parentIndex + 1);
117+
if (workspacesAboveParent.some((w) => w.hasUnsavedChanges)) {
118+
const okToClose = await promptForClosingWorkspaces({
119+
reason: 'CLOSE_WORKSPACE',
120+
explicit: true,
121+
windowName: openedWindow.windowName,
122+
workspaceName: openedWorkspaces[parentIndex + 1].workspaceName,
123+
});
124+
if (!okToClose) {
125+
return;
126+
}
127+
}
128+
}
129+
106130
openChildWorkspace(parentWorkspaceName, childWorkspaceName, childWorkspaceProps || {});
107131
},
108132
workspaceName: openedWorkspace.workspaceName,

packages/framework/esm-styleguide/src/workspaces2/workspace2.component.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export interface Workspace2DefinitionProps<
3030
* @param workspaceName
3131
* @param workspaceProps
3232
*/
33-
launchChildWorkspace<Props extends object>(workspaceName: string, workspaceProps?: Props): void;
33+
launchChildWorkspace<Props extends object>(workspaceName: string, workspaceProps?: Props): Promise<void>;
3434

3535
/**
3636
* closes the current workspace, along with its children.
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
import { describe, expect, it } from 'vitest';
2+
import type { WorkspaceStoreState2 } from '@openmrs/esm-extensions';
3+
import { workspace2StoreActions } from './workspace2';
4+
5+
function makeState(overrides: Partial<WorkspaceStoreState2> = {}): WorkspaceStoreState2 {
6+
return {
7+
registeredGroupsByName: {},
8+
registeredWindowsByName: {},
9+
registeredWorkspacesByName: {
10+
'parent-workspace': { name: 'parent-workspace', component: 'parent', window: 'test-window', moduleName: 'test' },
11+
'child-workspace': { name: 'child-workspace', component: 'child', window: 'test-window', moduleName: 'test' },
12+
'grandchild-workspace': {
13+
name: 'grandchild-workspace',
14+
component: 'grandchild',
15+
window: 'test-window',
16+
moduleName: 'test',
17+
},
18+
'other-window-workspace': {
19+
name: 'other-window-workspace',
20+
component: 'other',
21+
window: 'other-window',
22+
moduleName: 'test',
23+
},
24+
},
25+
openedGroup: null,
26+
openedWindows: [],
27+
isMostRecentlyOpenedWindowHidden: false,
28+
workspaceTitleByWorkspaceName: {},
29+
...overrides,
30+
};
31+
}
32+
33+
function makeOpenedWorkspace(name: string, hasUnsavedChanges = false) {
34+
return { workspaceName: name, props: {}, hasUnsavedChanges, uuid: `uuid-${name}` };
35+
}
36+
37+
describe('openChildWorkspace', () => {
38+
it('opens a child workspace from the leaf parent', () => {
39+
const state = makeState({
40+
openedWindows: [
41+
{
42+
windowName: 'test-window',
43+
openedWorkspaces: [makeOpenedWorkspace('parent-workspace')],
44+
props: null,
45+
maximized: false,
46+
},
47+
],
48+
});
49+
50+
const result = workspace2StoreActions.openChildWorkspace(state, 'parent-workspace', 'child-workspace', {
51+
foo: 'bar',
52+
});
53+
54+
expect(result.openedWindows[0].openedWorkspaces).toHaveLength(2);
55+
expect(result.openedWindows[0].openedWorkspaces[0].workspaceName).toBe('parent-workspace');
56+
expect(result.openedWindows[0].openedWorkspaces[1].workspaceName).toBe('child-workspace');
57+
expect(result.openedWindows[0].openedWorkspaces[1].props).toEqual({ foo: 'bar' });
58+
});
59+
60+
it('trims workspaces above the parent when parent is not the leaf (double-click race)', () => {
61+
const state = makeState({
62+
openedWindows: [
63+
{
64+
windowName: 'test-window',
65+
openedWorkspaces: [makeOpenedWorkspace('parent-workspace'), makeOpenedWorkspace('child-workspace')],
66+
props: null,
67+
maximized: false,
68+
},
69+
],
70+
});
71+
72+
// Simulates the second click in a double-click: parent tries to open the same child again
73+
const result = workspace2StoreActions.openChildWorkspace(state, 'parent-workspace', 'child-workspace', {
74+
newProps: true,
75+
});
76+
77+
expect(result.openedWindows[0].openedWorkspaces).toHaveLength(2);
78+
expect(result.openedWindows[0].openedWorkspaces[0].workspaceName).toBe('parent-workspace');
79+
expect(result.openedWindows[0].openedWorkspaces[1].workspaceName).toBe('child-workspace');
80+
expect(result.openedWindows[0].openedWorkspaces[1].props).toEqual({ newProps: true });
81+
});
82+
83+
it('trims grandchild when parent opens a new child', () => {
84+
const state = makeState({
85+
openedWindows: [
86+
{
87+
windowName: 'test-window',
88+
openedWorkspaces: [
89+
makeOpenedWorkspace('parent-workspace'),
90+
makeOpenedWorkspace('child-workspace'),
91+
makeOpenedWorkspace('grandchild-workspace'),
92+
],
93+
props: null,
94+
maximized: false,
95+
},
96+
],
97+
});
98+
99+
// Parent opens a new child — both child and grandchild should be trimmed
100+
const result = workspace2StoreActions.openChildWorkspace(state, 'parent-workspace', 'child-workspace', {});
101+
102+
expect(result.openedWindows[0].openedWorkspaces).toHaveLength(2);
103+
expect(result.openedWindows[0].openedWorkspaces[0].workspaceName).toBe('parent-workspace');
104+
expect(result.openedWindows[0].openedWorkspaces[1].workspaceName).toBe('child-workspace');
105+
});
106+
107+
it('throws when child workspace is not registered', () => {
108+
const state = makeState({
109+
openedWindows: [
110+
{
111+
windowName: 'test-window',
112+
openedWorkspaces: [makeOpenedWorkspace('parent-workspace')],
113+
props: null,
114+
maximized: false,
115+
},
116+
],
117+
});
118+
119+
expect(() =>
120+
workspace2StoreActions.openChildWorkspace(state, 'parent-workspace', 'nonexistent-workspace', {}),
121+
).toThrow('No workspace named "nonexistent-workspace" registered');
122+
});
123+
124+
it('throws when parent workspace is not registered', () => {
125+
const state = makeState({
126+
openedWindows: [
127+
{
128+
windowName: 'test-window',
129+
openedWorkspaces: [makeOpenedWorkspace('parent-workspace')],
130+
props: null,
131+
maximized: false,
132+
},
133+
],
134+
});
135+
136+
expect(() => workspace2StoreActions.openChildWorkspace(state, 'nonexistent-parent', 'child-workspace', {})).toThrow(
137+
'No workspace named "nonexistent-parent" registered',
138+
);
139+
});
140+
141+
it('throws when child belongs to a different window than parent', () => {
142+
const state = makeState({
143+
openedWindows: [
144+
{
145+
windowName: 'test-window',
146+
openedWorkspaces: [makeOpenedWorkspace('parent-workspace')],
147+
props: null,
148+
maximized: false,
149+
},
150+
],
151+
});
152+
153+
expect(() =>
154+
workspace2StoreActions.openChildWorkspace(state, 'parent-workspace', 'other-window-workspace', {}),
155+
).toThrow('does not belong to the same workspace window');
156+
});
157+
158+
it('throws when the window is not opened', () => {
159+
const state = makeState({
160+
openedWindows: [],
161+
});
162+
163+
expect(() => workspace2StoreActions.openChildWorkspace(state, 'parent-workspace', 'child-workspace', {})).toThrow(
164+
'window test-window is not opened',
165+
);
166+
});
167+
168+
it('throws when the parent workspace is not in the opened window', () => {
169+
const state = makeState({
170+
openedWindows: [
171+
{
172+
windowName: 'test-window',
173+
openedWorkspaces: [makeOpenedWorkspace('child-workspace')],
174+
props: null,
175+
maximized: false,
176+
},
177+
],
178+
});
179+
180+
expect(() =>
181+
workspace2StoreActions.openChildWorkspace(state, 'parent-workspace', 'grandchild-workspace', {}),
182+
).toThrow('parent is not opened within the workspace window');
183+
});
184+
185+
it('does not mutate the original state', () => {
186+
const originalWorkspaces = [makeOpenedWorkspace('parent-workspace'), makeOpenedWorkspace('child-workspace')];
187+
const state = makeState({
188+
openedWindows: [
189+
{
190+
windowName: 'test-window',
191+
openedWorkspaces: [...originalWorkspaces],
192+
props: null,
193+
maximized: false,
194+
},
195+
],
196+
});
197+
198+
workspace2StoreActions.openChildWorkspace(state, 'parent-workspace', 'child-workspace', {});
199+
200+
// Original state should be untouched
201+
expect(state.openedWindows[0].openedWorkspaces).toHaveLength(2);
202+
expect(state.openedWindows[0].openedWorkspaces[0].workspaceName).toBe('parent-workspace');
203+
expect(state.openedWindows[0].openedWorkspaces[1].workspaceName).toBe('child-workspace');
204+
});
205+
});

packages/framework/esm-styleguide/src/workspaces2/workspace2.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ export function promptForClosingWorkspaces(promptReason: PromptReason): Promise<
429429
});
430430
}
431431

432-
const workspace2StoreActions = {
432+
export const workspace2StoreActions = {
433433
setWindowMaximized(state: WorkspaceStoreState2, windowName: string, maximized: boolean) {
434434
const openedWindowIndex = state.openedWindows.findIndex((a) => a.windowName === windowName);
435435
const openedWindows = [...state.openedWindows];
@@ -522,18 +522,22 @@ const workspace2StoreActions = {
522522
}
523523
const openedWindow = state.openedWindows[openedWindowIndex];
524524
const { openedWorkspaces } = openedWindow;
525-
if (openedWorkspaces[openedWorkspaces.length - 1].workspaceName !== parentWorkspaceName) {
525+
const parentIndex = openedWorkspaces.findIndex((w) => w.workspaceName === parentWorkspaceName);
526+
if (parentIndex === -1) {
526527
throw new Error(
527-
`Cannot open child workspace ${childWorkspaceName} from parent workspace ${parentWorkspaceName} as the parent is not the most recently opened workspace within the workspace window`,
528+
`Cannot open child workspace ${childWorkspaceName} from parent workspace ${parentWorkspaceName} as the parent is not opened within the workspace window`,
528529
);
529530
}
530531

532+
// Close any workspaces above the parent (analogous to closeWorkspace's slice behavior)
533+
const trimmedWorkspaces = openedWorkspaces.slice(0, parentIndex + 1);
534+
531535
return {
532536
openedWindows: state.openedWindows.map((w, i) => {
533537
if (i == openedWindowIndex) {
534538
return {
535539
...w,
536-
openedWorkspaces: [...w.openedWorkspaces, newOpenedWorkspace(childWorkspaceName, childWorkspaceProps)],
540+
openedWorkspaces: [...trimmedWorkspaces, newOpenedWorkspace(childWorkspaceName, childWorkspaceProps)],
537541
};
538542
} else {
539543
return w;

0 commit comments

Comments
 (0)