Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update GroupingChooser with local model for transient UI state #3420

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
options you wish to set via the new `gridOptions` parameter. Please note that, as before, not
all `GridConfig` options are supported by (or make sense for) the `DataView` component.

### 🐞 Bug Fixes

* Fixed bug where having multiple `GroupingChooser` components rendered using the same model would
result in the popover for all components showing on click.

### 📚 Libraries

* mobx `6.8 -> 6.9`
Expand Down
2 changes: 1 addition & 1 deletion appcontainer/AppContainerModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ export class AppContainerModel extends HoistModel {
// Implementation
//-----------------------------
private async getAuthStatusFromServerAsync(): Promise<boolean> {
return XH.fetchService
return await XH.fetchService
.fetchJson({
url: 'xh/authStatus',
timeout: 3 * MINUTES // Accommodate delay for user at a credentials prompt
Expand Down
2 changes: 1 addition & 1 deletion appcontainer/AppOption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class AppOption {
async getValueAsync(name) {
const {valueGetter, prefName} = this;
if (isFunction(valueGetter)) {
return valueGetter();
return await valueGetter();
} else {
return XH.prefService.get(prefName);
}
Expand Down
2 changes: 1 addition & 1 deletion appcontainer/UserAgentModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ export class UserAgentModel extends HoistModel {
// Implementation
//---------------------
private get uaParser() {
return (this._uaParser = this._uaParser ?? new parser());
return this._uaParser = this._uaParser ?? new parser();
}
}
4 changes: 1 addition & 3 deletions cmp/ag-grid/AgGrid.scss
Original file line number Diff line number Diff line change
Expand Up @@ -409,9 +409,7 @@ body.xh-app {
border: var(--xh-menu-border);

// Matching box-shadow of Blueprint context menu popover
box-shadow:
0 0 0 1px rgba(16, 22, 26, 0.2),
0 2px 4px rgba(16, 22, 26, 0.4),
box-shadow: 0 0 0 1px rgba(16, 22, 26, 0.2), 0 2px 4px rgba(16, 22, 26, 0.4),
0 8px 24px rgba(16, 22, 26, 0.4);

.ag-menu-option.ag-menu-option-active {
Expand Down
13 changes: 5 additions & 8 deletions cmp/ag-grid/AgGridModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -430,14 +430,11 @@ export class AgGridModel extends HoistModel {

// Pre-clear if only toggling abs for any sort. Ag-Grid doesn't handle abs and would skip
if (
sortBy.some(
curr =>
prevSortBy?.some(
prev =>
curr.sort === prev.sort &&
curr.colId === prev.colId &&
curr.abs != prev.abs
)
sortBy.some(curr =>
prevSortBy?.some(
prev =>
curr.sort === prev.sort && curr.colId === prev.colId && curr.abs != prev.abs
)
)
) {
togglingAbsSort = true;
Expand Down
2 changes: 1 addition & 1 deletion cmp/chart/impl/copyToClipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ function memoryCleanup(svgUrl) {
*/
async function loadBlob(dataUrl) {
const fetched = await fetch(dataUrl);
return fetched.blob();
return await fetched.blob();
}

/**
Expand Down
21 changes: 9 additions & 12 deletions cmp/dataview/DataViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,16 @@ import {
import {bindable, makeObservable} from '@xh/hoist/mobx';
import {throwIf} from '@xh/hoist/utils/js';
import {isFunction, isNumber} from 'lodash';
import {ReactNode} from 'react';

/**
* Configuration for a DataView.
*
* Additional properties not specified here will be passed to the underlying
* GridModel. Note this is for advanced usage - not all configs supported, and many will
* override DataView defaults in ways that will break this component.
*/
export interface DataViewConfig {
export interface DataViewConfig extends GridConfig {
// TODO: Accept grid keys without publicizing them?
/** A Store instance, or a config to create one. */
store?: Store | StoreConfig;

Expand All @@ -59,7 +63,7 @@ export interface DataViewConfig {
selModel?: StoreSelectionModel | StoreSelectionConfig | 'single' | 'multiple' | 'disabled';

/** Text/HTML to display if view has no records.*/
emptyText?: ReactNode;
emptyText?: string;

/** True to highlight the currently hovered row.*/
showHover?: boolean;
Expand Down Expand Up @@ -100,13 +104,6 @@ export interface DataViewConfig {
* the row's data. (Note that this may be null - e.g. for clicks on full-width group rows.)
*/
onRowDoubleClicked?: (e: any) => void;

/**
* "Escape hatch" object to pass directly to GridModel. Note these options may be used
* / overwritten by the framework itself, and are not all guaranteed to be compatible
* with its usages of GridModel.
*/
gridOptions?: Omit<GridConfig, keyof DataViewConfig>;
}

export type ItemHeightFn = (params: {
Expand Down Expand Up @@ -147,7 +144,7 @@ export class DataViewModel extends HoistModel {
rowClassRules,
onRowClicked,
onRowDoubleClicked,
gridOptions
...restArgs
} = config;

throwIf(
Expand Down Expand Up @@ -188,7 +185,7 @@ export class DataViewModel extends HoistModel {
onRowClicked,
onRowDoubleClicked,
columns,
...gridOptions
...restArgs
});
}

Expand Down
2 changes: 1 addition & 1 deletion cmp/filter/FilterChooserFieldSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {fmtDate, parseNumber} from '@xh/hoist/format';
import {stripTags, throwIf} from '@xh/hoist/utils/js';
import {isFunction, isNil} from 'lodash';
import {isValidElement, ReactNode} from 'react';
import {renderToStaticMarkup} from '@xh/hoist/utils/react';
import {renderToStaticMarkup} from 'react-dom/server';

export interface FilterChooserFieldSpecConfig extends BaseFilterFieldSpecConfig {
/**
Expand Down
5 changes: 1 addition & 4 deletions cmp/filter/impl/QueryEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,7 @@ export class QueryEngine {
// Local Helper functions
//------------------------
function caselessStartsWith(target, queryStr) {
return target
?.toString()
.toLowerCase()
.startsWith(queryStr?.toString().toLowerCase());
return target?.toString().toLowerCase().startsWith(queryStr?.toString().toLowerCase());
}

function caselessEquals(target, queryStr) {
Expand Down
2 changes: 1 addition & 1 deletion cmp/form/field/BaseFieldModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ export abstract class BaseFieldModel extends HoistModel {
const {value, name, displayName} = this,
fieldState = {value, name, displayName, fieldModel: this};

return constraint(fieldState, this.formModel.values);
return await constraint(fieldState, this.formModel.values);
});

const ret = await Promise.all(promises);
Expand Down
14 changes: 3 additions & 11 deletions cmp/grid/columns/Column.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,7 @@ import {
keysIn,
toString
} from 'lodash';
import {
Attributes,
createElement,
forwardRef,
isValidElement,
ReactNode,
useImperativeHandle
} from 'react';
import {createElement, forwardRef, isValidElement, ReactNode, useImperativeHandle} from 'react';
import {GridModel} from '../GridModel';
import {GridSorter} from '../GridSorter';
import {managedRenderer} from '../impl/Utils';
Expand Down Expand Up @@ -462,7 +455,7 @@ export class Column {
autosizeBufferPx: number;
autoHeight: boolean;
editable: boolean | ColumnEditableFn;
editor: FunctionComponent | ColumnEditorFn;
editor: ColumnEditorFn;
editorIsPopup: boolean;
setValueFn: ColumnSetValueFn;
getValueFn: ColumnGetValueFn;
Expand Down Expand Up @@ -948,8 +941,7 @@ export class Column {
ref
};
// Can be a component or elem factory/ ad-hoc render function.
if ((editor as any).isHoistComponent)
return createElement(editor, props as Attributes);
if ((editor as any).isHoistComponent) return createElement(editor, props);
if (isFunction(editor)) return editor(props);
throw XH.exception('Column editor must be a HoistComponent or a render function');
});
Expand Down
2 changes: 1 addition & 1 deletion cmp/grid/impl/ColumnWidthCalculator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
takeRight
} from 'lodash';
import {isValidElement} from 'react';
import {renderToStaticMarkup} from '@xh/hoist/utils/react';
import {renderToStaticMarkup} from 'react-dom/server';
import {Column} from '../columns';
import {GridModel} from '../GridModel';

Expand Down
2 changes: 1 addition & 1 deletion cmp/grid/impl/MenuSupport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {filterConsecutiveMenuSeparators} from '@xh/hoist/utils/impl';
import copy from 'clipboard-copy';
import {isEmpty, isFunction, isNil, isString, uniq} from 'lodash';
import {isValidElement} from 'react';
import {renderToStaticMarkup} from '@xh/hoist/utils/react';
import {renderToStaticMarkup} from 'react-dom/server';
import {GridContextMenuItemLike, GridContextMenuSpec} from '../GridContextMenu';

import type {GetContextMenuItemsParams, MenuItemDef} from '@xh/hoist/kit/ag-grid';
Expand Down
120 changes: 2 additions & 118 deletions cmp/grouping/GroupingChooserModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,10 @@ import {
PlainObject,
XH
} from '@xh/hoist/core';
import {action, computed, observable, makeObservable} from '@xh/hoist/mobx';
import {genDisplayName} from '@xh/hoist/data';
import {action, makeObservable, observable} from '@xh/hoist/mobx';
import {throwIf} from '@xh/hoist/utils/js';
import {createObservableRef} from '@xh/hoist/utils/react';
import {
cloneDeep,
difference,
isFunction,
isArray,
isEmpty,
isEqual,
isString,
keys,
sortBy
} from 'lodash';
import {cloneDeep, isArray, isEmpty, isEqual, isFunction, isString, keys, sortBy} from 'lodash';

export interface GroupingChooserConfig {
/**
Expand Down Expand Up @@ -94,32 +83,6 @@ export class GroupingChooserModel extends HoistModel {
persistValue: boolean = false;
persistFavorites: boolean = false;

// Implementation fields for Control
@observable.ref pendingValue: string[] = [];
@observable editorIsOpen: boolean = false;
@observable favoritesIsOpen: boolean = false;

popoverRef = createObservableRef<HTMLElement>();

@computed
get availableDims(): string[] {
return difference(this.dimensionNames, this.pendingValue);
}

@computed
get isValid(): boolean {
return this.validateValue(this.pendingValue);
}

@computed
get isAddEnabled(): boolean {
const {pendingValue, maxDepth, dimensionNames, availableDims} = this,
limit =
maxDepth > 0 ? Math.min(maxDepth, dimensionNames.length) : dimensionNames.length,
atMaxDepth = pendingValue.length === limit;
return !atMaxDepth && !isEmpty(availableDims);
}

constructor({
dimensions,
initialValue = [],
Expand Down Expand Up @@ -176,13 +139,6 @@ export class GroupingChooserModel extends HoistModel {
}
}

this.addReaction({
track: () => this.pendingValue,
run: () => {
if (this.commitOnChange) this.setValue(this.pendingValue);
}
});

this.setValue(value);
this.setFavorites(favorites);
}
Expand All @@ -194,69 +150,6 @@ export class GroupingChooserModel extends HoistModel {
return;
}
this.value = value;
this.pendingValue = value;
}

@action
toggleEditor() {
this.pendingValue = this.value;
this.editorIsOpen = !this.editorIsOpen;
this.favoritesIsOpen = false;
}

@action
toggleFavoritesMenu() {
this.favoritesIsOpen = !this.favoritesIsOpen;
this.editorIsOpen = false;
}

@action
closePopover() {
this.editorIsOpen = false;
this.favoritesIsOpen = false;
}

//-------------------------
// Value handling
//-------------------------
@action
addPendingDim(dimName: string) {
if (!dimName) return;
this.pendingValue = [...this.pendingValue, dimName];
}

@action
replacePendingDimAtIdx(dimName: string, idx: number) {
if (!dimName) return this.removePendingDimAtIdx(idx);
const pendingValue = [...this.pendingValue];
pendingValue[idx] = dimName;
this.pendingValue = pendingValue;
}

@action
removePendingDimAtIdx(idx: number) {
const pendingValue = [...this.pendingValue];
pendingValue.splice(idx, 1);
this.pendingValue = pendingValue;
}

@action
movePendingDimToIndex(dimName: string, toIdx: number) {
const pendingValue = [...this.pendingValue],
dim = pendingValue.find(it => it === dimName),
fromIdx = pendingValue.indexOf(dim);

pendingValue.splice(toIdx, 0, pendingValue.splice(fromIdx, 1)[0]);
this.pendingValue = pendingValue;
}

@action
commitPendingValueAndClose() {
const {pendingValue, value} = this;
if (!isEqual(value, pendingValue) && this.validateValue(pendingValue)) {
this.setValue(pendingValue);
}
this.closePopover();
}

validateValue(value) {
Expand Down Expand Up @@ -292,15 +185,6 @@ export class GroupingChooserModel extends HoistModel {
return this.dimensions[dimName].displayName;
}

//--------------------
// Drag Drop
//--------------------
onDragEnd(result) {
const {draggableId, destination} = result;
if (!destination) return;
this.movePendingDimToIndex(draggableId, destination.index);
}

//--------------------
// Favorites
//--------------------
Expand Down
Loading