From 0cdaaeeba4bf5d2536bbf47d59ec3fb55b7c72d2 Mon Sep 17 00:00:00 2001 From: Anselm McClain Date: Fri, 15 Nov 2024 15:10:13 -0800 Subject: [PATCH 01/13] Remove extra labeling of `ViewManager` "Revert" menu item --- desktop/cmp/viewmanager/ViewManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/cmp/viewmanager/ViewManager.ts b/desktop/cmp/viewmanager/ViewManager.ts index 3f80ab33f..84986dd34 100644 --- a/desktop/cmp/viewmanager/ViewManager.ts +++ b/desktop/cmp/viewmanager/ViewManager.ts @@ -193,7 +193,7 @@ const viewMenu = hoistCmp.factory({ }), menuItem({ icon: Icon.reset(), - text: `Revert ${DisplayName}`, + text: `Revert`, disabled: !model.isDirty, onClick: () => model.resetAsync() }), From 0bc88043363b07f5c7adb68c4b10926739fecc34 Mon Sep 17 00:00:00 2001 From: lbwexler Date: Sat, 16 Nov 2024 19:39:24 -0500 Subject: [PATCH 02/13] + Fix labelling of folders + Refactor tree-construction to its own file. + Misc clean-ups --- core/persist/viewmanager/Types.ts | 4 +- core/persist/viewmanager/ViewManagerModel.ts | 68 ++----------------- .../persist/viewmanager/impl/BuildViewTree.ts | 68 +++++++++++++++++++ desktop/cmp/viewmanager/ViewManager.ts | 4 +- 4 files changed, 79 insertions(+), 65 deletions(-) create mode 100644 core/persist/viewmanager/impl/BuildViewTree.ts diff --git a/core/persist/viewmanager/Types.ts b/core/persist/viewmanager/Types.ts index b1a5df575..30d3f6731 100644 --- a/core/persist/viewmanager/Types.ts +++ b/core/persist/viewmanager/Types.ts @@ -20,8 +20,10 @@ export interface View { isShared: boolean; lastUpdated: number; lastUpdatedBy: string; - /** User-supplied descriptive name. */ + /** User-supplied descriptive name, including folder designating prefix. */ name: string; + /** User-supplied descriptive name, without folder designating prefix. */ + shortName: string; /** Original creator of the view, and the only user with access to it if not shared. */ owner: string; token: string; diff --git a/core/persist/viewmanager/ViewManagerModel.ts b/core/persist/viewmanager/ViewManagerModel.ts index 434e200f2..76e12f657 100644 --- a/core/persist/viewmanager/ViewManagerModel.ts +++ b/core/persist/viewmanager/ViewManagerModel.ts @@ -12,11 +12,12 @@ import { ViewManagerProvider, XH } from '@xh/hoist/core'; +import {buildViewTree} from '@xh/hoist/core/persist/viewmanager/impl/BuildViewTree'; import {genDisplayName} from '@xh/hoist/data'; import {action, bindable, computed, makeObservable, observable} from '@xh/hoist/mobx'; import {wait} from '@xh/hoist/promise'; import {executeIfFunction, pluralize, throwIf} from '@xh/hoist/utils/js'; -import {isEmpty, isEqual, isNil, lowerCase, sortBy, startCase} from 'lodash'; +import {isEmpty, isEqual, isNil, lowerCase, startCase} from 'lodash'; import {runInAction} from 'mobx'; import {SaveDialogModel} from './impl/SaveDialogModel'; import {View, ViewTree} from './Types'; @@ -137,11 +138,11 @@ export class ViewManagerModel return executeIfFunction(this._enableSharing); } + @computed get selectedView(): View { return this.views.find(it => it.token === this.selectedToken); } - @computed get isSharedViewSelected(): boolean { return !!this.selectedView?.isShared; } @@ -207,11 +208,11 @@ export class ViewManagerModel } get sharedViewTree(): ViewTree[] { - return this.buildViewTree(sortBy(this.sharedViews, 'name')); + return buildViewTree(this.sharedViews, this); } get privateViewTree(): ViewTree[] { - return this.buildViewTree(sortBy(this.privateViews, 'name')); + return buildViewTree(this.privateViews, this); } /** @@ -364,10 +365,6 @@ export class ViewManagerModel this.manageDialogOpen = false; } - getHierarchyDisplayName(name: string) { - return name?.substring(name.lastIndexOf('\\') + 1); - } - //------------------ // Favorites //------------------ @@ -409,6 +406,7 @@ export class ViewManagerModel const isShared = it.acl === '*'; return { ...it, + shortName: it.name?.substring(it.name.lastIndexOf('\\') + 1), isShared, group: isShared ? `Shared ${name}` : `My ${name}`, isFavorite: this.isFavorite(it.token) @@ -458,60 +456,6 @@ export class ViewManagerModel } } - private buildViewTree(views: View[], depth: number = 0): ViewTree[] { - const groups = {}, - unbalancedStableGroupsAndViews = []; - - views.forEach(view => { - // Leaf Node - if (this.getNameHierarchySubstring(view.name, depth + 1) == null) { - unbalancedStableGroupsAndViews.push(view); - return; - } - // Belongs to an already defined group - const group = this.getNameHierarchySubstring(view.name, depth); - if (groups[group]) { - groups[group].children.push(view); - return; - } - // Belongs to a not defined group, create it - groups[group] = {name: group, children: [view], isMenuFolder: true}; - unbalancedStableGroupsAndViews.push(groups[group]); - }); - - return unbalancedStableGroupsAndViews.map(it => { - const {name, isMenuFolder, children, description, token} = it; - if (isMenuFolder) { - return { - type: 'folder', - text: name, - items: this.buildViewTree(children, depth + 1), - selected: this.isFolderForEntry(name, this.selectedView?.name, depth) - }; - } - return { - type: 'view', - text: this.getHierarchyDisplayName(name), - selected: this.selectedToken === token, - token, - description - }; - }); - } - - private getNameHierarchySubstring(name: string, depth: number) { - const arr = name?.split('\\') ?? []; - if (arr.length <= depth) { - return null; - } - return arr.slice(0, depth + 1).join('\\'); - } - - private isFolderForEntry(folderName: string, entryName: string, depth: number) { - const name = this.getNameHierarchySubstring(entryName, depth); - return name && name === folderName && folderName.length < entryName.length; - } - // Update flag on each view, replacing entire views collection for observability. private onFavoritesChange() { this.views = this.views.map(view => ({ diff --git a/core/persist/viewmanager/impl/BuildViewTree.ts b/core/persist/viewmanager/impl/BuildViewTree.ts new file mode 100644 index 000000000..fcb05daf0 --- /dev/null +++ b/core/persist/viewmanager/impl/BuildViewTree.ts @@ -0,0 +1,68 @@ +import {View, ViewManagerModel, ViewTree} from '@xh/hoist/core/persist/viewmanager'; +import {sortBy} from 'lodash'; + +/** + * Create a menu-friendly, tree representation of a set of views, using the `\` + * in view names to create folders. + * + * @internal + */ +export function buildViewTree(views: View[], model: ViewManagerModel): ViewTree[] { + views = sortBy(views, 'name'); + return buildTreeInternal(views, model.selectedView, 0); +} + +function buildTreeInternal(views: View[], selected: View, depth: number): ViewTree[] { + // 1) Get groups and leaves at this level. + const groups = {}, + groupsAndLeaves = []; + views.forEach(view => { + // Leaf Node + if (getNameAtDepth(view.name, depth + 1) == null) { + groupsAndLeaves.push(view); + return; + } + // Belongs to an already defined group + const group = getNameAtDepth(view.name, depth); + if (groups[group]) { + groups[group].children.push(view); + return; + } + // Belongs to a not defined group, create it + groups[group] = {name: group, children: [view], isMenuFolder: true}; + groupsAndLeaves.push(groups[group]); + }); + + // 2) Make ViewTree, recursing for groups + return groupsAndLeaves.map(it => { + const {name, isMenuFolder, children, description, token} = it; + return isMenuFolder + ? { + type: 'folder', + text: getFolderDisplayName(name, depth), + items: buildTreeInternal(children, selected, depth + 1), + selected: isFolderForEntry(name, selected?.name, depth) + } + : { + type: 'view', + text: it.shortName, + selected: selected?.token === token, + token, + description + }; + }); +} + +function getNameAtDepth(name: string, depth: number) { + const arr = name?.split('\\') ?? []; + return arr.length <= depth ? null : arr.slice(0, depth + 1).join('\\'); +} + +function isFolderForEntry(folderName: string, entryName: string, depth: number) { + const name = getNameAtDepth(entryName, depth); + return name && name === folderName && folderName.length < entryName.length; +} + +function getFolderDisplayName(name: string, depth: number) { + return name?.split('\\')[depth]; +} diff --git a/desktop/cmp/viewmanager/ViewManager.ts b/desktop/cmp/viewmanager/ViewManager.ts index 3f80ab33f..8e44263b9 100644 --- a/desktop/cmp/viewmanager/ViewManager.ts +++ b/desktop/cmp/viewmanager/ViewManager.ts @@ -74,7 +74,7 @@ const menuButton = hoistCmp.factory({ const {selectedView, DisplayName} = model; return button({ className: 'xh-view-manager__menu-button', - text: model.getHierarchyDisplayName(selectedView?.name) ?? `Default ${DisplayName}`, + text: selectedView?.shortName ?? `Default ${DisplayName}`, icon: Icon.bookmark(), rightIcon: Icon.chevronDown(), outlined: true, @@ -119,7 +119,7 @@ const viewMenu = hoistCmp.factory({ key: `${it.token}-favorite`, icon: model.selectedToken === it.token ? Icon.check() : Icon.placeholder(), text: menuItemTextAndFaveToggle({ - view: {...it, text: model.getHierarchyDisplayName(it.name)} + view: {...it, text: it.shortName} }), onClick: () => model.selectViewAsync(it.token).linkTo(model.loadModel), title: it.description From 189c8b4cbbce9635344636e689ff53f7d0906741 Mon Sep 17 00:00:00 2001 From: lbwexler Date: Sat, 16 Nov 2024 20:06:47 -0500 Subject: [PATCH 03/13] + Tweak import --- core/persist/viewmanager/ViewManagerModel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/persist/viewmanager/ViewManagerModel.ts b/core/persist/viewmanager/ViewManagerModel.ts index 76e12f657..f8002773e 100644 --- a/core/persist/viewmanager/ViewManagerModel.ts +++ b/core/persist/viewmanager/ViewManagerModel.ts @@ -12,7 +12,6 @@ import { ViewManagerProvider, XH } from '@xh/hoist/core'; -import {buildViewTree} from '@xh/hoist/core/persist/viewmanager/impl/BuildViewTree'; import {genDisplayName} from '@xh/hoist/data'; import {action, bindable, computed, makeObservable, observable} from '@xh/hoist/mobx'; import {wait} from '@xh/hoist/promise'; @@ -20,6 +19,7 @@ import {executeIfFunction, pluralize, throwIf} from '@xh/hoist/utils/js'; import {isEmpty, isEqual, isNil, lowerCase, startCase} from 'lodash'; import {runInAction} from 'mobx'; import {SaveDialogModel} from './impl/SaveDialogModel'; +import {buildViewTree} from './impl/BuildViewTree'; import {View, ViewTree} from './Types'; export interface ViewManagerConfig { From 6fb5c8a92e9420c4b4754d6e5ff3d3c44c244c55 Mon Sep 17 00:00:00 2001 From: lbwexler Date: Sat, 16 Nov 2024 22:04:07 -0500 Subject: [PATCH 04/13] + Load View Definitions dynamically, when switching to them. + Tweak to type of JsonBlobService. --- core/persist/viewmanager/ViewManagerModel.ts | 86 +++++++++++--------- svc/JsonBlobService.ts | 9 +- 2 files changed, 49 insertions(+), 46 deletions(-) diff --git a/core/persist/viewmanager/ViewManagerModel.ts b/core/persist/viewmanager/ViewManagerModel.ts index f8002773e..605975503 100644 --- a/core/persist/viewmanager/ViewManagerModel.ts +++ b/core/persist/viewmanager/ViewManagerModel.ts @@ -14,9 +14,8 @@ import { } from '@xh/hoist/core'; import {genDisplayName} from '@xh/hoist/data'; import {action, bindable, computed, makeObservable, observable} from '@xh/hoist/mobx'; -import {wait} from '@xh/hoist/promise'; import {executeIfFunction, pluralize, throwIf} from '@xh/hoist/utils/js'; -import {isEmpty, isEqual, isNil, lowerCase, startCase} from 'lodash'; +import {isEqual, isNil, lowerCase, startCase} from 'lodash'; import {runInAction} from 'mobx'; import {SaveDialogModel} from './impl/SaveDialogModel'; import {buildViewTree} from './impl/BuildViewTree'; @@ -106,8 +105,11 @@ export class ViewManagerModel @observable.ref pendingValue: T = {} as T; /** Loaded saved view definitions - both private and shared. */ @observable.ref views: View[] = []; - /** Token identifier for the currently selected view, or null if in default mode. */ - @bindable selectedToken: string = null; + + /** Currently selected view, or null if in default mode. Token only will be set during pre-loading.*/ + @observable selectedToken: string = null; + @observable.ref selectedView: View = null; + /** List of tokens for the user's favorite views. */ @bindable favorites: string[] = []; /** @@ -138,11 +140,6 @@ export class ViewManagerModel return executeIfFunction(this._enableSharing); } - @computed - get selectedView(): View { - return this.views.find(it => it.token === this.selectedToken); - } - get isSharedViewSelected(): boolean { return !!this.selectedView?.isShared; } @@ -273,36 +270,27 @@ export class ViewManagerModel } override async doLoadAsync(loadSpec: LoadSpec) { - const rawViews = await XH.jsonBlobService.listAsync({ - type: this.viewType, - includeValue: true, - loadSpec - }); + const rawViews = await XH.jsonBlobService.listAsync({type: this.viewType, loadSpec}); if (loadSpec.isStale) return; - runInAction(() => (this.views = this.processRaw(rawViews))); + runInAction(() => { + this.views = rawViews.map(it => this.processRaw(it)); + }); const token = loadSpec.meta.selectToken ?? - this.selectedView?.token ?? + this.selectedToken ?? (this.enableDefault ? null : this.views[0]?.token); await this.selectViewAsync(token); } async selectViewAsync(token: string) { - // Introduce minimal wait and link to viewSelectionObserver to allow apps to mask. - await wait(100) - .then(() => { - this.selectedToken = token; - - // Allow this model to restore its own persisted state in its ctor and note the desired - // selected token before views have been loaded. Once views are loaded, this method will - // be called again with the desired token and will proceed to set the value. - if (isEmpty(this.views)) return; - - this.setValue(this.selectedView?.value ?? ({} as T)); - }) - .linkTo(this.viewSelectionObserver); + // If views have not been loaded yet (e.g. constructing), nothing to be done but pre-set state + if (!this.views) { + this.selectedToken = token; + return; + } + await this.selectViewInternalAsync(token).linkTo(this.viewSelectionObserver); } async saveAsync(skipToast: boolean = false) { @@ -400,20 +388,38 @@ export class ViewManagerModel //------------------ // Implementation //------------------ - private processRaw(raw: PlainObject[]): View[] { - const name = pluralize(this.DisplayName); - return raw.map(it => { - const isShared = it.acl === '*'; - return { - ...it, - shortName: it.name?.substring(it.name.lastIndexOf('\\') + 1), - isShared, - group: isShared ? `Shared ${name}` : `My ${name}`, - isFavorite: this.isFavorite(it.token) - } as View; + private async selectViewInternalAsync(token: string) { + let view: View = null; + if (token != null) { + try { + const raw = await XH.jsonBlobService.getAsync(token); + view = this.processRaw(raw); + } catch (e) { + XH.handleException(e, {showAlert: false}); + view = null; + token = null; + } + } + + runInAction(() => { + this.selectedToken = token; + this.selectedView = view; + this.setValue(this.selectedView?.value ?? ({} as T)); }); } + private processRaw(raw: PlainObject): View { + const name = pluralize(this.DisplayName); + const isShared = raw.acl === '*'; + return { + ...raw, + shortName: raw.name?.substring(raw.name.lastIndexOf('\\') + 1), + isShared, + group: isShared ? `Shared ${name}` : `My ${name}`, + isFavorite: this.isFavorite(raw.token) + } as View; + } + @action private setValue(value: T) { value = this.cleanValue(value); diff --git a/svc/JsonBlobService.ts b/svc/JsonBlobService.ts index 0324f6889..023bdc72c 100644 --- a/svc/JsonBlobService.ts +++ b/svc/JsonBlobService.ts @@ -58,15 +58,12 @@ export class JsonBlobService extends HoistService { } /** Retrieve all blobs of a particular type that are visible to the current user. */ - async listAsync({ - type, - includeValue, - loadSpec - }: { + async listAsync(spec: { type: string; includeValue?: boolean; loadSpec?: LoadSpec; - }) { + }): Promise { + const {type, includeValue, loadSpec} = spec; return XH.fetchJson({ url: 'xh/listJsonBlobs', params: {type, includeValue}, From 2376a5d607b201656e9624f13a09e9a46a88dcfd Mon Sep 17 00:00:00 2001 From: lbwexler Date: Sun, 17 Nov 2024 06:18:44 -0500 Subject: [PATCH 05/13] + Tweaks to ManageDialogModel grid --- core/persist/viewmanager/impl/ManageDialogModel.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/persist/viewmanager/impl/ManageDialogModel.ts b/core/persist/viewmanager/impl/ManageDialogModel.ts index 69ead3a7b..176edd2e6 100644 --- a/core/persist/viewmanager/impl/ManageDialogModel.ts +++ b/core/persist/viewmanager/impl/ManageDialogModel.ts @@ -214,6 +214,8 @@ export class ManageDialogModel extends HoistModel { hideHeaders: true, showGroupRowCounts: false, selModel: 'multiple', + contextMenu: null, + sizingMode: 'standard', store: { idSpec: 'token', fields: [ From e6739966986e1b8742dd7d26e2db2cc70e1d4b0a Mon Sep 17 00:00:00 2001 From: lbwexler Date: Sun, 17 Nov 2024 09:52:40 -0500 Subject: [PATCH 06/13] Checkpoint: + Show dirty save button for "Save As" + persist user AutoSave. + related cleanups + Use 'impl' directory --- core/persist/viewmanager/ViewManagerModel.ts | 93 +++++++++---------- desktop/cmp/viewmanager/ViewManager.ts | 84 ++++++++++------- .../viewmanager/{cmp => impl}/ManageDialog.ts | 0 .../viewmanager/{cmp => impl}/SaveDialog.ts | 0 desktop/cmp/viewmanager/index.ts | 4 +- 5 files changed, 95 insertions(+), 86 deletions(-) rename desktop/cmp/viewmanager/{cmp => impl}/ManageDialog.ts (100%) rename desktop/cmp/viewmanager/{cmp => impl}/SaveDialog.ts (100%) diff --git a/core/persist/viewmanager/ViewManagerModel.ts b/core/persist/viewmanager/ViewManagerModel.ts index 605975503..2394f5769 100644 --- a/core/persist/viewmanager/ViewManagerModel.ts +++ b/core/persist/viewmanager/ViewManagerModel.ts @@ -15,7 +15,7 @@ import { import {genDisplayName} from '@xh/hoist/data'; import {action, bindable, computed, makeObservable, observable} from '@xh/hoist/mobx'; import {executeIfFunction, pluralize, throwIf} from '@xh/hoist/utils/js'; -import {isEqual, isNil, lowerCase, startCase} from 'lodash'; +import {isEqual, isNil, isUndefined, lowerCase, startCase} from 'lodash'; import {runInAction} from 'mobx'; import {SaveDialogModel} from './impl/SaveDialogModel'; import {buildViewTree} from './impl/BuildViewTree'; @@ -116,7 +116,7 @@ export class ViewManagerModel * True if user has opted-in to automatically saving changes to personal views (if auto-save * generally available as per `enableAutoSave`). */ - @bindable autoSaveActive = false; + @bindable autoSave = false; /** * TaskObserver linked to {@link selectViewAsync}. If a change to the active view is likely to @@ -140,46 +140,30 @@ export class ViewManagerModel return executeIfFunction(this._enableSharing); } - get isSharedViewSelected(): boolean { - return !!this.selectedView?.isShared; - } - @computed get canSave(): boolean { - const {selectedView} = this; - return ( - selectedView && - this.isDirty && - (this.enableSharing || !selectedView.isShared) && - !this.loadModel.isPending - ); + const {loadModel, selectedView, enableSharing} = this; + return !loadModel.isPending && selectedView && (enableSharing || !selectedView.isShared); } - /** - * True if displaying the save button is appropriate from the model's point of view, even if - * that button might be disabled due to no changes having been made. Works in concert with the - * desktop ViewManager component's `showSaveButton` prop. - */ @computed - get canShowSaveButton(): boolean { - const {selectedView} = this; + get canAutoSave(): boolean { + const {enableAutoSave, autoSave, loadModel, selectedView} = this; return ( + enableAutoSave && + autoSave && + !loadModel.isPending && selectedView && - (!this.enableAutoSave || !this.autoSaveActive) && - (this.enableSharing || !selectedView.isShared) + !selectedView.isShared ); } @computed - get enableAutoSaveToggle(): boolean { - return this.selectedView && !this.isSharedViewSelected; - } - - @computed - get disabledAutoSaveReason(): string { - const {displayName} = this; - if (!this.selectedView) return `Cannot auto-save default ${displayName}.`; - if (this.isSharedViewSelected) return `Cannot auto-save shared ${displayName}.`; + get autoSaveUnavailableReason(): string { + const {canAutoSave, selectedView, displayName} = this; + if (canAutoSave) return null; + if (!selectedView) return `Cannot auto-save default ${displayName}.`; + if (selectedView.isShared) return `Cannot auto-save shared ${displayName}.`; return null; } @@ -255,11 +239,11 @@ export class ViewManagerModel this.addReaction( { - track: () => this.pendingValue, + track: () => this.isDirty, run: () => this.maybeAutoSaveAsync({skipToast: true}) }, { - track: () => this.autoSaveActive, + track: () => this.canAutoSave, run: () => this.maybeAutoSaveAsync({skipToast: false}) }, { @@ -293,11 +277,14 @@ export class ViewManagerModel await this.selectViewInternalAsync(token).linkTo(this.viewSelectionObserver); } + //------------------------ + // Saving/resetting + //------------------------ async saveAsync(skipToast: boolean = false) { - const {canSave, selectedToken, pendingValue, isSharedViewSelected, DisplayName} = this; - throwIf(!canSave, 'Unable to save view at this time.'); // sanity check - user should not reach + const {canSave, selectedToken, pendingValue, selectedView, DisplayName} = this; + throwIf(!canSave, 'Unable to save view.'); - if (isSharedViewSelected) { + if (selectedView?.isShared) { if (!(await this.confirmSaveForSharedViewAsync())) return; } @@ -376,13 +363,27 @@ export class ViewManagerModel // Persistable //------------------ getPersistableState(): PersistableState { - return new PersistableState({selectedToken: this.selectedToken, favorites: this.favorites}); + const state: ViewManagerModelPersistState = { + selectedToken: this.selectedToken, + favorites: this.favorites + }; + if (this.enableAutoSave) { + state.autoSave = this.autoSave; + } + return new PersistableState(state); } setPersistableState(state: PersistableState) { - const {selectedToken, favorites} = state.value; - if (selectedToken) this.selectViewAsync(selectedToken); - if (favorites) this.favorites = favorites; + const {selectedToken, favorites, autoSave} = state.value; + if (!isUndefined(selectedToken)) { + this.selectViewAsync(selectedToken); + } + if (!isUndefined(favorites)) { + this.favorites = favorites; + } + if (!isUndefined(autoSave) && this.enableAutoSave) { + this.autoSave = autoSave; + } } //------------------ @@ -452,12 +453,7 @@ export class ViewManagerModel } private async maybeAutoSaveAsync({skipToast}: {skipToast: boolean}) { - if ( - this.enableAutoSave && - this.autoSaveActive && - this.canSave && - !this.isSharedViewSelected - ) { + if (this.canAutoSave && this.isDirty) { await this.saveAsync(skipToast); } } @@ -472,6 +468,7 @@ export class ViewManagerModel } interface ViewManagerModelPersistState { - selectedToken: string; - favorites: string[]; + selectedToken?: string; + favorites?: string[]; + autoSave?: boolean; } diff --git a/desktop/cmp/viewmanager/ViewManager.ts b/desktop/cmp/viewmanager/ViewManager.ts index b4aeab3d4..dd8f37f3a 100644 --- a/desktop/cmp/viewmanager/ViewManager.ts +++ b/desktop/cmp/viewmanager/ViewManager.ts @@ -5,8 +5,8 @@ import {ViewTree} from '@xh/hoist/core/persist/viewmanager'; import {ViewManagerModel} from '@xh/hoist/core/persist/viewmanager/ViewManagerModel'; import {button, ButtonProps} from '@xh/hoist/desktop/cmp/button'; import {switchInput} from '@xh/hoist/desktop/cmp/input'; -import {manageDialog} from '@xh/hoist/desktop/cmp/viewmanager/cmp/ManageDialog'; -import {saveDialog} from '@xh/hoist/desktop/cmp/viewmanager/cmp/SaveDialog'; +import {manageDialog} from './impl/ManageDialog'; +import {saveDialog} from './impl/SaveDialog'; import {Icon} from '@xh/hoist/icon'; import {menu, menuDivider, menuItem, popover} from '@xh/hoist/kit/blueprint'; import {consumeEvent, pluralize} from '@xh/hoist/utils/js'; @@ -86,9 +86,9 @@ const menuButton = hoistCmp.factory({ const saveButton = hoistCmp.factory({ render({model, showSaveButton, ...rest}) { if ( - !model.canShowSaveButton || showSaveButton === 'never' || - (showSaveButton === 'whenDirty' && !model.isDirty) + (showSaveButton === 'whenDirty' && !model.isDirty) || + model.canAutoSave ) { return null; } @@ -98,8 +98,10 @@ const saveButton = hoistCmp.factory({ icon: Icon.save(), tooltip: `Save changes to this ${model.displayName}`, intent: 'primary', - disabled: !model.canSave, - onClick: () => model.saveAsync(false).linkTo(model.loadModel), + disabled: !model.isDirty, + onClick: () => { + model.canSave ? model.saveAsync() : model.saveAsAsync(); + }, ...rest }); } @@ -107,63 +109,72 @@ const saveButton = hoistCmp.factory({ const viewMenu = hoistCmp.factory({ render({model, showPrivateViewsInSubMenu, showSharedViewsInSubMenu}) { - const {DisplayName} = model, - pluralDisp = pluralize(DisplayName), - items = []; + const { + autoSaveUnavailableReason, + enableDefault, + canSave, + selectedToken, + enableAutoSave, + DisplayName, + autoSave, + privateViewTree, + sharedViewTree, + favoriteViews, + views, + isDirty + } = model; - if (!isEmpty(model.favoriteViews)) { + const pluralDisp = pluralize(DisplayName), + items = []; + if (!isEmpty(favoriteViews)) { items.push( menuDivider({title: 'Favorites'}), - ...model.favoriteViews.map(it => { + ...favoriteViews.map(it => { return menuItem({ key: `${it.token}-favorite`, icon: model.selectedToken === it.token ? Icon.check() : Icon.placeholder(), text: menuItemTextAndFaveToggle({ view: {...it, text: it.shortName} }), - onClick: () => model.selectViewAsync(it.token).linkTo(model.loadModel), + onClick: () => model.selectViewAsync(it.token), title: it.description }); }) ); } - if (!isEmpty(model.privateViewTree)) { + if (!isEmpty(privateViewTree)) { if (showPrivateViewsInSubMenu) { items.push( menuDivider({omit: isEmpty(items)}), menuItem({ text: `My ${pluralDisp}`, shouldDismissPopover: false, - children: model.privateViewTree.map(it => { - return buildMenuItem(it, model); - }) + children: privateViewTree.map(it => buildMenuItem(it, model)) }) ); } else { items.push( menuDivider({title: `My ${pluralDisp}`}), - ...model.privateViewTree.map(it => buildMenuItem(it, model)) + ...privateViewTree.map(it => buildMenuItem(it, model)) ); } } - if (!isEmpty(model.sharedViewTree)) { + if (!isEmpty(sharedViewTree)) { if (showSharedViewsInSubMenu) { items.push( menuDivider({omit: isEmpty(items)}), menuItem({ text: `Shared ${pluralDisp}`, shouldDismissPopover: false, - children: model.sharedViewTree.map(it => { - return buildMenuItem(it, model); - }) + children: sharedViewTree.map(it => buildMenuItem(it, model)) }) ); } else { items.push( menuDivider({title: `Shared ${pluralDisp}`}), - ...model.sharedViewTree.map(it => buildMenuItem(it, model)) + ...sharedViewTree.map(it => buildMenuItem(it, model)) ); } } @@ -172,19 +183,19 @@ const viewMenu = hoistCmp.factory({ className: 'xh-view-manager__menu', items: [ ...items, - menuDivider({omit: !model.enableDefault || isEmpty(items)}), + menuDivider({omit: !enableDefault || isEmpty(items)}), menuItem({ - icon: model.selectedToken ? Icon.placeholder() : Icon.check(), + icon: selectedToken ? Icon.placeholder() : Icon.check(), text: `Default ${DisplayName}`, - omit: !model.enableDefault, + omit: !enableDefault, onClick: () => model.selectViewAsync(null) }), menuDivider(), menuItem({ icon: Icon.save(), text: 'Save', - disabled: !model.canSave, - onClick: () => model.saveAsync(false) + disabled: !canSave || !isDirty, + onClick: () => model.saveAsync() }), menuItem({ icon: Icon.copy(), @@ -194,25 +205,26 @@ const viewMenu = hoistCmp.factory({ menuItem({ icon: Icon.reset(), text: `Revert`, - disabled: !model.isDirty, + disabled: !isDirty, onClick: () => model.resetAsync() }), - menuDivider({omit: !model.enableAutoSave}), + menuDivider({omit: !enableAutoSave}), menuItem({ - omit: !model.enableAutoSave, + omit: !enableAutoSave, text: switchInput({ label: 'Auto Save', - bind: 'autoSaveActive', - inline: true, - disabled: !model.enableAutoSaveToggle + value: !autoSaveUnavailableReason && autoSave, + disabled: !!autoSaveUnavailableReason, + onChange: v => (model.autoSave = v), + inline: true }), - title: model.disabledAutoSaveReason, + title: autoSaveUnavailableReason, shouldDismissPopover: false }), menuDivider(), menuItem({ icon: Icon.gear(), - disabled: isEmpty(model.views), + disabled: isEmpty(views), text: `Manage ${pluralDisp}...`, onClick: () => model.openManageDialog() }) @@ -242,7 +254,7 @@ function buildMenuItem(viewOrFolder: ViewTree, model: ViewManagerModel): ReactNo icon, text: menuItemTextAndFaveToggle({model, view: viewOrFolder}), title: viewOrFolder.description, - onClick: () => model.selectViewAsync(viewOrFolder.token).linkTo(model.loadModel) + onClick: () => model.selectViewAsync(viewOrFolder.token) }); } } diff --git a/desktop/cmp/viewmanager/cmp/ManageDialog.ts b/desktop/cmp/viewmanager/impl/ManageDialog.ts similarity index 100% rename from desktop/cmp/viewmanager/cmp/ManageDialog.ts rename to desktop/cmp/viewmanager/impl/ManageDialog.ts diff --git a/desktop/cmp/viewmanager/cmp/SaveDialog.ts b/desktop/cmp/viewmanager/impl/SaveDialog.ts similarity index 100% rename from desktop/cmp/viewmanager/cmp/SaveDialog.ts rename to desktop/cmp/viewmanager/impl/SaveDialog.ts diff --git a/desktop/cmp/viewmanager/index.ts b/desktop/cmp/viewmanager/index.ts index 764b8f961..9b39918f8 100644 --- a/desktop/cmp/viewmanager/index.ts +++ b/desktop/cmp/viewmanager/index.ts @@ -1,3 +1,3 @@ export * from './ViewManager'; -export * from './cmp/ManageDialog'; -export * from './cmp/SaveDialog'; +export * from '@xh/hoist/desktop/cmp/viewmanager/impl/ManageDialog'; +export * from '@xh/hoist/desktop/cmp/viewmanager/impl/SaveDialog'; From 80ff574558b714f683e5bf37165b0ccce9dd77fe Mon Sep 17 00:00:00 2001 From: lbwexler Date: Sun, 17 Nov 2024 10:44:01 -0500 Subject: [PATCH 07/13] Support for Revert Button --- desktop/cmp/viewmanager/ViewManager.ts | 59 ++++++++++++++++++++------ 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/desktop/cmp/viewmanager/ViewManager.ts b/desktop/cmp/viewmanager/ViewManager.ts index dd8f37f3a..6464caa76 100644 --- a/desktop/cmp/viewmanager/ViewManager.ts +++ b/desktop/cmp/viewmanager/ViewManager.ts @@ -13,11 +13,28 @@ import {consumeEvent, pluralize} from '@xh/hoist/utils/js'; import {isEmpty} from 'lodash'; import {ReactNode} from 'react'; +/** + * Visibility options for save/revert button. + * + * 'never' to hide button. + * 'whenDirty' to only show when persistence state is dirty and button is therefore enabled. + * 'always' will always show button, unless autoSave is active. + * + * Note that we never show the button when 'autoSave' is active because it would never be enabled + * for more than a flash. + */ +export type ViewManagerStateButtonMode = 'whenDirty' | 'always' | 'never'; + export interface ViewManagerProps extends HoistProps { menuButtonProps?: Partial; saveButtonProps?: Partial; - /** 'whenDirty' to only show saveButton when persistence state is dirty. (Default 'whenDirty') */ - showSaveButton?: 'whenDirty' | 'always' | 'never'; + revertButtonProps?: Partial; + + /** Default 'whenDirty' */ + showSaveButton?: ViewManagerStateButtonMode; + /** Default 'never' */ + showRevertButton?: ViewManagerStateButtonMode; + /** True to render private views in sub-menu (Default false)*/ showPrivateViewsInSubMenu?: boolean; /** True to render shared views in sub-menu (Default false)*/ @@ -40,7 +57,9 @@ export const [ViewManager, viewManager] = hoistCmp.withFactory className, menuButtonProps, saveButtonProps, + revertButtonProps, showSaveButton = 'whenDirty', + showRevertButton = 'never', showPrivateViewsInSubMenu = false, showSharedViewsInSubMenu = false }: ViewManagerProps) { @@ -55,8 +74,12 @@ export const [ViewManager, viewManager] = hoistCmp.withFactory popoverClassName: 'xh-view-manager__popover' }), saveButton({ - showSaveButton, + mode: showSaveButton, ...saveButtonProps + }), + revertButton({ + mode: showRevertButton, + ...revertButtonProps }) ] }), @@ -84,15 +107,8 @@ const menuButton = hoistCmp.factory({ }); const saveButton = hoistCmp.factory({ - render({model, showSaveButton, ...rest}) { - if ( - showSaveButton === 'never' || - (showSaveButton === 'whenDirty' && !model.isDirty) || - model.canAutoSave - ) { - return null; - } - + render({model, mode, ...rest}) { + if (hideStateButton(model, mode)) return null; return button({ className: 'xh-view-manager__save-button', icon: Icon.save(), @@ -107,6 +123,25 @@ const saveButton = hoistCmp.factory({ } }); +const revertButton = hoistCmp.factory({ + render({model, mode, ...rest}) { + if (hideStateButton(model, mode)) return null; + return button({ + className: 'xh-view-manager__revert-button', + icon: Icon.reset(), + tooltip: `Revert changes to this ${model.displayName}`, + intent: 'danger', + disabled: !model.isDirty, + onClick: () => model.resetAsync(), + ...rest + }); + } +}); + +function hideStateButton(model: ViewManagerModel, mode: ViewManagerStateButtonMode): boolean { + return mode === 'never' || (mode === 'whenDirty' && !model.isDirty) || model.canAutoSave; +} + const viewMenu = hoistCmp.factory({ render({model, showPrivateViewsInSubMenu, showSharedViewsInSubMenu}) { const { From 265bd2c2f60f675e040033bee3dc3c64a09cf2be Mon Sep 17 00:00:00 2001 From: lbwexler Date: Mon, 18 Nov 2024 09:27:27 -0500 Subject: [PATCH 08/13] Split-apart Autosave and Save handling. Do not do full refresh of view for either. --- core/persist/viewmanager/ViewManagerModel.ts | 35 +++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/core/persist/viewmanager/ViewManagerModel.ts b/core/persist/viewmanager/ViewManagerModel.ts index 2394f5769..e2123d615 100644 --- a/core/persist/viewmanager/ViewManagerModel.ts +++ b/core/persist/viewmanager/ViewManagerModel.ts @@ -239,12 +239,9 @@ export class ViewManagerModel this.addReaction( { - track: () => this.isDirty, - run: () => this.maybeAutoSaveAsync({skipToast: true}) - }, - { - track: () => this.canAutoSave, - run: () => this.maybeAutoSaveAsync({skipToast: false}) + // Track pendingValue, so we retry on fail if view stays dirty -- could use backup timer + track: () => [this.pendingValue, this.autoSave], + run: () => this.maybeAutoSaveAsync() }, { track: () => this.favorites, @@ -280,7 +277,7 @@ export class ViewManagerModel //------------------------ // Saving/resetting //------------------------ - async saveAsync(skipToast: boolean = false) { + async saveAsync() { const {canSave, selectedToken, pendingValue, selectedView, DisplayName} = this; throwIf(!canSave, 'Unable to save view.'); @@ -290,13 +287,13 @@ export class ViewManagerModel try { await XH.jsonBlobService.updateAsync(selectedToken, {value: pendingValue}); + runInAction(() => { + this.value = this.pendingValue; + }); + XH.successToast(`${DisplayName} successfully saved.`); } catch (e) { XH.handleException(e, {alertType: 'toast'}); - skipToast = true; // don't show the success toast below, but still refresh. } - - await this.refreshAsync({selectToken: selectedToken}); - if (!skipToast) XH.successToast(`${DisplayName} successfully saved.`); } async saveAsAsync() { @@ -421,13 +418,11 @@ export class ViewManagerModel } as View; } - @action private setValue(value: T) { value = this.cleanValue(value); if (isEqual(value, this.value) && isEqual(value, this.pendingValue)) return; - this.value = value; - this.pendingValue = value; + this.value = this.pendingValue = value; this.providers.forEach(it => it.pushStateToTarget()); } @@ -452,9 +447,17 @@ export class ViewManagerModel }); } - private async maybeAutoSaveAsync({skipToast}: {skipToast: boolean}) { + private async maybeAutoSaveAsync() { if (this.canAutoSave && this.isDirty) { - await this.saveAsync(skipToast); + const {selectedToken, pendingValue} = this; + try { + await XH.jsonBlobService.updateAsync(selectedToken, {value: pendingValue}); + runInAction(() => { + this.value = this.pendingValue; + }); + } catch (e) { + XH.handleException(e, {showAlert: false}); + } } } From fa2ab414bbdd061c99be6c6c7bb69347ff122830 Mon Sep 17 00:00:00 2001 From: lbwexler Date: Mon, 18 Nov 2024 10:40:50 -0500 Subject: [PATCH 09/13] Feedback from CR --- desktop/cmp/viewmanager/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/desktop/cmp/viewmanager/index.ts b/desktop/cmp/viewmanager/index.ts index 9b39918f8..df8c5416b 100644 --- a/desktop/cmp/viewmanager/index.ts +++ b/desktop/cmp/viewmanager/index.ts @@ -1,3 +1 @@ export * from './ViewManager'; -export * from '@xh/hoist/desktop/cmp/viewmanager/impl/ManageDialog'; -export * from '@xh/hoist/desktop/cmp/viewmanager/impl/SaveDialog'; From 8534ba8f51faeb1256d22f54341d70af57b3e38d Mon Sep 17 00:00:00 2001 From: Anselm McClain Date: Mon, 18 Nov 2024 10:47:12 -0800 Subject: [PATCH 10/13] Fix closing manage dialog when multi-select active --- desktop/cmp/viewmanager/impl/ManageDialog.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/cmp/viewmanager/impl/ManageDialog.ts b/desktop/cmp/viewmanager/impl/ManageDialog.ts index dc25f3ea8..934f18d69 100644 --- a/desktop/cmp/viewmanager/impl/ManageDialog.ts +++ b/desktop/cmp/viewmanager/impl/ManageDialog.ts @@ -59,7 +59,7 @@ const formPanel = hoistCmp.factory({ isOwnView = values.owner === XH.getUsername(); if (model.hasMultiSelection) { - return multiSelectionPanel(); + return multiSelectionPanel({onClose}); } if (!model.selectedId) From 77e6dc0c04fd5a2239730f24a8d6557ff5b24026 Mon Sep 17 00:00:00 2001 From: Anselm McClain Date: Mon, 18 Nov 2024 11:35:14 -0800 Subject: [PATCH 11/13] Don't append (COPY) in save-as dialog + Validation will ensure that users choose a new unique name before saving. + Adding "(COPY)" does not serve any particular instances - a user will need to delete it (extra typing) or will leave it in place, resulting in sub-optimal view names. --- core/persist/viewmanager/impl/SaveDialogModel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/persist/viewmanager/impl/SaveDialogModel.ts b/core/persist/viewmanager/impl/SaveDialogModel.ts index 141473b19..d4c39e3ed 100644 --- a/core/persist/viewmanager/impl/SaveDialogModel.ts +++ b/core/persist/viewmanager/impl/SaveDialogModel.ts @@ -40,7 +40,7 @@ export class SaveDialogModel extends HoistModel { this.invalidNames = invalidNames; this.formModel.init({ - name: viewStub.name ? `${viewStub.name} (COPY)` : '', + name: viewStub.name ?? '', description: viewStub.description }); From 2328dd4efb8709ad819e67871edcc9b66b6e93ee Mon Sep 17 00:00:00 2001 From: lbwexler Date: Mon, 18 Nov 2024 18:04:52 -0500 Subject: [PATCH 12/13] Feedback from CR --- core/persist/viewmanager/ViewManagerModel.ts | 10 +++++++--- desktop/cmp/viewmanager/ViewManager.ts | 6 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/core/persist/viewmanager/ViewManagerModel.ts b/core/persist/viewmanager/ViewManagerModel.ts index e2123d615..eb3e1094a 100644 --- a/core/persist/viewmanager/ViewManagerModel.ts +++ b/core/persist/viewmanager/ViewManagerModel.ts @@ -104,7 +104,7 @@ export class ViewManagerModel /** Current state of the active view, can include not-yet-persisted changes. */ @observable.ref pendingValue: T = {} as T; /** Loaded saved view definitions - both private and shared. */ - @observable.ref views: View[] = []; + @observable.ref views: View[] = null; /** Currently selected view, or null if in default mode. Token only will be set during pre-loading.*/ @observable selectedToken: string = null; @@ -150,9 +150,9 @@ export class ViewManagerModel get canAutoSave(): boolean { const {enableAutoSave, autoSave, loadModel, selectedView} = this; return ( + !loadModel.isPending && enableAutoSave && autoSave && - !loadModel.isPending && selectedView && !selectedView.isShared ); @@ -251,7 +251,11 @@ export class ViewManagerModel } override async doLoadAsync(loadSpec: LoadSpec) { - const rawViews = await XH.jsonBlobService.listAsync({type: this.viewType, loadSpec}); + const rawViews = await XH.jsonBlobService.listAsync({ + type: this.viewType, + includeValue: false, + loadSpec + }); if (loadSpec.isStale) return; runInAction(() => { diff --git a/desktop/cmp/viewmanager/ViewManager.ts b/desktop/cmp/viewmanager/ViewManager.ts index 6464caa76..bcacdf21f 100644 --- a/desktop/cmp/viewmanager/ViewManager.ts +++ b/desktop/cmp/viewmanager/ViewManager.ts @@ -185,7 +185,7 @@ const viewMenu = hoistCmp.factory({ menuItem({ text: `My ${pluralDisp}`, shouldDismissPopover: false, - children: privateViewTree.map(it => buildMenuItem(it, model)) + items: privateViewTree.map(it => buildMenuItem(it, model)) }) ); } else { @@ -203,7 +203,7 @@ const viewMenu = hoistCmp.factory({ menuItem({ text: `Shared ${pluralDisp}`, shouldDismissPopover: false, - children: sharedViewTree.map(it => buildMenuItem(it, model)) + items: sharedViewTree.map(it => buildMenuItem(it, model)) }) ); } else { @@ -278,7 +278,7 @@ function buildMenuItem(viewOrFolder: ViewTree, model: ViewManagerModel): ReactNo text, icon, shouldDismissPopover: false, - children: viewOrFolder.items + items: viewOrFolder.items ? viewOrFolder.items.map(child => buildMenuItem(child, model)) : [] }); From a9ff7e7f875c61000fcfc1a533732dc7c5268033 Mon Sep 17 00:00:00 2001 From: lbwexler Date: Mon, 18 Nov 2024 18:08:00 -0500 Subject: [PATCH 13/13] Provisional message for Changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 814f3fc5c..e864b7633 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## v71.0.0-SNAPSHOT - unreleased + +### ⚙️ Technical +* Misc. Improvements to ViewManager + + ## v70.0.0 - 2024-11-15 ### 💥 Breaking Changes (upgrade difficulty: 🟢 LOW - changes to advanced persistence APIs)