Skip to content

Commit a2d4b18

Browse files
texodustexodus
authored
Merge pull request #3000 from finos/workspace-toolbar-fix
`<perspective-workspace>` toolbar fix
2 parents 425b6eb + 33de0a8 commit a2d4b18

21 files changed

Lines changed: 342 additions & 55 deletions

File tree

‎packages/perspective-viewer-datagrid/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
"@finos/perspective": "workspace:^",
3636
"@finos/perspective-viewer": "workspace:^",
3737
"chroma-js": "^1.3.4",
38-
"regular-table": "=0.6.6"
38+
"regular-table": "=0.6.7"
3939
},
4040
"devDependencies": {
4141
"@prospective.co/procss": "^0.1.16",

‎packages/perspective-viewer-datagrid/src/less/toolbar.less‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
:host {
1414
position: relative;
1515
display: block;
16+
height: 100%;
1617
}
1718

1819
:host #container {
@@ -29,19 +30,18 @@
2930

3031
:host #toolbar {
3132
display: flex;
32-
align-items: center;
33-
height: 36px;
33+
align-items: stretch;
34+
height: 100%;
3435
}
3536

3637
:host #toolbar .hover-target {
3738
margin: 0;
3839
display: inline-flex;
3940
align-items: center;
40-
height: 48px;
4141
cursor: pointer;
4242

4343
&:hover {
44-
outline: 4px solid var(--icon--color);
44+
box-shadow: -4px 0 0 var(--icon--color), 4px 0 0 var(--icon--color);
4545
background-color: var(--icon--color);
4646
}
4747
}
@@ -162,6 +162,7 @@
162162
padding: 0;
163163
}
164164

165+
.hover-target:focus-within .button,
165166
.hover-target:hover .button {
166167
position: relative;
167168
background-color: var(--icon--color);
@@ -172,11 +173,13 @@
172173
cursor: pointer;
173174
}
174175

176+
.hover-target:focus-within .button:before,
175177
.hover-target:hover .button:before {
176178
background-color: var(--plugin--background);
177179
}
178180

179181
// The label.
182+
.hover-target:focus-within .button > span,
180183
.hover-target:hover .button > span {
181184
display: block;
182185
position: absolute;

‎packages/perspective-workspace/src/themes/pro-dark.less‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ perspective-workspace-menu {
123123
--select-arrow-light--background-image
124124
);
125125

126+
--select-arrow--hover--background-image: var(
127+
--select-arrow-dark--background-image
128+
);
129+
126130
// Syntax
127131
--code-editor-symbol--color: white;
128132
--code-editor-literal--color: #7dc3f0;

‎packages/perspective-workspace/src/ts/perspective-workspace.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,15 @@ export class HTMLPerspectiveWorkspaceElement extends HTMLElement {
141141
await this.workspace!.restore(layout);
142142
}
143143

144+
async clear() {
145+
await this.restore({
146+
sizes: [],
147+
master: { sizes: [] },
148+
detail: { sizes: [] },
149+
viewers: {},
150+
});
151+
}
152+
144153
/**
145154
* Await all asynchronous tasks for all viewers in this workspace. This is
146155
* useful to make sure asynchonous side effects of synchronous methods calls

‎packages/perspective-workspace/src/ts/workspace/widget.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ export class PerspectiveViewerWidget extends Widget {
8181
table: this.viewer.getAttribute("table"),
8282
};
8383

84-
delete config["theme"];
8584
delete config["settings"];
8685
return config;
8786
}
@@ -97,6 +96,6 @@ export class PerspectiveViewerWidget extends Widget {
9796
this.viewer.parentElement.removeChild(this.viewer);
9897
}
9998

100-
await this.viewer.delete().then(() => this.viewer.free());
99+
await this.viewer.delete();
101100
}
102101
}

‎packages/perspective-workspace/src/ts/workspace/workspace.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ export class PerspectiveWorkspace extends SplitPanel {
237237
// Using ES generators as context managers ..
238238
for (const viewers of this._capture_viewers()) {
239239
for (const widgets of this._capture_widgets()) {
240+
for (const v of viewers) {
241+
v.removeAttribute("class");
242+
}
243+
240244
const callback = this._restore_callback.bind(
241245
this,
242246
viewer_configs,
@@ -965,8 +969,15 @@ export class PerspectiveWorkspace extends SplitPanel {
965969

966970
const updated = async (event: CustomEvent) => {
967971
this.workspaceUpdated();
968-
widget.title.label = event.detail.title;
969-
widget._is_pivoted = event.detail.group_by?.length > 0;
972+
// Sometimes plugins or other external code fires this event and
973+
// does not populate this field!
974+
const config =
975+
typeof event.detail === "undefined"
976+
? await widget.viewer.save()
977+
: event.detail;
978+
979+
widget.title.label = config.title;
980+
widget._is_pivoted = config.group_by?.length > 0;
970981
};
971982

972983
widget.node.addEventListener("contextmenu", contextMenu);

‎packages/perspective-workspace/test/js/restore.spec.js‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,136 @@ function tests(context, compare) {
116116
`${context}-restore-workspace-is-symmetric-with-addviewer.txt`
117117
);
118118
});
119+
120+
test.describe("Toggle master/detail", () => {
121+
test("restore a blank view removes master panel", async ({ page }) => {
122+
const config = {
123+
detail: {
124+
main: {
125+
type: "tab-area",
126+
widgets: ["PERSPECTIVE_GENERATED_ID_1"],
127+
currentIndex: 0,
128+
},
129+
},
130+
master: { widgets: ["PERSPECTIVE_GENERATED_ID_0"], sizes: [1] },
131+
viewers: {
132+
PERSPECTIVE_GENERATED_ID_0: {
133+
table: "superstore",
134+
},
135+
PERSPECTIVE_GENERATED_ID_1: {
136+
table: "superstore",
137+
},
138+
},
139+
};
140+
141+
const empty_config = {
142+
detail: {},
143+
viewers: {},
144+
};
145+
146+
const x = await page.evaluate(async (config) => {
147+
const workspace = document.getElementById("workspace");
148+
await workspace.restore(config);
149+
await workspace.flush();
150+
return workspace.outerHTML;
151+
}, config);
152+
153+
test.expect(x).toEqual(
154+
'<perspective-workspace id="workspace"><perspective-viewer slot="PERSPECTIVE_GENERATED_ID_1" table="superstore" theme="Pro Light"><perspective-viewer-datagrid style="position: absolute; inset: 0px; opacity: 1;" class="edit-mode-allowed" data-edit-mode="READ_ONLY"></perspective-viewer-datagrid><perspective-viewer-datagrid-toolbar slot="plugin-settings"></perspective-viewer-datagrid-toolbar></perspective-viewer><perspective-viewer slot="PERSPECTIVE_GENERATED_ID_0" table="superstore" class="workspace-master-widget" selectable="" theme="Pro Light"><perspective-viewer-datagrid style="position: absolute; inset: 0px; opacity: 1;" data-edit-mode="READ_ONLY"></perspective-viewer-datagrid><perspective-viewer-datagrid-toolbar slot="plugin-settings"></perspective-viewer-datagrid-toolbar></perspective-viewer></perspective-workspace>'
155+
);
156+
157+
await page.evaluate(async (config) => {
158+
const workspace = document.getElementById("workspace");
159+
await workspace.restore(config);
160+
await workspace.flush();
161+
}, empty_config);
162+
163+
return compare(
164+
page,
165+
`${context}-restore-a-blank-view-removes-master-panel.txt`
166+
);
167+
});
168+
169+
test("restore a view which reuses a master viewer, removes master panel", async ({
170+
page,
171+
}) => {
172+
const config = {
173+
detail: {
174+
main: {
175+
type: "tab-area",
176+
widgets: ["PERSPECTIVE_GENERATED_ID_1"],
177+
currentIndex: 0,
178+
},
179+
},
180+
master: { widgets: ["PERSPECTIVE_GENERATED_ID_0"], sizes: [1] },
181+
viewers: {
182+
PERSPECTIVE_GENERATED_ID_0: {
183+
table: "superstore",
184+
title: "One",
185+
},
186+
PERSPECTIVE_GENERATED_ID_1: {
187+
table: "superstore",
188+
title: "Two",
189+
},
190+
},
191+
};
192+
193+
const config2 = {
194+
detail: {
195+
main: {
196+
type: "split-area",
197+
orientation: "horizontal",
198+
children: [
199+
{
200+
type: "tab-area",
201+
widgets: ["PERSPECTIVE_GENERATED_ID_0"],
202+
currentIndex: 0,
203+
},
204+
{
205+
type: "tab-area",
206+
widgets: ["PERSPECTIVE_GENERATED_ID_1"],
207+
currentIndex: 0,
208+
},
209+
],
210+
sizes: [0.5, 0.5],
211+
},
212+
},
213+
master: { widgets: [], sizes: [] },
214+
viewers: {
215+
PERSPECTIVE_GENERATED_ID_0: {
216+
table: "superstore",
217+
title: "One",
218+
},
219+
PERSPECTIVE_GENERATED_ID_1: {
220+
table: "superstore",
221+
title: "Two",
222+
},
223+
},
224+
};
225+
226+
const x = await page.evaluate(async (config) => {
227+
const workspace = document.getElementById("workspace");
228+
await workspace.restore(config);
229+
await workspace.flush();
230+
return workspace.outerHTML;
231+
}, config);
232+
233+
test.expect(x).toEqual(
234+
'<perspective-workspace id="workspace"><perspective-viewer slot="PERSPECTIVE_GENERATED_ID_1" table="superstore" theme="Pro Light"><perspective-viewer-datagrid style="position: absolute; inset: 0px; opacity: 1;" class="edit-mode-allowed" data-edit-mode="READ_ONLY"></perspective-viewer-datagrid><perspective-viewer-datagrid-toolbar slot="plugin-settings"></perspective-viewer-datagrid-toolbar></perspective-viewer><perspective-viewer slot="PERSPECTIVE_GENERATED_ID_0" table="superstore" class="workspace-master-widget" selectable="" theme="Pro Light"><perspective-viewer-datagrid style="position: absolute; inset: 0px; opacity: 1;" data-edit-mode="READ_ONLY"></perspective-viewer-datagrid><perspective-viewer-datagrid-toolbar slot="plugin-settings"></perspective-viewer-datagrid-toolbar></perspective-viewer></perspective-workspace>'
235+
);
236+
237+
await page.evaluate(async (config) => {
238+
const workspace = document.getElementById("workspace");
239+
await workspace.restore(config);
240+
await workspace.flush();
241+
}, config2);
242+
243+
return compare(
244+
page,
245+
`${context}-restore-which-reseats-master-viewer.txt`
246+
);
247+
});
248+
});
119249
}
120250

121251
test.describe("Workspace restore", () => {

0 commit comments

Comments
 (0)