Skip to content

Commit

Permalink
feat: forms inhibited when summary selected and vice versa (#1037)
Browse files Browse the repository at this point in the history
  • Loading branch information
@hexaltation
hexaltation authored Aug 14, 2024
1 parent dfb8168 commit 93ed1be
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 18 deletions.
76 changes: 59 additions & 17 deletions app/client/ui/PageWidgetPicker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -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'),
)
Expand Down Expand Up @@ -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'),
Expand Down Expand Up @@ -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);
}

}
Expand Down Expand Up @@ -535,6 +572,7 @@ const cssEntry = styled('div', `
&-disabled {
color: ${theme.widgetPickerItemDisabledBg};
cursor: default;
pointer-events: none;
}
&-disabled&-selected {
background-color: inherit;
Expand Down Expand Up @@ -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', `
Expand Down
27 changes: 26 additions & 1 deletion test/nbrowser/saveViewSection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
});
});

0 comments on commit 93ed1be

Please sign in to comment.