(core) Adding confirmation before remove last widget for a table

Summary:
When last widget for a table is removed, user is informed
about that and can decide between removing the widget and removing
both table and widget

Test Plan: Updated

Reviewers: georgegevoian

Reviewed By: georgegevoian

Differential Revision: https://phab.getgrist.com/D4295
This commit is contained in:
Jarosław Sadziński 2024-07-24 13:33:53 +02:00
parent f2141851be
commit 61942f6f4b
8 changed files with 249 additions and 30 deletions

View File

@ -195,7 +195,7 @@ export class LayoutTray extends DisposableWithEvents {
box.dispose(); box.dispose();
// And ask the viewLayout to save the specs. // And ask the viewLayout to save the specs.
viewLayout.saveLayoutSpec(); viewLayout.saveLayoutSpec().catch(reportError);
}, },
restoreSection: () => { restoreSection: () => {
// Get the section that is collapsed and clicked (we are setting this value). // Get the section that is collapsed and clicked (we are setting this value).
@ -206,14 +206,18 @@ export class LayoutTray extends DisposableWithEvents {
viewLayout.viewModel.activeCollapsedSections.peek().filter(x => x !== leafId) viewLayout.viewModel.activeCollapsedSections.peek().filter(x => x !== leafId)
); );
viewLayout.viewModel.activeSectionId(leafId); viewLayout.viewModel.activeSectionId(leafId);
viewLayout.saveLayoutSpec(); viewLayout.saveLayoutSpec().catch(reportError);
}, },
// Delete collapsed section. // Delete collapsed section.
deleteCollapsedSection: () => { deleteCollapsedSection: async () => {
// This section is still in the view (but not in the layout). So we can just remove it. // This section is still in the view (but not in the layout). So we can just remove it.
const leafId = viewLayout.viewModel.activeCollapsedSectionId(); const leafId = viewLayout.viewModel.activeCollapsedSectionId();
if (!leafId) { return; } if (!leafId) { return; }
this.viewLayout.removeViewSection(leafId);
viewLayout.docModel.docData.bundleActions('removing section', async () => {
if (!await this.viewLayout.removeViewSection(leafId)) {
return;
}
// We need to manually update the layout. Main layout editor doesn't care about missing sections. // We need to manually update the layout. Main layout editor doesn't care about missing sections.
// but we can't afford that. Without removing it, user can add another section that will be collapsed // but we can't afford that. Without removing it, user can add another section that will be collapsed
// from the start, as the id will be the same as the one we just removed. // from the start, as the id will be the same as the one we just removed.
@ -222,7 +226,8 @@ export class LayoutTray extends DisposableWithEvents {
validSections.delete(leafId); validSections.delete(leafId);
currentSpec.collapsed = currentSpec.collapsed currentSpec.collapsed = currentSpec.collapsed
?.filter(x => typeof x.leaf === 'number' && validSections.has(x.leaf)); ?.filter(x => typeof x.leaf === 'number' && validSections.has(x.leaf));
viewLayout.saveLayoutSpec(currentSpec); await viewLayout.saveLayoutSpec(currentSpec);
}).catch(reportError);
} }
}; };
this.autoDispose(commands.createGroup(commandGroup, this, true)); this.autoDispose(commands.createGroup(commandGroup, this, true));
@ -843,7 +848,7 @@ class ExternalLeaf extends Disposable implements Dropped {
// and the section won't be created on time. // and the section won't be created on time.
this.model.viewLayout.layoutEditor.triggerUserEditStop(); this.model.viewLayout.layoutEditor.triggerUserEditStop();
// Manually save the layout. // Manually save the layout.
this.model.viewLayout.saveLayoutSpec(); this.model.viewLayout.saveLayoutSpec().catch(reportError);
} }
})); }));

View File

@ -20,7 +20,12 @@ import {reportError} from 'app/client/models/errors';
import {getTelemetryWidgetTypeFromVS} from 'app/client/ui/widgetTypesMap'; import {getTelemetryWidgetTypeFromVS} from 'app/client/ui/widgetTypesMap';
import {isNarrowScreen, mediaSmall, testId, theme} from 'app/client/ui2018/cssVars'; import {isNarrowScreen, mediaSmall, testId, theme} from 'app/client/ui2018/cssVars';
import {icon} from 'app/client/ui2018/icons'; import {icon} from 'app/client/ui2018/icons';
import {ISaveModalOptions, saveModal} from 'app/client/ui2018/modals';
import {DisposableWithEvents} from 'app/common/DisposableWithEvents'; import {DisposableWithEvents} from 'app/common/DisposableWithEvents';
import {makeT} from 'app/client/lib/localization';
import {urlState} from 'app/client/models/gristUrlState';
import {cssRadioCheckboxOptions, radioCheckboxOption} from 'app/client/ui2018/checkbox';
import {cssLink} from 'app/client/ui2018/links';
import {mod} from 'app/common/gutil'; import {mod} from 'app/common/gutil';
import { import {
Computed, Computed,
@ -39,6 +44,8 @@ import * as ko from 'knockout';
import debounce from 'lodash/debounce'; import debounce from 'lodash/debounce';
import * as _ from 'underscore'; import * as _ from 'underscore';
const t = makeT('ViewLayout');
// tslint:disable:no-console // tslint:disable:no-console
const viewSectionTypes: {[key: string]: any} = { const viewSectionTypes: {[key: string]: any} = {
@ -125,7 +132,7 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
this.listenTo(this.layout, 'layoutUserEditStop', () => { this.listenTo(this.layout, 'layoutUserEditStop', () => {
this.isResizing.set(false); this.isResizing.set(false);
this.layoutSaveDelay.schedule(1000, () => { this.layoutSaveDelay.schedule(1000, () => {
this.saveLayoutSpec(); this.saveLayoutSpec().catch(reportError);
}); });
}); });
@ -187,7 +194,7 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
})); }));
const commandGroup = { const commandGroup = {
deleteSection: () => { this.removeViewSection(this.viewModel.activeSectionId()); }, deleteSection: () => { this.removeViewSection(this.viewModel.activeSectionId()).catch(reportError); },
nextSection: () => { this._otherSection(+1); }, nextSection: () => { this._otherSection(+1); },
prevSection: () => { this._otherSection(-1); }, prevSection: () => { this._otherSection(-1); },
printSection: () => { printViewSection(this.layout, this.viewModel.activeSection()).catch(reportError); }, printSection: () => { printViewSection(this.layout, this.viewModel.activeSection()).catch(reportError); },
@ -265,31 +272,83 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
this._savePending.set(false); this._savePending.set(false);
// Cancel the automatic delay. // Cancel the automatic delay.
this.layoutSaveDelay.cancel(); this.layoutSaveDelay.cancel();
if (!this.layout) { return; } if (!this.layout) { return Promise.resolve(); }
// Only save layout changes when the document isn't read-only. // Only save layout changes when the document isn't read-only.
if (!this.gristDoc.isReadonly.get()) { if (!this.gristDoc.isReadonly.get()) {
if (!specs) { if (!specs) {
specs = this.layout.getLayoutSpec(); specs = this.layout.getLayoutSpec();
specs.collapsed = this.viewModel.activeCollapsedSections.peek().map((leaf)=> ({leaf})); specs.collapsed = this.viewModel.activeCollapsedSections.peek().map((leaf)=> ({leaf}));
} }
this.viewModel.layoutSpecObj.setAndSave(specs).catch(reportError); return this.viewModel.layoutSpecObj.setAndSave(specs).catch(reportError);
} }
this._onResize(); this._onResize();
return Promise.resolve();
} }
// Removes a view section from the current view. Should only be called if there is /**
// more than one viewsection in the view. * Removes a view section from the current view. Should only be called if there is more than
public removeViewSection(viewSectionRowId: number) { * one viewsection in the view.
* @returns A promise that resolves with true when the view section is removed. If user was
* prompted and decided to cancel, the promise resolves with false.
*/
public async removeViewSection(viewSectionRowId: number) {
this.maximized.set(null); this.maximized.set(null);
const viewSection = this.viewModel.viewSections().all().find(s => s.getRowId() === viewSectionRowId); const viewSection = this.viewModel.viewSections().all().find(s => s.getRowId() === viewSectionRowId);
if (!viewSection) { if (!viewSection) {
throw new Error(`Section not found: ${viewSectionRowId}`); throw new Error(`Section not found: ${viewSectionRowId}`);
} }
const tableId = viewSection.table.peek().tableId.peek();
// Check if this is a UserTable (not summary) and if so, if it is available on any other page
// we have access to (or even on this page but in different widget). If yes, then we are safe
// to remove it, otherwise we need to warn the user.
const logTelemetry = () => {
const widgetType = getTelemetryWidgetTypeFromVS(viewSection); const widgetType = getTelemetryWidgetTypeFromVS(viewSection);
logTelemetryEvent('deletedWidget', {full: {docIdDigest: this.gristDoc.docId(), widgetType}}); logTelemetryEvent('deletedWidget', {full: {docIdDigest: this.gristDoc.docId(), widgetType}});
};
this.gristDoc.docData.sendAction(['RemoveViewSection', viewSectionRowId]).catch(reportError); const isUserTable = () => viewSection.table.peek().isSummary.peek() === false;
const notInAnyOtherSection = () => {
// Get all viewSection we have access to, and check if the table is used in any of them.
const others = this.gristDoc.docModel.viewSections.rowModels
.filter(vs => !vs.isDisposed())
.filter(vs => vs.id.peek() !== viewSectionRowId)
.filter(vs => vs.isRaw.peek() === false)
.filter(vs => vs.isRecordCard.peek() === false)
.filter(vs => vs.tableId.peek() === viewSection.tableId.peek());
return others.length === 0;
};
const REMOVED = true, IGNORED = false;
const possibleActions = {
[DELETE_WIDGET]: async () => {
logTelemetry();
await this.gristDoc.docData.sendAction(['RemoveViewSection', viewSectionRowId]);
return REMOVED;
},
[DELETE_DATA]: async () => {
logTelemetry();
await this.gristDoc.docData.sendActions([
['RemoveViewSection', viewSectionRowId],
['RemoveTable', tableId],
]);
return REMOVED;
},
[CANCEL]: async () => IGNORED,
};
const tableName = () => viewSection.table.peek().tableNameDef.peek();
const needPrompt = isUserTable() && notInAnyOtherSection();
const decision = needPrompt
? widgetRemovalPrompt(tableName())
: Promise.resolve(DELETE_WIDGET as PromptAction);
return possibleActions[await decision]();
} }
public rebuildLayout(layoutSpec: BoxSpec) { public rebuildLayout(layoutSpec: BoxSpec) {
@ -417,6 +476,47 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
} }
} }
const DELETE_WIDGET = 'deleteOnlyWidget';
const DELETE_DATA = 'deleteDataAndWidget';
const CANCEL = 'cancel';
type PromptAction = typeof DELETE_WIDGET | typeof DELETE_DATA | typeof CANCEL;
function widgetRemovalPrompt(tableName: string): Promise<PromptAction> {
return new Promise<PromptAction>((resolve) => {
saveModal((ctl, owner): ISaveModalOptions => {
const selected = Observable.create<PromptAction | ''>(owner, '');
const saveDisabled = Computed.create(owner, use => use(selected) === '');
const saveFunc = async () => selected.get() && resolve(selected.get() as PromptAction);
owner.onDispose(() => resolve(CANCEL));
return {
title: t('Table {{tableName}} will no longer be visible', { tableName }),
body: dom('div',
testId('removePopup'),
cssRadioCheckboxOptions(
radioCheckboxOption(selected, DELETE_DATA, t("Delete data and this widget.")),
radioCheckboxOption(selected, DELETE_WIDGET,
t(
`Keep data and delete widget. Table will remain available in {{rawDataLink}}`,
{
rawDataLink: cssLink(
t('raw data page'),
urlState().setHref({docPage: 'data'}),
{target: '_blank'},
)
}
)
),
),
),
saveDisabled,
saveLabel: t("Delete"),
saveFunc,
width: 'fixed-wide',
};
});
});
}
const cssLayoutBox = styled('div', ` const cssLayoutBox = styled('div', `
@media screen and ${mediaSmall} { @media screen and ${mediaSmall} {
&-active, &-inactive { &-active, &-inactive {

View File

@ -39,6 +39,7 @@ export interface TableRec extends IRowModel<"_grist_Tables"> {
// If user can select this table in various places. // If user can select this table in various places.
// Note: Some hidden tables can still be visible on RawData view. // Note: Some hidden tables can still be visible on RawData view.
isHidden: ko.Computed<boolean>; isHidden: ko.Computed<boolean>;
isSummary: ko.Computed<boolean>;
tableColor: string; tableColor: string;
disableAddRemoveRows: ko.Computed<boolean>; disableAddRemoveRows: ko.Computed<boolean>;
@ -68,6 +69,8 @@ export function createTableRec(this: TableRec, docModel: DocModel): void {
this.primaryTableId = ko.pureComputed(() => this.primaryTableId = ko.pureComputed(() =>
this.summarySourceTable() ? this.summarySource().tableId() : this.tableId()); this.summarySourceTable() ? this.summarySource().tableId() : this.tableId());
this.isSummary = this.autoDispose(ko.pureComputed(() => Boolean(this.summarySourceTable())));
this.groupByColumns = ko.pureComputed(() => this.columns().all().filter(c => c.summarySourceCol())); this.groupByColumns = ko.pureComputed(() => this.columns().all().filter(c => c.summarySourceCol()));
this.groupDesc = ko.pureComputed(() => { this.groupDesc = ko.pureComputed(() => {

View File

@ -93,9 +93,12 @@ export interface ViewSectionRec extends IRowModel<"_grist_Views_section">, RuleO
// in which case the UI prevents various things like hiding columns or changing the widget type. // in which case the UI prevents various things like hiding columns or changing the widget type.
isRaw: ko.Computed<boolean>; isRaw: ko.Computed<boolean>;
tableRecordCard: ko.Computed<ViewSectionRec> /** Is this table card viewsection (the one available after pressing spacebar) */
isRecordCard: ko.Computed<boolean>; isRecordCard: ko.Computed<boolean>;
/** Card record viewSection for associated table (might be the same section) */
tableRecordCard: ko.Computed<ViewSectionRec>;
/** True if this section is disabled. Currently only used by Record Card sections. */ /** True if this section is disabled. Currently only used by Record Card sections. */
disabled: modelUtil.KoSaveableObservable<boolean>; disabled: modelUtil.KoSaveableObservable<boolean>;

View File

@ -178,9 +178,7 @@ describe('SelectBySummary', function() {
it('should filter a summary table selected by a less detailed summary table', async function() { it('should filter a summary table selected by a less detailed summary table', async function() {
// Delete the Table1 widget so that we can hide the table in ACL without hiding the whole page. // Delete the Table1 widget so that we can hide the table in ACL without hiding the whole page.
const menu = await gu.openSectionMenu('viewLayout', 'TABLE1'); await gu.deleteWidget('TABLE1');
await menu.findContent('.test-cmd-name', 'Delete widget').click();
await gu.waitForServer();
// Open the ACL UI // Open the ACL UI
await driver.find('.test-tools-access-rules').click(); await driver.find('.test-tools-access-rules').click();

View File

@ -761,6 +761,96 @@ describe("ViewLayoutCollapse", function() {
await gu.checkForErrors(); await gu.checkForErrors();
}); });
it('should prompt when last section is removed from tray', async () => {
const revert = await gu.begin();
// Add brand new table and collapse it.
await gu.addNewSection('Table', 'New Table', {tableName: 'ToCollapse'});
await collapseByMenu('ToCollapse');
// Now try to remove it, we should see prompt.
await openCollapsedSectionMenu('ToCollapse');
await driver.find('.test-section-delete').click();
assert.match(
await driver.find('.test-modal-title').getText(),
/Table ToCollapse will no longer be visible/
);
// Select first option, to delete both table and widget.
await driver.find('.test-option-deleteDataAndWidget').click();
await driver.find('.test-modal-confirm').click();
await gu.waitForServer();
// Make sure it is removed.
assert.deepEqual(await collapsedSectionTitles(), []);
assert.deepEqual(await visibleTables(), ['Companies', 'Investments']);
await gu.sendKeys(Key.ESCAPE);
// Single undo should add it back.
await gu.undo();
assert.deepEqual(await collapsedSectionTitles(), ['TOCOLLAPSE']);
assert.deepEqual(await visibleTables(), ['Companies', 'Investments', 'ToCollapse']);
// Now do the same but, keep data.
await openCollapsedSectionMenu('ToCollapse');
await driver.find('.test-section-delete').click();
await driver.findWait('.test-modal-dialog', 100);
await driver.find('.test-option-deleteOnlyWidget').click();
await driver.find('.test-modal-confirm').click();
await gu.waitForServer();
// Make sure it is removed.
assert.deepEqual(await collapsedSectionTitles(), []);
assert.deepEqual(await visibleTables(), ['Companies', 'Investments', 'ToCollapse']);
// Test single undo.
await gu.undo();
assert.deepEqual(await collapsedSectionTitles(), ['TOCOLLAPSE']);
assert.deepEqual(await visibleTables(), ['Companies', 'Investments', 'ToCollapse']);
// Uncollapse it, and do the same with normal section.
await addToMainByMenu('ToCollapse');
// Now try to remove it, we should see prompt.
assert.include(
await driver.findAll('.test-viewsection-title', e => e.getText()), 'TOCOLLAPSE');
await gu.openSectionMenu('viewLayout', 'ToCollapse');
await driver.find('.test-section-delete').click();
await driver.findWait('.test-modal-dialog', 100);
await driver.find('.test-option-deleteOnlyWidget').click();
await driver.find('.test-modal-confirm').click();
await gu.waitForServer();
assert.notInclude(
await driver.findAll('.test-viewsection-title', e => e.getText()), 'TOCOLLAPSE');
assert.deepEqual(await visibleTables(), ['Companies', 'Investments', 'ToCollapse']);
// Test undo.
await gu.undo();
assert.include(
await driver.findAll('.test-viewsection-title', e => e.getText()), 'TOCOLLAPSE');
// Do the same but delete data and widget.
await gu.openSectionMenu('viewLayout', 'ToCollapse');
await driver.find('.test-section-delete').click();
await driver.findWait('.test-modal-dialog', 100);
await driver.find('.test-option-deleteDataAndWidget').click();
await driver.find('.test-modal-confirm').click();
await gu.waitForServer();
// Make sure it is removed.
assert.notInclude(
await driver.findAll('.test-viewsection-title', e => e.getText()), 'TOCOLLAPSE');
assert.deepEqual(await visibleTables(), ['Companies', 'Investments']);
// Test undo.
await gu.undo();
assert.include(
await driver.findAll('.test-viewsection-title', e => e.getText()), 'TOCOLLAPSE');
assert.deepEqual(await visibleTables(), ['Companies', 'Investments', 'ToCollapse']);
await revert();
});
it("should switch active section when collapsed", async () => { it("should switch active section when collapsed", async () => {
const revert = await gu.begin(); const revert = await gu.begin();
await gu.selectSectionByTitle(gu.exactMatch(COMPANIES)); await gu.selectSectionByTitle(gu.exactMatch(COMPANIES));
@ -989,3 +1079,11 @@ async function waitForSave() {
await gu.waitForServer(); await gu.waitForServer();
}, 3000); }, 3000);
} }
async function visibleTables() {
await driver.findWait('.test-dp-add-new', 2000).doClick();
await driver.find('.test-dp-add-new-page').doClick();
const titles = await driver.findAll('.test-wselect-table', e => e.getText());
await gu.sendKeys(Key.ESCAPE);
return titles.map(x => x.trim()).filter(Boolean).filter(x => x !== 'New Table');
}

View File

@ -113,8 +113,7 @@ describe('Views.ntest', function() {
await gu.waitForServer(); await gu.waitForServer();
await gu.actions.viewSection('TABLE4').selectSection(); await gu.actions.viewSection('TABLE4').selectSection();
// Delete the section // Delete the section
await gu.actions.viewSection('TABLE4').selectMenuOption('viewLayout', 'Delete widget'); await gu.deleteWidget('TABLE4');
await gu.waitForServer();
// Assert that the default section (Table1 record) is now active. // Assert that the default section (Table1 record) is now active.
assert.equal(await $('.active_section > .viewsection_title').text(), 'TABLE1'); assert.equal(await $('.active_section > .viewsection_title').text(), 'TABLE1');
// Assert that focus is returned to the deleted section on undo. // Assert that focus is returned to the deleted section on undo.

View File

@ -3791,6 +3791,19 @@ export async function waitForAccessDenied() {
}); });
} }
/**
* Deletes a widget by title. Optionally confirms deletion only for the widget without the data.
*/
export async function deleteWidget(title: string) {
const menu = await openSectionMenu('viewLayout', title);
await menu.findContent('.test-cmd-name', 'Delete widget').click();
if (await driver.findWait('.test-option-deleteOnlyWidget', 100).isPresent()) {
await driver.find('.test-option-deleteOnlyWidget').click();
await driver.find('.test-modal-confirm').click();
}
await waitForServer();
}
} // end of namespace gristUtils } // end of namespace gristUtils
stackWrapOwnMethods(gristUtils); stackWrapOwnMethods(gristUtils);