From cb88a22a7631ce9f7dcc97553389644b768bfab8 Mon Sep 17 00:00:00 2001 From: Denys Vuika Date: Thu, 7 Jun 2018 23:28:01 +0100 Subject: [PATCH] [ADF-2859] conditional evaluation of disabled state for content actions (#3450) * react on [disabled] binding changes * [disabled] binding updates for context menu items * evaluating disabled state with a function * unit test * restore original description * remove irrelevant test * fix tests --- .../app/components/files/files.component.html | 6 ++ .../app/components/files/files.component.ts | 11 ++ .../content-action.component.md | 84 ++++++++++++++- .../content-action.component.ts | 11 +- .../document-list.component.spec.ts | 30 +++++- .../components/document-list.component.ts | 14 ++- .../models/content-action.model.ts | 7 +- .../context-menu-holder.component.ts | 100 ++++++++---------- .../datatable/datatable.component.spec.ts | 14 --- .../datatable/datatable.component.ts | 15 +-- 10 files changed, 199 insertions(+), 93 deletions(-) diff --git a/demo-shell/src/app/components/files/files.component.html b/demo-shell/src/app/components/files/files.component.html index a82f279edd..adb0f167f5 100644 --- a/demo-shell/src/app/components/files/files.component.html +++ b/demo-shell/src/app/components/files/files.component.html @@ -279,6 +279,12 @@ + + { + if (node && node.entry && node.entry.name === 'custom') { + return false; + } + return true; + } + + runCustomAction(event) { + console.log(event); + } } diff --git a/docs/content-services/content-action.component.md b/docs/content-services/content-action.component.md index b6e2d79536..87bbee1ea7 100644 --- a/docs/content-services/content-action.component.md +++ b/docs/content-services/content-action.component.md @@ -87,11 +87,11 @@ export class MyView { | Name | Type | Default value | Description | | -- | -- | -- | -- | | disableWithNoPermission | `boolean` | | Should this action be disabled in the menu if the user doesn't have permission for it? | -| disabled | `boolean` | false | Is the menu item disabled? | +| disabled | `boolean \|Function` | false | Is the menu item disabled? | | handler | `string` | | System actions. Can be "delete", "download", "copy" or "move". | | icon | `string` | | The name of the icon to display next to the menu command (can be left blank). | | permission | `string` | | The permission type. | -| target | `string` | [`ContentActionTarget`](../../lib/content-services/document-list/models/content-action.model.ts).All | Type of item that the action appies to. Can be "document" or "folder" | +| target | `string` | [`ContentActionTarget`](../../lib/content-services/document-list/models/content-action.model.ts).All | Type of item that the action applies to. Can be "document" or "folder" | | title | `string` | "Action" | The title of the action as shown in the menu. | | visible | `boolean \| Function` | true | Visibility state (see examples). | @@ -393,7 +393,85 @@ Please note that if you want to preserve `this` context within the evaluator fun its property should be declared as a lambda one: ```ts -functionName = (parameters): boolean => { +funcName = (parameters): boolean => { + // implementation + return true; +} +``` + +### Conditional disabled state + +Similar to `visible` property, it is possible to control the `disabled` state with the following scenarios: + +- direct value of `boolean` type +- binding to a property of the `boolean` type +- binding to a property of the `Function` type that evaluates condition and returns `boolean` value + +#### Using direct value of boolean type + +```html + + +``` + +#### Using a property of the boolean type + +```html + + +``` + +The markup above relies on the `shouldDisableAction` property declared at your component class level: + +```ts +export class MyComponent { + + @Input() + shouldDisableAction = true; + +} +``` + +#### Using a property of the Function type + +```html + + +``` + +The code above relies on the `isCustomActionDisabled` property of a `Function` type declared at your component class level: + +```ts +export class MyComponent { + + isCustomActionDisabled = (node: MinimalNodeEntity): boolean => { + if (node && node.entry && node.entry.name === 'custom') { + return false; + } + return true; + } +} +``` + +Code above checks the node name, and evaluates to `true` only if corresponding node is called "custom". + +Please note that if you want to preserve `this` context within the evaluator function, +its property should be declared as a lambda one: + +```ts +funcName = (parameters): boolean => { // implementation return true; } diff --git a/lib/content-services/document-list/components/content-action/content-action.component.ts b/lib/content-services/document-list/components/content-action/content-action.component.ts index 904b2f4141..b599826e83 100644 --- a/lib/content-services/document-list/components/content-action/content-action.component.ts +++ b/lib/content-services/document-list/components/content-action/content-action.component.ts @@ -66,7 +66,7 @@ export class ContentActionComponent implements OnInit, OnChanges, OnDestroy { /** Is the menu item disabled? */ @Input() - disabled: boolean = false; + disabled: boolean | Function = false; /** Emitted when the user selects the action from the menu. */ @Output() @@ -117,6 +117,15 @@ export class ContentActionComponent implements OnInit, OnChanges, OnDestroy { this.folderActionModel.visible = changes.visible.currentValue; } } + + if (changes.disabled && !changes.disabled.firstChange) { + if (this.documentActionModel) { + this.documentActionModel.disabled = changes.disabled.currentValue; + } + if (this.folderActionModel) { + this.folderActionModel.disabled = changes.disabled.currentValue; + } + } } ngOnDestroy() { diff --git a/lib/content-services/document-list/components/document-list.component.spec.ts b/lib/content-services/document-list/components/document-list.component.spec.ts index 3baf6345d7..e169a1c910 100644 --- a/lib/content-services/document-list/components/document-list.component.spec.ts +++ b/lib/content-services/document-list/components/document-list.component.spec.ts @@ -359,6 +359,28 @@ describe('DocumentList', () => { expect(actions[0].title).toBe('Action1'); }); + it('should evaluate conditional disabled state for content action', () => { + documentList.actions = [ + new ContentActionModel({ + target: 'document', + title: 'Action1', + disabled: (): boolean => true + }), + new ContentActionModel({ + target: 'document', + title: 'Action2', + disabled: (): boolean => false + }) + ]; + + const nodeFile = { entry: { isFile: true, name: 'xyz' } }; + const actions = documentList.getNodeActions(nodeFile); + + expect(actions.length).toBe(2); + expect(actions[0].disabled).toBeTruthy(); + expect(actions[1].disabled).toBeFalsy(); + }); + it('should not disable the action if there is copy permission', () => { let documentMenu = new ContentActionModel({ disableWithNoPermission: true, @@ -418,7 +440,7 @@ describe('DocumentList', () => { let actions = documentList.getNodeActions(nodeFile); expect(actions.length).toBe(1); expect(actions[0].title).toEqual('FileAction'); - expect(actions[0].disabled).toBeUndefined(); + expect(actions[0].disabled).toBeFalsy(); }); it('should not disable the action if there is the right permission for the folder', () => { @@ -438,7 +460,7 @@ describe('DocumentList', () => { let actions = documentList.getNodeActions(nodeFile); expect(actions.length).toBe(1); expect(actions[0].title).toEqual('FolderAction'); - expect(actions[0].disabled).toBeUndefined(); + expect(actions[0].disabled).toBeFalsy(); }); it('should not disable the action if there are no permissions for the file and disable with no permission is false', () => { @@ -458,7 +480,7 @@ describe('DocumentList', () => { let actions = documentList.getNodeActions(nodeFile); expect(actions.length).toBe(1); expect(actions[0].title).toEqual('FileAction'); - expect(actions[0].disabled).toBeUndefined(); + expect(actions[0].disabled).toBeFalsy(); }); it('should not disable the action if there are no permissions for the folder and disable with no permission is false', () => { @@ -478,7 +500,7 @@ describe('DocumentList', () => { let actions = documentList.getNodeActions(nodeFile); expect(actions.length).toBe(1); expect(actions[0].title).toEqual('FolderAction'); - expect(actions[0].disabled).toBeUndefined(); + expect(actions[0].disabled).toBeFalsy(); }); it('should disable the action if there are no permissions for the file and disable with no permission is true', () => { diff --git a/lib/content-services/document-list/components/document-list.component.ts b/lib/content-services/document-list/components/document-list.component.ts index d27405647b..c8fba28479 100644 --- a/lib/content-services/document-list/components/document-list.component.ts +++ b/lib/content-services/document-list/components/document-list.component.ts @@ -465,7 +465,7 @@ export class DocumentListComponent implements OnInit, OnChanges, OnDestroy, Afte .map(action => new ContentActionModel(action)); actionsByTarget.forEach((action) => { - this.disableActionsWithNoPermissions(node, action); + action.disabled = this.isActionDisabled(action, node); }); return actionsByTarget; @@ -475,10 +475,16 @@ export class DocumentListComponent implements OnInit, OnChanges, OnDestroy, Afte return []; } - disableActionsWithNoPermissions(node: MinimalNodeEntity, action: ContentActionModel) { - if (action.permission && action.disableWithNoPermission && !this.contentService.hasPermission(node.entry, action.permission)) { - action.disabled = true; + private isActionDisabled(action: ContentActionModel, node: MinimalNodeEntity): boolean { + if (typeof action.disabled === 'function') { + return action.disabled(node); } + + if (action.permission && action.disableWithNoPermission && !this.contentService.hasPermission(node.entry, action.permission)) { + return true; + } + + return false; } @HostListener('contextmenu', ['$event']) diff --git a/lib/content-services/document-list/models/content-action.model.ts b/lib/content-services/document-list/models/content-action.model.ts index 241b3da8b6..693e16f7de 100644 --- a/lib/content-services/document-list/models/content-action.model.ts +++ b/lib/content-services/document-list/models/content-action.model.ts @@ -23,7 +23,7 @@ export class ContentActionModel { target: string; permission: string; disableWithNoPermission: boolean = false; - disabled: boolean = false; + disabled: boolean | Function = false; visible: boolean | Function = true; constructor(obj?: any) { @@ -35,7 +35,10 @@ export class ContentActionModel { this.target = obj.target; this.permission = obj.permission; this.disableWithNoPermission = obj.disableWithNoPermission; - this.disabled = obj.disabled; + + if (obj.hasOwnProperty('disabled')) { + this.disabled = obj.disabled; + } if (obj.hasOwnProperty('visible')) { this.visible = obj.visible; diff --git a/lib/core/context-menu/context-menu-holder.component.ts b/lib/core/context-menu/context-menu-holder.component.ts index 933a2ebb5e..523190e045 100644 --- a/lib/core/context-menu/context-menu-holder.component.ts +++ b/lib/core/context-menu/context-menu-holder.component.ts @@ -29,15 +29,12 @@ import { ContextMenuService } from './context-menu.service'; template: ` - ` @@ -47,9 +44,7 @@ export class ContextMenuHolderComponent implements OnInit, OnDestroy { private mouseLocation: { left: number, top: number } = {left: 0, top: 0}; private menuElement = null; - private openSubscription: Subscription; - private closeSubscription: Subscription; - private contextSubscription: Subscription; + private subscriptions: Subscription[] = []; private contextMenuListenerFn: () => void; @Input() @@ -68,7 +63,7 @@ export class ContextMenuHolderComponent implements OnInit, OnDestroy { @HostListener('window:resize', ['$event']) onResize(event) { if (this.mdMenuElement) { - this.setPositionAfterCDKrecalculation(); + this.updatePosition(); } } @@ -80,33 +75,36 @@ export class ContextMenuHolderComponent implements OnInit, OnDestroy { ) {} ngOnInit() { - this.contextSubscription = this.contextMenuService.show.subscribe(e => this.showMenu(e.event, e.obj)); + this.subscriptions.push( + this.contextMenuService.show.subscribe(e => this.showMenu(e.event, e.obj)), - this.openSubscription = this.menuTrigger.onMenuOpen.subscribe(() => { - const container = this.overlayContainer.getContainerElement(); - if (container) { - this.contextMenuListenerFn = this.renderer.listen(container, 'contextmenu', (e: Event) => { - e.preventDefault(); - }); - } - this.menuElement = this.getContextMenuElement(); - }); + this.menuTrigger.onMenuOpen.subscribe(() => { + const container = this.overlayContainer.getContainerElement(); + if (container) { + this.contextMenuListenerFn = this.renderer.listen(container, 'contextmenu', (e: Event) => { + e.preventDefault(); + }); + } + this.menuElement = this.getContextMenuElement(); + }), - this.closeSubscription = this.menuTrigger.onMenuClose.subscribe(() => { - this.menuElement = null; - if (this.contextMenuListenerFn) { - this.contextMenuListenerFn(); - } - }); + this.menuTrigger.onMenuClose.subscribe(() => { + this.menuElement = null; + if (this.contextMenuListenerFn) { + this.contextMenuListenerFn(); + } + }) + ); } ngOnDestroy() { if (this.contextMenuListenerFn) { this.contextMenuListenerFn(); } - this.contextSubscription.unsubscribe(); - this.openSubscription.unsubscribe(); - this.closeSubscription.unsubscribe(); + + this.subscriptions.forEach(subscription => subscription.unsubscribe()); + this.subscriptions = []; + this.menuElement = null; } @@ -132,16 +130,10 @@ export class ContextMenuHolderComponent implements OnInit, OnDestroy { this.menuTrigger.openMenu(); if (this.mdMenuElement) { - this.setPositionAfterCDKrecalculation(); + this.updatePosition(); } } - setPositionAfterCDKrecalculation() { - setTimeout(() => { - this.setPosition(); - }, 0); - } - get mdMenuElement() { return this.menuElement; } @@ -153,22 +145,24 @@ export class ContextMenuHolderComponent implements OnInit, OnDestroy { }; } - private setPosition() { - if (this.mdMenuElement.clientWidth + this.mouseLocation.left > this.viewport.getViewportRect().width) { - this.menuTrigger.menu.xPosition = 'before'; - this.mdMenuElement.parentElement.style.left = this.mouseLocation.left - this.mdMenuElement.clientWidth + 'px'; - } else { - this.menuTrigger.menu.xPosition = 'after'; - this.mdMenuElement.parentElement.style.left = this.locationCss().left; - } + private updatePosition() { + setTimeout(() => { + if (this.mdMenuElement.clientWidth + this.mouseLocation.left > this.viewport.getViewportRect().width) { + this.menuTrigger.menu.xPosition = 'before'; + this.mdMenuElement.parentElement.style.left = this.mouseLocation.left - this.mdMenuElement.clientWidth + 'px'; + } else { + this.menuTrigger.menu.xPosition = 'after'; + this.mdMenuElement.parentElement.style.left = this.locationCss().left; + } - if (this.mdMenuElement.clientHeight + this.mouseLocation.top > this.viewport.getViewportRect().height) { - this.menuTrigger.menu.yPosition = 'above'; - this.mdMenuElement.parentElement.style.top = this.mouseLocation.top - this.mdMenuElement.clientHeight + 'px'; - } else { - this.menuTrigger.menu.yPosition = 'below'; - this.mdMenuElement.parentElement.style.top = this.locationCss().top; - } + if (this.mdMenuElement.clientHeight + this.mouseLocation.top > this.viewport.getViewportRect().height) { + this.menuTrigger.menu.yPosition = 'above'; + this.mdMenuElement.parentElement.style.top = this.mouseLocation.top - this.mdMenuElement.clientHeight + 'px'; + } else { + this.menuTrigger.menu.yPosition = 'below'; + this.mdMenuElement.parentElement.style.top = this.locationCss().top; + } + }, 0); } private getContextMenuElement() { diff --git a/lib/core/datatable/components/datatable/datatable.component.spec.ts b/lib/core/datatable/components/datatable/datatable.component.spec.ts index 3c85fac7a5..e2873a5927 100644 --- a/lib/core/datatable/components/datatable/datatable.component.spec.ts +++ b/lib/core/datatable/components/datatable/datatable.component.spec.ts @@ -952,20 +952,6 @@ fdescribe('DataTable', () => { expect(dataTable.getCellTooltip(row, col)).toBeNull(); }); - it('should cache the rows menu', () => { - let emitted = 0; - dataTable.showRowActionsMenu.subscribe(() => { emitted++; }); - - const column = {}; - const row = { getValue: function (key: string) { return 'id'; } }; - - dataTable.getRowActions(row, column); - dataTable.getRowActions(row, column); - dataTable.getRowActions(row, column); - - expect(emitted).toBe(1); - }); - it('should reset the menu cache after rows change', () => { let emitted = 0; dataTable.showRowActionsMenu.subscribe(() => { emitted++; }); diff --git a/lib/core/datatable/components/datatable/datatable.component.ts b/lib/core/datatable/components/datatable/datatable.component.ts index 5a08bfd408..bdf7499cd4 100644 --- a/lib/core/datatable/components/datatable/datatable.component.ts +++ b/lib/core/datatable/components/datatable/datatable.component.ts @@ -166,8 +166,6 @@ export class DataTableComponent implements AfterContentInit, OnChanges, DoCheck, private click$: Observable; private differ: any; - private rowMenuCache: object = {}; - private subscriptions: Subscription[] = []; private singleClickStreamSub: Subscription; private multiClickStreamSub: Subscription; @@ -299,7 +297,6 @@ export class DataTableComponent implements AfterContentInit, OnChanges, DoCheck, private initTable() { this.data = new ObjectDataTableAdapter(this.rows, this.columns); this.setupData(this.data); - this.rowMenuCache = {}; } private setupData(adapter: DataTableAdapter) { @@ -556,15 +553,9 @@ export class DataTableComponent implements AfterContentInit, OnChanges, DoCheck, } getRowActions(row: DataRow, col: DataColumn): any[] { - const id = row.getValue('id'); - - if (!this.rowMenuCache[id]) { - let event = new DataCellEvent(row, col, []); - this.showRowActionsMenu.emit(event); - this.rowMenuCache[id] = event.value.actions; - } - - return this.rowMenuCache[id]; + let event = new DataCellEvent(row, col, []); + this.showRowActionsMenu.emit(event); + return event.value.actions; } onExecuteRowAction(row: DataRow, action: any) {