feat: forms inhibited when summary selected and vice versa (#1037)

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<IWidgetType> = [];
   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<IWidgetType> = ['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<any>;
@@ -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),
@@ -357,7 +386,10 @@ export class PageWidgetSelect extends Disposable {
               ),
                 cssPivot(
                   cssBigIcon('Pivot'),
-                cssEntry.cls('-selected', (use) => use(this._value.summarize) && use(this._value.table) === table.id()),
+                  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'),
               ),
@@ -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', `

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);
+  });
 });