From 93ed1bec5ea3f25fa198c95f3367d7374a0143df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Cutzach?= Date: Wed, 14 Aug 2024 16:59:06 +0200 Subject: [PATCH] feat: forms inhibited when summary selected and vice versa (#1037) --- app/client/ui/PageWidgetPicker.ts | 76 ++++++++++++++++++++++++------- test/nbrowser/saveViewSection.ts | 27 ++++++++++- 2 files changed, 85 insertions(+), 18 deletions(-) diff --git a/app/client/ui/PageWidgetPicker.ts b/app/client/ui/PageWidgetPicker.ts index 61552422..e61757f7 100644 --- a/app/client/ui/PageWidgetPicker.ts +++ b/app/client/ui/PageWidgetPicker.ts @@ -95,25 +95,46 @@ export interface IOptions extends ISelectOptions { placement?: Popper.Placement; } +export interface ICompatibleTypes { + + // true if "New Page" is selected in Page Picker + isNewPage: Boolean | undefined; + + // true if can be summarized + summarize: Boolean; +} + const testId = makeTestId('test-wselect-'); // The picker disables some choices that do not make much sense. This function return the list of // compatible types given the tableId and whether user is creating a new page or not. -function getCompatibleTypes(tableId: TableRef, isNewPage: boolean|undefined): IWidgetType[] { +function getCompatibleTypes(tableId: TableRef, + {isNewPage, summarize}: ICompatibleTypes): IWidgetType[] { + let compatibleTypes: Array = []; if (tableId !== 'New Table') { - return ['record', 'single', 'detail', 'chart', 'custom', 'custom.calendar', 'form']; + compatibleTypes = ['record', 'single', 'detail', 'chart', 'custom', 'custom.calendar', 'form']; } else if (isNewPage) { // New view + new table means we'll be switching to the primary view. - return ['record', 'form']; + compatibleTypes = ['record', 'form']; } else { // The type 'chart' makes little sense when creating a new table. - return ['record', 'single', 'detail', 'form']; + compatibleTypes = ['record', 'single', 'detail', 'form']; } + return summarize ? compatibleTypes.filter((el) => isSummaryCompatible(el)) : compatibleTypes; +} + +// The Picker disables some choices that do not make much sense. +// This function return a boolean telling if summary can be used with this type. +function isSummaryCompatible(widgetType: IWidgetType): boolean { + const incompatibleTypes: Array = ['form']; + return !incompatibleTypes.includes(widgetType); } // Whether table and type make for a valid selection whether the user is creating a new page or not. -function isValidSelection(table: TableRef, type: IWidgetType, isNewPage: boolean|undefined) { - return table !== null && getCompatibleTypes(table, isNewPage).includes(type); +function isValidSelection(table: TableRef, + type: IWidgetType, + {isNewPage, summarize}: ICompatibleTypes) { + return table !== null && getCompatibleTypes(table, {isNewPage, summarize}).includes(type); } export type ISaveFunc = (val: IPageWidget) => Promise; @@ -213,7 +234,13 @@ export function buildPageWidgetPicker( // whether the current selection is valid function isValid() { - return isValidSelection(value.table.get(), value.type.get(), options.isNewPage); + return isValidSelection( + value.table.get(), + value.type.get(), + { + isNewPage: options.isNewPage, + summarize: value.summarize.get() + }); } // Summarizing a table causes the 'Group By' panel to expand on the right. To prevent it from @@ -299,7 +326,7 @@ export class PageWidgetSelect extends Disposable { null; private _isNewTableDisabled = Computed.create(this, this._value.type, (use, type) => !isValidSelection( - 'New Table', type, this._options.isNewPage)); + 'New Table', type, {isNewPage: this._options.isNewPage, summarize: use(this._value.summarize)})); constructor( private _value: IWidgetValueObs, @@ -318,7 +345,9 @@ export class PageWidgetSelect extends Disposable { header(t("Select Widget")), sectionTypes.map((value) => { const widgetInfo = getWidgetTypes(value); - const disabled = computed(this._value.table, (use, tid) => this._isTypeDisabled(value, tid)); + const disabled = computed(this._value.table, + (use, tid) => this._isTypeDisabled(value, tid, use(this._value.summarize)) + ); return cssEntry( dom.autoDispose(disabled), cssTypeIcon(widgetInfo.icon), @@ -355,11 +384,14 @@ export class PageWidgetSelect extends Disposable { cssEntry.cls('-selected', (use) => use(this._value.table) === table.id()), testId('table-label') ), - cssPivot( - cssBigIcon('Pivot'), - cssEntry.cls('-selected', (use) => use(this._value.summarize) && use(this._value.table) === table.id()), - dom.on('click', (ev, el) => this._selectPivot(table.id(), el as HTMLElement)), - testId('pivot'), + cssPivot( + cssBigIcon('Pivot'), + cssEntry.cls('-selected', (use) => use(this._value.summarize) && + use(this._value.table) === table.id() + ), + cssEntry.cls('-disabled', (use) => !isSummaryCompatible(use(this._value.type))), + dom.on('click', (ev, el) => this._selectPivot(table.id(), el as HTMLElement)), + testId('pivot'), ), testId('table'), ) @@ -410,7 +442,12 @@ export class PageWidgetSelect extends Disposable { // there are no changes. this._options.buttonLabel || t("Add to Page"), dom.prop('disabled', (use) => !isValidSelection( - use(this._value.table), use(this._value.type), this._options.isNewPage) + use(this._value.table), + use(this._value.type), + { + isNewPage: this._options.isNewPage, + summarize: use(this._value.summarize) + }) ), dom.on('click', () => this._onSave().catch(reportError)), testId('addBtn'), @@ -464,11 +501,11 @@ export class PageWidgetSelect extends Disposable { this._value.columns.set(newIds); } - private _isTypeDisabled(type: IWidgetType, table: TableRef) { + private _isTypeDisabled(type: IWidgetType, table: TableRef, isSummaryOn: boolean) { if (table === null) { return false; } - return !getCompatibleTypes(table, this._options.isNewPage).includes(type); + return !getCompatibleTypes(table, {isNewPage: this._options.isNewPage, summarize: isSummaryOn}).includes(type); } } @@ -535,6 +572,7 @@ const cssEntry = styled('div', ` &-disabled { color: ${theme.widgetPickerItemDisabledBg}; cursor: default; + pointer-events: none; } &-disabled&-selected { background-color: inherit; @@ -578,6 +616,10 @@ const cssBigIcon = styled(icon, ` width: 24px; height: 24px; background-color: ${theme.widgetPickerSummaryIcon}; + .${cssEntry.className}-disabled > & { + opacity: 0.25; + filter: saturate(0); + } `); const cssFooter = styled('div', ` diff --git a/test/nbrowser/saveViewSection.ts b/test/nbrowser/saveViewSection.ts index a1b678f2..002225c3 100644 --- a/test/nbrowser/saveViewSection.ts +++ b/test/nbrowser/saveViewSection.ts @@ -99,7 +99,6 @@ describe("saveViewSection", function() { await switchTypeAndAssert('Card'); await switchTypeAndAssert('Table'); await switchTypeAndAssert('Chart'); - }); it("should work correctly when changing grouped by column", async () => { @@ -160,4 +159,30 @@ describe("saveViewSection", function() { // Check all columns are visible. await assertActiveSectionColumns('Test', 'count'); }); + + it("should disable summary when form type is selected", async () => { + // select form type + await driver.find('.test-dp-add-new').doClick(); + await driver.find('.test-dp-add-new-page').doClick(); + await driver.findContent('.test-wselect-type', gu.exactMatch("Form")).doClick(); + + // check that summary is disabled + assert.ok(await driver.find('.test-wselect-pivot[class*=-disabled]')); + + // close page widget picker + await driver.sendKeys(Key.ESCAPE); + }); + + it("should disable form when summary is selected", async () => { + // select table type then select summary for a Table + await driver.find('.test-dp-add-new').doClick(); + await driver.find('.test-dp-add-new-page').doClick(); + await driver.find('.test-wselect-pivot').doClick(); + + // check that form is disabled + assert.equal(await driver.find('.test-wselect-type[class*=-disabled]').getText(), "Form"); + + // close page widget picker + await driver.sendKeys(Key.ESCAPE); + }); });