From 5bcd3268911d7169a725d60cd7b4e218db58b60a Mon Sep 17 00:00:00 2001 From: Mark Steadman <47225088+Steady5063@users.noreply.github.com> Date: Thu, 19 Mar 2020 03:13:42 -0500 Subject: [PATCH] [ACA-2619][ACA-2616] a11y fixes for Share Link dialog (#5454) * chore: proper disabling of fields for a11y * component fixes * update tests * update tests * fix aria labels * aria-label fixes * update layout * update e2e tests Co-authored-by: Denys Vuika Co-authored-by: Cilibiu Bogdan --- .../share-file/share-file.e2e.ts | 11 +--- e2e/pages/adf/dialog/share-dialog.page.ts | 29 ++++++---- .../content-node-share.dialog.html | 56 ++++++++++++++----- .../content-node-share.dialog.spec.ts | 45 ++++++++------- .../content-node-share.dialog.ts | 35 ++++++------ 5 files changed, 106 insertions(+), 70 deletions(-) diff --git a/e2e/content-services/share-file/share-file.e2e.ts b/e2e/content-services/share-file/share-file.e2e.ts index 0e911440b3..491bd450cb 100644 --- a/e2e/content-services/share-file/share-file.e2e.ts +++ b/e2e/content-services/share-file/share-file.e2e.ts @@ -121,6 +121,7 @@ describe('Share file', () => { it('[C286578] Should disable today option in expiration day calendar', async () => { await contentServicesPage.clickShareButton(); await shareDialog.checkDialogIsDisplayed(); + await shareDialog.clickExpireToggle(); await shareDialog.clickDateTimePickerButton(); await shareDialog.calendarTodayDayIsDisabled(); await BrowserActions.closeMenuAndDialogs(); @@ -129,6 +130,7 @@ describe('Share file', () => { it('[C286548] Should be possible to set expiry date for link', async () => { await contentServicesPage.clickShareButton(); await shareDialog.checkDialogIsDisplayed(); + await shareDialog.clickExpireToggle(); await shareDialog.setDefaultDay(); await shareDialog.setDefaultHour(); await shareDialog.setDefaultMinutes(); @@ -142,18 +144,11 @@ describe('Share file', () => { await BrowserActions.closeMenuAndDialogs(); }); - it('[C286578] Should disable today option in expiration day calendar', async () => { - await contentServicesPage.clickShareButton(); - await shareDialog.checkDialogIsDisplayed(); - await shareDialog.clickDateTimePickerButton(); - await shareDialog.calendarTodayDayIsDisabled(); - await BrowserActions.closeMenuAndDialogs(); - }); - it('[C310329] Should be possible to set expiry date only for link', async () => { await LocalStorageUtil.setConfigField('sharedLinkDateTimePickerType', JSON.stringify('date')); await contentServicesPage.clickShareButton(); await shareDialog.checkDialogIsDisplayed(); + await shareDialog.clickExpireToggle(); await shareDialog.setDefaultDay(); await shareDialog.dateTimePickerDialogIsClosed(); const value = await shareDialog.getExpirationDate(); diff --git a/e2e/pages/adf/dialog/share-dialog.page.ts b/e2e/pages/adf/dialog/share-dialog.page.ts index abeacb87a0..54152bcab2 100644 --- a/e2e/pages/adf/dialog/share-dialog.page.ts +++ b/e2e/pages/adf/dialog/share-dialog.page.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { element, by, ElementFinder } from 'protractor'; +import { element, by } from 'protractor'; import { BrowserVisibility, TogglePage, BrowserActions, DateTimePickerPage } from '@alfresco/adf-testing'; import moment = require('moment'); @@ -23,17 +23,18 @@ export class ShareDialogPage { togglePage = new TogglePage(); dateTimePickerPage = new DateTimePickerPage(); - shareDialog: ElementFinder = element(by.css('adf-share-dialog')); - dialogTitle: ElementFinder = element(by.css('[data-automation-id="adf-share-dialog-title"]')); - shareToggle: ElementFinder = element(by.css('[data-automation-id="adf-share-toggle"] label')); - shareToggleChecked: ElementFinder = element(by.css('mat-dialog-container mat-slide-toggle.mat-checked')); - shareLink: ElementFinder = element(by.css('[data-automation-id="adf-share-link"]')); - closeButton: ElementFinder = element(by.css('button[data-automation-id="adf-share-dialog-close"]')); - copySharedLinkButton: ElementFinder = element(by.css('.adf-input-action')); - expirationDateInput: ElementFinder = element(by.css('input[formcontrolname="time"]')); - confirmationDialog: ElementFinder = element(by.css('adf-confirm-dialog')); - confirmationCancelButton: ElementFinder = element(by.id('adf-confirm-cancel')); - confirmationRemoveButton: ElementFinder = element(by.id('adf-confirm-accept')); + shareDialog = element(by.css('adf-share-dialog')); + dialogTitle = element(by.css('[data-automation-id="adf-share-dialog-title"]')); + shareToggle = element(by.css('[data-automation-id="adf-share-toggle"] label')); + expireToggle = element(by.css(`[data-automation-id="adf-expire-toggle"] label`)); + shareToggleChecked = element(by.css('mat-dialog-container mat-slide-toggle.mat-checked')); + shareLink = element(by.css('[data-automation-id="adf-share-link"]')); + closeButton = element(by.css('button[data-automation-id="adf-share-dialog-close"]')); + copySharedLinkButton = element(by.css('.adf-input-action')); + expirationDateInput = element(by.css('input[formcontrolname="time"]')); + confirmationDialog = element(by.css('adf-confirm-dialog')); + confirmationCancelButton = element(by.id('adf-confirm-cancel')); + confirmationRemoveButton = element(by.id('adf-confirm-accept')); async checkDialogIsDisplayed(): Promise { await BrowserVisibility.waitUntilElementIsVisible(this.dialogTitle); @@ -43,6 +44,10 @@ export class ShareDialogPage { await this.togglePage.enableToggle(this.shareToggle); } + async clickExpireToggle() { + await this.togglePage.enableToggle(this.expireToggle); + } + async clickConfirmationDialogCancelButton(): Promise { await BrowserActions.click(this.confirmationCancelButton); } diff --git a/lib/content-services/src/lib/content-node-share/content-node-share.dialog.html b/lib/content-services/src/lib/content-node-share/content-node-share.dialog.html index eff64477f3..f59b01beef 100644 --- a/lib/content-services/src/lib/content-node-share/content-node-share.dialog.html +++ b/lib/content-services/src/lib/content-node-share/content-node-share.dialog.html @@ -7,19 +7,32 @@

{{ 'SHARE.DESCRIPTION' | translate }}

- +
{{ 'SHARE.TITLE' | translate }}
- +
- - link @@ -27,16 +40,31 @@
- - - +
{{ 'SHARE.EXPIRES' | translate }}
+ +
- - - + + + + + { ContentNodeShareModule ], providers: [ + { provide: AlfrescoApiService, useClass: AlfrescoApiServiceMock }, { provide: AppConfigService, useClass: AppConfigServiceMock }, { provide: NotificationService, useValue: notificationServiceMock }, { provide: MatDialogRef, useValue: { close: () => {}} }, @@ -77,6 +80,12 @@ describe('ShareDialogComponent', () => { properties: {} } }; + + spyOn(nodesApiService, 'updateNode').and.returnValue(of({})); + }); + + afterEach(() => { + fixture.destroy(); }); describe('Error Handling', () => { @@ -130,8 +139,10 @@ describe('ShareDialogComponent', () => { expect(fixture.nativeElement.querySelector('.mat-slide-toggle').classList).toContain('mat-checked'); }); - it(`should not toggle share action when file has 'sharedId' property`, async(() => { - spyOn(sharedLinksApiService, 'createSharedLinks'); + it(`should not toggle share action when file has 'sharedId' property`, async () => { + spyOn(sharedLinksApiService, 'createSharedLinks').and.returnValue(of({ + entry: { id: 'sharedId', sharedId: 'sharedId' } + })); spyOn(renditionService, 'generateRenditionForNode').and.returnValue(empty()); node.entry.properties['qshare:sharedId'] = 'sharedId'; @@ -143,19 +154,18 @@ describe('ShareDialogComponent', () => { fixture.detectChanges(); - fixture.whenStable().then(() => { - fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); - expect(sharedLinksApiService.createSharedLinks).not.toHaveBeenCalled(); - expect(fixture.nativeElement.querySelector('input[formcontrolname="sharedUrl"]').value).toBe('some-url/sharedId'); - expect(fixture.nativeElement.querySelector('.mat-slide-toggle').classList).toContain('mat-checked'); - - }); - })); + expect(sharedLinksApiService.createSharedLinks).not.toHaveBeenCalled(); + expect(fixture.nativeElement.querySelector('input[formcontrolname="sharedUrl"]').value).toBe('some-url/sharedId'); + expect(fixture.nativeElement.querySelector('.mat-slide-toggle').classList).toContain('mat-checked'); + }); it('should open a confirmation dialog when unshare button is triggered', () => { spyOn(matDialog, 'open').and.returnValue({ beforeClose: () => of(false) }); spyOn(sharedLinksApiService, 'deleteSharedLink').and.callThrough(); + node.entry.properties['qshare:sharedId'] = 'sharedId'; component.data = { @@ -175,7 +185,7 @@ describe('ShareDialogComponent', () => { it('should unshare file when confirmation dialog returns true', fakeAsync(() => { spyOn(matDialog, 'open').and.returnValue({ beforeClose: () => of(true) }); - spyOn(sharedLinksApiService, 'deleteSharedLink').and.callThrough(); + spyOn(sharedLinksApiService, 'deleteSharedLink').and.returnValue(of({})); node.entry.properties['qshare:sharedId'] = 'sharedId'; component.data = { @@ -230,7 +240,6 @@ describe('ShareDialogComponent', () => { }); it('should reset expiration date when toggle is unchecked', () => { - spyOn(nodesApiService, 'updateNode').and.returnValue(of({})); node.entry.properties['qshare:sharedId'] = 'sharedId'; node.entry.properties['qshare:sharedId'] = '2017-04-15T18:31:37+00:00'; node.entry.allowableOperations = ['update']; @@ -282,7 +291,6 @@ describe('ShareDialogComponent', () => { const date = moment(); node.entry.allowableOperations = ['update']; node.entry.properties['qshare:sharedId'] = 'sharedId'; - spyOn(nodesApiService, 'updateNode').and.returnValue(of({})); fixture.componentInstance.form.controls['time'].setValue(null); component.data = { @@ -308,7 +316,6 @@ describe('ShareDialogComponent', () => { describe('datetimepicker type', () => { beforeEach(() => { - spyOn(nodesApiService, 'updateNode').and.returnValue(of({})); spyOn(sharedLinksApiService, 'createSharedLinks').and.returnValue(of({})); node.entry.allowableOperations = ['update']; component.data = { @@ -326,7 +333,7 @@ describe('ShareDialogComponent', () => { fixture.nativeElement.querySelector('mat-slide-toggle[data-automation-id="adf-expire-toggle"] label') .dispatchEvent(new MouseEvent('click')); - fixture.componentInstance.form.controls['time'].setValue(date); + fixture.componentInstance.time.setValue(date); fixture.detectChanges(); expect(nodesApiService.updateNode).toHaveBeenCalledWith('nodeId', { @@ -337,13 +344,13 @@ describe('ShareDialogComponent', () => { it('it should update node with input date and time when type is `datetime`', () => { const dateTimePickerType = 'datetime'; const date = moment('2525-01-01 13:00:00'); - spyOn(appConfigService, 'get').and.callFake(() => dateTimePickerType); + spyOn(appConfigService, 'get').and.returnValue(dateTimePickerType); fixture.detectChanges(); fixture.nativeElement.querySelector('mat-slide-toggle[data-automation-id="adf-expire-toggle"] label') .dispatchEvent(new MouseEvent('click')); - fixture.componentInstance.form.controls['time'].setValue(date); + fixture.componentInstance.time.setValue(date); fixture.detectChanges(); expect(nodesApiService.updateNode).toHaveBeenCalledWith('nodeId', { diff --git a/lib/content-services/src/lib/content-node-share/content-node-share.dialog.ts b/lib/content-services/src/lib/content-node-share/content-node-share.dialog.ts index ecd853cd92..8d9b9c9de7 100644 --- a/lib/content-services/src/lib/content-node-share/content-node-share.dialog.ts +++ b/lib/content-services/src/lib/content-node-share/content-node-share.dialog.ts @@ -24,13 +24,12 @@ import { OnDestroy } from '@angular/core'; import { MAT_DIALOG_DATA, MatDialogRef, MatDialog, MatSlideToggleChange } from '@angular/material'; -import { FormGroup, FormControl } from '@angular/forms'; -import { Observable, throwError, Subject } from 'rxjs'; +import { FormGroup, FormControl, AbstractControl } from '@angular/forms'; +import { Observable, Subject } from 'rxjs'; import { skip, distinctUntilChanged, mergeMap, - catchError, takeUntil } from 'rxjs/operators'; import { @@ -62,13 +61,10 @@ export class ShareDialogComponent implements OnInit, OnDestroy { isDisabled: boolean = false; form: FormGroup = new FormGroup({ sharedUrl: new FormControl(''), - time: new FormControl({ value: '', disabled: false }) + time: new FormControl({ value: '', disabled: true }) }); type = 'datetime'; - @ViewChild('matDatetimepickerToggle') - matDatetimepickerToggle; - @ViewChild('slideToggleExpirationDate') slideToggleExpirationDate; @@ -92,10 +88,10 @@ export class ShareDialogComponent implements OnInit, OnDestroy { this.type = this.appConfigService.get('sharedLinkDateTimePickerType', 'datetime'); if (!this.canUpdate) { - this.form.controls['time'].disable(); + this.time.disable(); } - this.form.controls.time.valueChanges + this.time.valueChanges .pipe( skip(1), distinctUntilChanged(), @@ -103,9 +99,6 @@ export class ShareDialogComponent implements OnInit, OnDestroy { (updates) => this.updateNode(updates), (formUpdates) => formUpdates ), - catchError((error) => { - return throwError(error); - }), takeUntil(this.onDestroy$) ) .subscribe(updates => this.updateEntryExpiryDate(updates)); @@ -120,12 +113,15 @@ export class ShareDialogComponent implements OnInit, OnDestroy { } else { this.sharedId = properties['qshare:sharedId']; this.isFileShared = true; - this.updateForm(); } } } + get time(): AbstractControl { + return this.form.controls['time']; + } + ngOnDestroy() { this.onDestroy$.next(true); this.onDestroy$.complete(); @@ -155,17 +151,16 @@ export class ShareDialogComponent implements OnInit, OnDestroy { onToggleExpirationDate(slideToggle: MatSlideToggleChange) { if (slideToggle.checked) { - this.matDatetimepickerToggle.datetimepicker.open(); + this.time.enable(); } else { - this.matDatetimepickerToggle.datetimepicker.close(); - this.form.controls.time.setValue(null); + this.time.disable(); } } onDatetimepickerClosed() { this.dateTimePickerInput.nativeElement.blur(); - if (!this.form.controls.time.value) { + if (!this.time.value) { this.slideToggleExpirationDate.checked = false; } } @@ -275,6 +270,12 @@ export class ShareDialogComponent implements OnInit, OnDestroy { sharedUrl: `${this.baseShareUrl}${this.sharedId}`, time: expiryDate ? moment(expiryDate).local() : null }); + + if (expiryDate) { + this.time.enable(); + } else { + this.time.disable(); + } } private updateNode(date: moment.Moment): Observable {