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

[MNT-23433] Close Preview button position #3535

Merged
merged 11 commits into from
Jan 3, 2024
Next Next commit
[MNT-23433] preview button configure
AnukritiGL committed Jan 3, 2024
commit 8d5d7426d793e3bad94025b167dec87feb0f5e95
3 changes: 2 additions & 1 deletion app/src/app.config.json
Original file line number Diff line number Diff line change
@@ -1137,6 +1137,7 @@
"downloadPromptDelay": 50,
"downloadPromptReminderDelay": 30,
"enableFileAutoDownload": true,
"fileAutoDownloadSizeThresholdInMB": 15
"fileAutoDownloadSizeThresholdInMB": 15,
"isCloseButtonOnLeft": true
}
}
17 changes: 17 additions & 0 deletions projects/aca-content/assets/app.extensions.json
Original file line number Diff line number Diff line change
@@ -1180,6 +1180,23 @@
}
}
]
},
{
"id": "app.viewer.separator.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Ids of elements should be unique

"type": "separator",
"order": 11000
},
{
"id": "app.viewer.close",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should add new button for that, the close button already exist in ADF and ACA should just set the position value which should be passed to the ADF component which will correctly place the button.

Copy link
Contributor Author

@AnukritiGL AnukritiGL Nov 27, 2023

Choose a reason for hiding this comment

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

@MichalKinas to move the close button to the right side in the header we have to add the close button in toolbar-actions, and for left position we are using the existing button only.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you think you have to add new button in ACA? This is another thing that has to be maintained, is not documented properly, has naming issues and rule canClosePreview doesn't do what it's name suggests. This should be part of ADF component which should take a param with position of close button. This solution works in ACA only and overrides default component behaviour, I still think that the solution should be adding new param for ADF component and component itself should decide how to display the close button

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MichalKinas as per your suggestion I have removed the close button from toolbar-actions,now I am planning to add one more button in ADF viewer.component.html which will be show/hide as per the position set by customer in app.config.json which can be configuration from ACA or ADF.

"order": 12000,
"title": "APP.ACTIONS.CLOSE",
"icon": "close",
"actions": {
"click": "CLOSE_PREVIEW"
},
"rules": {
"visible": "canClosePreview"
}
}
],
"shared": {
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
@@ -196,6 +196,7 @@ export class ContentServiceExtensionModule {
canShowExpand: rules.canShowExpand,
canInfoPreview: rules.canInfoPreview,
showInfoSelectionButton: rules.showInfoSelectionButton,
canClosePreview: rules.canClosePreview,

'app.selection.canDelete': rules.canDeleteSelection,
'app.selection.file.canUnlock': rules.canUnlockFile,
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@
[allowDownload]="false"
[allowFullScreen]="false"
[overlayMode]="true"
[allowGoBack]="'viewer.isCloseButtonOnLeft' | adfAppConfig: true"
rbahirsheth marked this conversation as resolved.
Show resolved Hide resolved
(showViewerChange)="onViewerVisibilityChanged()"
[canNavigateBefore]="!!previousNodeId"
[canNavigateNext]="!!nextNodeId"
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@ export class ViewerComponent extends BaseComponent {
private static rootElement = 'adf-viewer';

private viewerLocator = this.getChild('.adf-viewer-render-layout-content');
public closeButtonLocator = this.getChild('.adf-viewer-close-button');
public closeButtonLocator = this.getChild('button[id="app.viewer.close"]');
public fileTitleButtonLocator = this.getChild('.adf-viewer__file-title');
public pdfViewerContentPages = this.getChild('.adf-pdf-viewer__content .page');
public shareButton = this.getChild('button[id="share-action-button"]');
22 changes: 22 additions & 0 deletions projects/aca-shared/rules/src/app.rules.spec.ts
Original file line number Diff line number Diff line change
@@ -541,6 +541,28 @@ describe('app.evaluators', () => {
});
});

describe('canClosePreview', () => {
it('should return false when viewer.isCloseButtonOnLeft is true', () => {
const context: any = {
appConfig: {
get: () => true
AleksanderSklorz marked this conversation as resolved.
Show resolved Hide resolved
}
};

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

it('should return true when viewer.isCloseButtonOnLeft is false', () => {
const context: any = {
appConfig: {
get: () => false
AleksanderSklorz marked this conversation as resolved.
Show resolved Hide resolved
}
};

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

describe('isLibraryManager', () => {
it('should return true when role is SiteManager', () => {
const context: any = {
12 changes: 12 additions & 0 deletions projects/aca-shared/rules/src/app.rules.ts
Original file line number Diff line number Diff line change
@@ -448,6 +448,18 @@ export const isTrashcanItemSelected = (context: RuleContext): boolean => [naviga
*/
export const canViewFile = (context: RuleContext): boolean => [hasFileSelected(context), navigation.isNotTrashcan(context)].every(Boolean);

/**
* Checks if user can **Close** opened file.
* JSON ref: `canClosePreview`
*
* @param context Rule execution context
*/
export const canClosePreview = (context: AcaRuleContext): boolean => {
rbahirsheth marked this conversation as resolved.
Show resolved Hide resolved
const flag = context.appConfig.get<boolean>('viewer.isCloseButtonOnLeft', true);

return !flag;
};

/**
* Checks if user can **Leave** selected library.
* JSON ref: `canLeaveLibrary`
Original file line number Diff line number Diff line change
@@ -32,7 +32,7 @@ export class Viewer extends Component {
root = browser.$('adf-viewer');
viewerLayout = this.byCss('.adf-viewer-render-layout-content');
viewerContainer = this.byCss('.adf-viewer-render-content-container');
closeButton = this.byCss('.adf-viewer-close-button');
closeButton = this.byCss('button[id="app.viewer.close"]');
fileTitle = this.byCss('.adf-viewer__file-title');
viewerExtensionContent = this.byCss('adf-preview-extension');
txtViewerContent = this.byCss('.adf-txt-viewer-content');