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

[ACS-9158] Remove 'View Details' button from node Details page #4351

Open
wants to merge 2 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
3 changes: 2 additions & 1 deletion docs/extending/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ for example mixing `core.every` and `core.not`.
`!app.navigation.isTrashcan` is the opposite of the `app.navigation.isTrashcan`.

| Version | Key | Description |
| ------- | --------------------------------- | ---------------------------------------------------------------- |
|---------|-----------------------------------|------------------------------------------------------------------|
| 1.7.0 | app.navigation.folder.canCreate | User can create content in the currently opened folder. |
| 1.7.0 | app.navigation.folder.canUpload | User can upload content to the currently opened folder. |
| 1.7.0 | app.navigation.isTrashcan | User is using the **Trashcan** page. |
Expand All @@ -269,6 +269,7 @@ for example mixing `core.every` and `core.not`.
| 1.7.0 | app.navigation.isPreview | Current page is **Preview**. |
| 1.7.0 | app.navigation.isPersonalFiles | Current page is **Personal Files**. |
| 1.7.0 | app.navigation.isLibraryFiles | Current page is **Library Files**. |
| 5.3.0 | app.navigation.isNotDetails | Current page is not **Details**. |

**Tip:** See the [Registration](./registration) section for more details
on how to register your own entries to be re-used at runtime.
Expand Down
1 change: 1 addition & 0 deletions projects/aca-content/src/lib/aca-content.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ export class ContentServiceExtensionModule {
'app.navigation.isSharedPreview': rules.isSharedPreview,
'app.navigation.isFavoritesPreview': rules.isFavoritesPreview,
'app.navigation.isSharedFileViewer': rules.isSharedFileViewer,
'app.navigation.isNotDetails': rules.isNotDetails,

'repository.isQuickShareEnabled': rules.hasQuickShareEnabled,
'user.isAdmin': rules.isAdmin,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { AppTestingModule } from '../../testing/app-testing.module';
import { DetailsComponent } from './details.component';
import { ActivatedRoute, NavigationStart, Router } from '@angular/router';
import { ActivatedRoute } from '@angular/router';
import { BehaviorSubject, of, Subject } from 'rxjs';
import { NO_ERRORS_SCHEMA } from '@angular/core';
import { DefaultProjectorFn, MemoizedSelector, Store } from '@ngrx/store';
import { Store } from '@ngrx/store';
import { ContentApiService } from '@alfresco/aca-shared';
import { AppStore, isInfoDrawerOpened, NavigateToFolder, NavigateToPreviousPage, SetSelectedNodesAction } from '@alfresco/aca-shared/store';
import { NavigateToFolder, SetSelectedNodesAction } from '@alfresco/aca-shared/store';
import { Node, NodeEntry, PathElement } from '@alfresco/js-api';
import { RouterTestingModule } from '@angular/router/testing';
import { AuthenticationService, CORE_PIPES, PageTitleService } from '@alfresco/adf-core';
Expand Down Expand Up @@ -258,45 +258,4 @@ describe('DetailsComponent', () => {
component.setActiveTab('permissions');
expect(component.activeTab).not.toBe(2);
});

describe('infoDrawerOpened$ event', () => {
let infoDrawerOpened$: Subject<boolean>;

beforeEach(() => {
infoDrawerOpened$ = new Subject<boolean>();
spyOn(store, 'select').and.callFake((mapFn: MemoizedSelector<AppStore, boolean, DefaultProjectorFn<boolean>>) =>
mapFn === isInfoDrawerOpened ? infoDrawerOpened$ : mockStream
);
});

it('should dispatch store NavigateToPreviousPage by store if info drawer is closed', () => {
component.ngOnInit();

infoDrawerOpened$.next(false);
expect(storeMock.dispatch).toHaveBeenCalledWith(jasmine.any(NavigateToPreviousPage));
});

it('should not dispatch store NavigateToPreviousPage by store if info drawer is opened', () => {
component.ngOnInit();

infoDrawerOpened$.next(true);
expect(storeMock.dispatch).not.toHaveBeenCalledWith(jasmine.any(NavigateToPreviousPage));
});

it('should not dispatch store NavigateToPreviousPage by store if info drawer opening state is not changed', () => {
component.ngOnInit();

expect(storeMock.dispatch).not.toHaveBeenCalledWith(jasmine.any(NavigateToPreviousPage));
});

it('should not dispatch store NavigateToPreviousPage by store if info drawer is closed but there occurred NavigationStart event', () => {
Object.defineProperty(TestBed.inject(Router), 'events', {
value: of(new NavigationStart(1, ''))
});
component.ngOnInit();

infoDrawerOpened$.next(false);
expect(storeMock.dispatch).not.toHaveBeenCalledWith(jasmine.any(NavigateToPreviousPage));
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@
*/

import { Component, OnDestroy, OnInit, ViewEncapsulation } from '@angular/core';
import { ActivatedRoute, NavigationStart } from '@angular/router';
import { ActivatedRoute } from '@angular/router';
import { ContentApiService, PageComponent, PageLayoutComponent, ToolbarComponent } from '@alfresco/aca-shared';
import { NavigateToFolder, NavigateToPreviousPage, SetSelectedNodesAction } from '@alfresco/aca-shared/store';
import { merge, Subject } from 'rxjs';
import { BreadcrumbComponent, ContentService, NodesApiService, PermissionListComponent } from '@alfresco/adf-content-services';
import { CommonModule } from '@angular/common';
import { TranslateModule } from '@ngx-translate/core';
Expand All @@ -37,7 +36,7 @@ import { MatButtonModule } from '@angular/material/button';
import { MetadataTabComponent } from '../info-drawer/metadata-tab/metadata-tab.component';
import { CommentsTabComponent } from '../info-drawer/comments-tab/comments-tab.component';
import { NodeEntry, PathElement } from '@alfresco/js-api';
import { first, takeUntil } from 'rxjs/operators';
import { first } from 'rxjs/operators';
import { ContentActionRef } from '@alfresco/adf-extensions';
import { FileSizePipe, InfoDrawerButtonsDirective } from '@alfresco/adf-core';
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
Expand Down Expand Up @@ -73,8 +72,6 @@ export class DetailsComponent extends PageComponent implements OnInit, OnDestroy
nodeIcon: string;
canManagePermissions = true;

private readonly onDestroy$: Subject<void> = new Subject<void>();

constructor(
private readonly route: ActivatedRoute,
private readonly contentApi: ContentApiService,
Expand Down Expand Up @@ -110,20 +107,6 @@ export class DetailsComponent extends PageComponent implements OnInit, OnDestroy
.subscribe((aspectActions) => {
this.aspectActions = aspectActions;
});
this.infoDrawerOpened$
.pipe(
first((opened) => !opened),
takeUntil(
merge(
this.onDestroy$,
AleksanderSklorz marked this conversation as resolved.
Show resolved Hide resolved
this.router.events.pipe(
first((event) => event instanceof NavigationStart),
takeUntil(this.onDestroy$)
)
)
)
)
.subscribe(() => this.goBack());
}

setActiveTab(tabName: string) {
Expand Down Expand Up @@ -153,8 +136,6 @@ export class DetailsComponent extends PageComponent implements OnInit, OnDestroy
}

ngOnDestroy(): void {
this.onDestroy$.next();
this.onDestroy$.complete();
this.store.dispatch(new SetSelectedNodesAction([]));
super.ngOnDestroy();
}
Expand Down
7 changes: 5 additions & 2 deletions projects/aca-shared/rules/src/app.rules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1412,16 +1412,19 @@ describe('app.evaluators', () => {
expect(app.canShowInfoDrawer(context)).toBeFalse();
});

it('should return false when user is in libraries or trashcan', () => {
it('should return false when user is in libraries or trashcan or details', () => {
context.selection.isEmpty = false;
context.navigation.url = '/trashcan/test';
expect(app.canShowInfoDrawer(context)).toBeFalse();

context.navigation.url = '/test/libraries';
expect(app.canShowInfoDrawer(context)).toBeFalse();

context.navigation.url = '/test/details';
expect(app.canShowInfoDrawer(context)).toBeFalse();
});

it('should return true when selection exists and user is not in trashcan or libraries', () => {
it('should return true when selection exists and user is not in trashcan, libraries or details', () => {
context.navigation.url = '/personal-files/test';
context.selection.isEmpty = false;
expect(app.canShowInfoDrawer(context)).toBeTrue();
Expand Down
2 changes: 1 addition & 1 deletion projects/aca-shared/rules/src/app.rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ export const canToggleSharedLink = (context: RuleContext): boolean =>
* @param context Rule execution context
*/
export const canShowInfoDrawer = (context: RuleContext): boolean =>
[hasSelection(context), navigation.isNotLibraries(context), navigation.isNotTrashcan(context)].every(Boolean);
[hasSelection(context), navigation.isNotLibraries(context), navigation.isNotTrashcan(context), navigation.isNotDetails(context)].every(Boolean);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the context of the visibility rules PR, the canShowInfoDrawer rule and method will be removed in favour of individual array based rules. So the reference of this rule in app.extensions.json at line num 498 and 1057 will no longer be present once that PR is merged, and would be replaced with separate arrays as can be seen in that PR. Can you please update the code here so that instead of adding the navigation.isNotDetails(context) call here, its added as an element of the visible array in the json file itself. I see that this rule is already registered in aca-content.module.ts in this PR, so you only need to modify the rule in the json. The change should be something similar to

"visible": [ "canShowInfoDrawer", "app.navigation.isNotDetails" ]


/**
* Checks if user can manage file versions for the selected node.
Expand Down
22 changes: 22 additions & 0 deletions projects/aca-shared/rules/src/navigation.rules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,28 @@ describe('navigation.evaluators', () => {
});
});

describe('isNotDetails', () => {
it('should return true if url does not include `/details`', () => {
const context: any = {
navigation: {
url: '/path'
}
};

expect(app.isNotDetails(context)).toBe(true);
});

it('should return false if url includes `/details`', () => {
const context: any = {
navigation: {
url: 'personal-files/details/path'
}
};

expect(app.isNotDetails(context)).toBe(false);
});
});

describe('isRecentFiles', () => {
it('should return [true] if url starts with `/recent-files`', () => {
const context: any = {
Expand Down
6 changes: 6 additions & 0 deletions projects/aca-shared/rules/src/navigation.rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ export function isDetails(context: RuleContext): boolean {
return url?.includes('/details');
}

/**
* Checks if the activated route is not **Details**.
* JSON ref: `app.navigation.isNotDetails`
*/
export const isNotDetails = (context: RuleContext): boolean => !isDetails(context);

/**
* Checks if the activated route is neither **Libraries** nor **Library Search Results**.
* JSON ref: `app.navigation.isNotLibraries`
Expand Down
Loading