diff --git a/app/client/components/ViewLayout.ts b/app/client/components/ViewLayout.ts index 12efaa90..f017f992 100644 --- a/app/client/components/ViewLayout.ts +++ b/app/client/components/ViewLayout.ts @@ -334,7 +334,12 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent { function addToSpec(leafId: number) { const newBox = tmpLayout.buildLayoutBox({ leaf: leafId }); - const rows = tmpLayout.rootBox()!.childBoxes.peek(); + const root = tmpLayout.rootBox(); + if (!root || root.isDisposed()) { + tmpLayout.setRoot(newBox); + return newBox; + } + const rows = root.childBoxes.peek(); const lastRow = rows[rows.length - 1]; if (rows.length >= 1 && lastRow.isLeaf()) { // Add a new child to the last row. diff --git a/app/client/components/duplicatePage.ts b/app/client/components/duplicatePage.ts index a4e01775..c8e996f4 100644 --- a/app/client/components/duplicatePage.ts +++ b/app/client/components/duplicatePage.ts @@ -199,13 +199,20 @@ function newViewSectionAction(widget: IPageWidget, viewId: number) { /** * Replaces each `leaf` id in layoutSpec by its corresponding id in mapIds. Leave unchanged if id is * missing from mapIds. + * LayoutSpec is a tree structure with leaves (that have `leaf` property) or containers of leaves. The root + * container (or leaf) also includes a list of collapsed leaves in `collapsed` property. + * + * Example use: + * patchLayoutSpec({ + * leaf: 1, +* collapsed: [{leaf: 2}] + * }, {1: 10, 2: 20}) */ export function patchLayoutSpec(layoutSpec: any, mapIds: {[id: number]: number}) { - return cloneDeepWith(layoutSpec, (val) => { - if (typeof val === 'object' && val !== null) { - if (mapIds[val.leaf]) { - return {...val, leaf: mapIds[val.leaf]}; - } + const cloned = cloneDeepWith(layoutSpec, (val, key) => { + if (key === 'leaf' && mapIds[val]) { + return mapIds[val]; } }); + return cloned; } diff --git a/test/nbrowser/ViewLayoutCollapse.ts b/test/nbrowser/ViewLayoutCollapse.ts index b1643e66..d157bc6b 100644 --- a/test/nbrowser/ViewLayoutCollapse.ts +++ b/test/nbrowser/ViewLayoutCollapse.ts @@ -16,11 +16,86 @@ describe("ViewLayoutCollapse", function() { before(async () => { session = await gu.session().login(); - await session.tempDoc(cleanup, 'Investment Research.grist'); - await gu.openPage("Overview"); + await session.tempNewDoc(cleanup); + }); + + it('fix:copies collapsed sections properly', async function() { + // When one of 2 widget was collapsed, the resulting widget can become a root section. Then, + // when a page was duplicated, the layout was duplicated incorrectly (with wrong collapsed + // section). This resulted in a bug, when the root section was deleted, as it was the last + // section in the saved layout, but not the last section on the visible layout. + + // Add new page with new table. + await gu.addNewPage('Table', 'New Table', { + tableName: 'Broken' + }); + + await gu.renameActiveSection('Collapsed'); + + // Add section here (with the same table). + await gu.addNewSection('Table', 'Broken'); + + // Rename it so that it is easier to find. + await gu.renameActiveSection('NotCollapsed'); + + // Now store the layout, by amending it (so move the collapsed widget below). + const {height} = await gu.getSection('NotCollapsed').getRect(); + await dragMain('Collapsed'); + await move(gu.getSection('NotCollapsed'), { x: 50, y: height / 2 }); + await driver.sleep(300); + await move(gu.getSection('NotCollapsed'), { x: 100, y: height / 2 }); + await driver.sleep(300); + await driver.withActions(actions => actions.release()); + // Wait for the debounced save. + await driver.sleep(1500); + await gu.waitForServer(); + + // Now collapse it. + await collapseByMenu('Collapsed'); + + // Now duplicate the page. + await gu.duplicatePage('Broken', 'Broken2'); + + // Now on this page we saw two uncollapsed sections (make sure this is not the case). + assert.deepEqual(await gu.getSectionTitles(), ['NotCollapsed']); + }); + + it('fix:can delete root section', async function() { + // But even if the layout spec was corrupted, we still should be able to delete the root section + // when replacing it with new one. + + // Break the spec. + const specJson: string = await driver.executeScript( + 'return gristDocPageModel.gristDoc.get().docModel.views.rowModels[3].layoutSpec()' + ); + + // To break the spec, we will replace id of the collapsed section, then viewLayout will try to fix it, + // by rendering the missing section without patching the layout spec (which is good, because this could + // happen on readonly doc or a snapshot). + const spec = JSON.parse(specJson); + spec.collapsed[0].leaf = -10; + + await driver.executeScript( + `gristDocPageModel.gristDoc.get().docModel.views.rowModels[3].layoutSpec.setAndSave('${JSON.stringify(spec)}')` + ); + + await gu.waitForServer(); + + // We now should see two sections. + assert.deepEqual(await gu.getSectionTitles(), ['NotCollapsed', 'Collapsed']); + + // And we should be able to delete the top one (NotCollapsed). + await gu.openSectionMenu('viewLayout', 'NotCollapsed'); + await driver.findContent('.test-cmd-name', 'Delete widget').click(); + await gu.waitForServer(); + + await gu.checkForErrors(); }); it('fix: custom widget should restart when added back after collapsing', async function() { + await session.tempDoc(cleanup, 'Investment Research.grist'); + await gu.openPage("Overview"); + const revert = await gu.begin(); // Add custom section. diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 96fce930..b53faa3a 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -1186,6 +1186,27 @@ export async function addNewPage( await driver.wait(async () => (await driver.getCurrentUrl()) !== url, 2000); } +export async function duplicatePage(name: string|RegExp, newName?: string) { + await openPageMenu(name); + await driver.find('.test-docpage-duplicate').click(); + + if (newName) { + // Input will select text on focus, which can alter the text we enter, + // so make sure we type correct value. + await waitToPass(async () => { + const input = driver.find('.test-modal-dialog input'); + await input.click(); + await selectAll(); + await driver.sendKeys(newName); + assert.equal(await input.value(), newName); + }); + } + + await driver.find('.test-modal-confirm').click(); + await driver.findContentWait('.test-docpage-label', newName ?? /copy/, 6000); + await waitForServer(); +} + export async function openAddWidgetToPage() { await driver.findWait('.test-dp-add-new', 2000).doClick(); await driver.findWait('.test-dp-add-widget-to-page', 2000).doClick();