From b86ae8dd143d9970e126c2a6f5154a9c9c5ec74e Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 14 Sep 2021 14:16:04 -0500 Subject: [PATCH 1/7] Allow enter key to expand or contract menu based on current active status --- .../expandable-navbar-section.component.html | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index cb96435b77..45edc12b12 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,18 +1,20 @@ -
- - - - -
+ *ngComponentOutlet="(sectionMap$ | async).get(section.id).component; injector: (sectionMap$ | async).get(section.id).injector;"> + + + + From 8e2ab83d92741e78a8a10984f1cac673ac960af0 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 17 Sep 2021 11:15:36 -0500 Subject: [PATCH 2/7] Fix existing tests. Add new specs for enter key presses --- .../expandable-navbar-section.component.html | 37 ++++++++--------- ...xpandable-navbar-section.component.spec.ts | 41 ++++++++++++++++++- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index 45edc12b12..04bc6067ac 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,20 +1,19 @@ - -
- +
+ + + +
    + - -
      - - - -
    -
- + *ngComponentOutlet="(sectionMap$ | async).get(subSection.id).component; injector: (sectionMap$ | async).get(subSection.id).injector;"> + + +
diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts index 8eea76e5c6..6b7c3d4bfc 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts @@ -9,6 +9,7 @@ import { HostWindowService } from '../../shared/host-window.service'; import { MenuService } from '../../shared/menu/menu.service'; import { HostWindowServiceStub } from '../../shared/testing/host-window-service.stub'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { VarDirective } from '../../shared/utils/var.directive'; describe('ExpandableNavbarSectionComponent', () => { let component: ExpandableNavbarSectionComponent; @@ -19,7 +20,7 @@ describe('ExpandableNavbarSectionComponent', () => { beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [NoopAnimationsModule], - declarations: [ExpandableNavbarSectionComponent, TestComponent], + declarations: [ExpandableNavbarSectionComponent, TestComponent, VarDirective], providers: [ { provide: 'sectionDataProvider', useValue: {} }, { provide: MenuService, useValue: menuService }, @@ -76,6 +77,42 @@ describe('ExpandableNavbarSectionComponent', () => { }); }); + describe('when Enter key is pressed on section header (while inactive)', () => { + beforeEach(() => { + spyOn(menuService, 'activateSection'); + // Make sure section is 'inactive'. Requires calling ngOnInit() to update component 'active' property. + spyOn(menuService, 'isSectionActive').and.returnValue(observableOf(false)); + component.ngOnInit(); + fixture.detectChanges(); + + const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); + // dispatch the (keyup.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); + }); + + it('should call activateSection on the menuService', () => { + expect(menuService.activateSection).toHaveBeenCalled(); + }); + }); + + describe('when Enter key is pressed on section header (while active)', () => { + beforeEach(() => { + spyOn(menuService, 'deactivateSection'); + // Make sure section is 'active'. Requires calling ngOnInit() to update component 'active' property. + spyOn(menuService, 'isSectionActive').and.returnValue(observableOf(true)); + component.ngOnInit(); + fixture.detectChanges(); + + const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); + // dispatch the (keyup.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); + }); + + it('should call deactivateSection on the menuService', () => { + expect(menuService.deactivateSection).toHaveBeenCalled(); + }); + }); + describe('when a click occurs on the section header', () => { beforeEach(() => { spyOn(menuService, 'toggleActiveSection'); @@ -96,7 +133,7 @@ describe('ExpandableNavbarSectionComponent', () => { beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [NoopAnimationsModule], - declarations: [ExpandableNavbarSectionComponent, TestComponent], + declarations: [ExpandableNavbarSectionComponent, TestComponent, VarDirective], providers: [ { provide: 'sectionDataProvider', useValue: {} }, { provide: MenuService, useValue: menuService }, From 523fca2177f79a42fea3293e7361ee0b183d3a8b Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 17 Sep 2021 11:16:00 -0500 Subject: [PATCH 3/7] Minor updates to README to provide more helpful info on unit tests --- README.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0e98bc7cf9..69b6132478 100644 --- a/README.md +++ b/README.md @@ -212,13 +212,17 @@ Once you have tested the Pull Request, please add a comment and/or approval to t ### Unit Tests -Unit tests use Karma. You can find the configuration file at the same level of this README file:`./karma.conf.js` If you are going to use a remote test environment you need to edit the `./karma.conf.js`. Follow the instructions you will find inside it. To executing tests whenever any file changes you can modify the 'autoWatch' option to 'true' and 'singleRun' option to 'false'. A coverage report is also available at: http://localhost:9876/ after you run: `yarn run coverage`. +Unit tests use the [Jasmine test framework](https://jasmine.github.io/), and are run via [Karma](https://karma-runner.github.io/). + +You can find the Karma configuration file at the same level of this README file:`./karma.conf.js` If you are going to use a remote test environment you need to edit the `./karma.conf.js`. Follow the instructions you will find inside it. To executing tests whenever any file changes you can modify the 'autoWatch' option to 'true' and 'singleRun' option to 'false'. A coverage report is also available at: http://localhost:9876/ after you run: `yarn run coverage`. The default browser is Google Chrome. -Place your tests in the same location of the application source code files that they test. +Place your tests in the same location of the application source code files that they test, e.g. ending with `*.component.spec.ts` -and run: `yarn run test` +and run: `yarn test` + +If you run into odd test errors, see the Angular guide to debugging tests: https://angular.io/guide/test-debugging ### E2E Tests @@ -258,6 +262,10 @@ _Hint: Creating e2e tests is easiest in an IDE (like Visual Studio), as it can h More Information: [docs.cypress.io](https://docs.cypress.io/) has great guides & documentation helping you learn more about writing/debugging e2e tests in Cypress. +### Learning how to build tests + +See our [DSpace Code Testing Guide](https://wiki.lyrasis.org/display/DSPACE/Code+Testing+Guide) for more hints/tips. + Documentation -------------- From e5e6e9c07aca19fc4edfb44b18a79e0f8f5742e3 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 28 Sep 2021 12:20:53 -0500 Subject: [PATCH 4/7] Also support using spacebar to open/close menu --- .../expandable-navbar-section.component.html | 1 + ...xpandable-navbar-section.component.spec.ts | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index 04bc6067ac..171530ecde 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,6 +1,7 @@
{ }); }); + describe('when spacebar is pressed on section header (while inactive)', () => { + beforeEach(() => { + spyOn(menuService, 'activateSection'); + // Make sure section is 'inactive'. Requires calling ngOnInit() to update component 'active' property. + spyOn(menuService, 'isSectionActive').and.returnValue(observableOf(false)); + component.ngOnInit(); + fixture.detectChanges(); + + const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); + // dispatch the (keyup.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); + }); + + it('should call activateSection on the menuService', () => { + expect(menuService.activateSection).toHaveBeenCalled(); + }); + }); + + describe('when spacebar is pressed on section header (while active)', () => { + beforeEach(() => { + spyOn(menuService, 'deactivateSection'); + // Make sure section is 'active'. Requires calling ngOnInit() to update component 'active' property. + spyOn(menuService, 'isSectionActive').and.returnValue(observableOf(true)); + component.ngOnInit(); + fixture.detectChanges(); + + const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); + // dispatch the (keyup.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); + }); + + it('should call deactivateSection on the menuService', () => { + expect(menuService.deactivateSection).toHaveBeenCalled(); + }); + }); + describe('when a click occurs on the section header', () => { beforeEach(() => { spyOn(menuService, 'toggleActiveSection'); From 7747a8c4e2c05a569e6c8db94d2ddd627f94835d Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Thu, 30 Sep 2021 17:11:34 -0500 Subject: [PATCH 5/7] Attempt to stop scroll to bottom of page when clicking spacebar --- .../expandable-navbar-section.component.html | 2 +- .../expandable-navbar-section.component.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index 171530ecde..fe31c73066 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,7 +1,7 @@
{ @@ -57,6 +58,7 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp * @param {Event} event The user event that triggered this function */ deactivateSection(event): void { + event.preventDefault(); this.windowService.isXsOrSm().pipe( first() ).subscribe((isMobile) => { From 1d0a5c0e87c42a391acfb6a4f668721e951a5d89 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 1 Oct 2021 16:39:17 -0500 Subject: [PATCH 6/7] Switch from keyup to keydown to fix spacebar scrolling issue. --- .../expandable-navbar-section.component.html | 4 ++-- .../expandable-navbar-section.component.spec.ts | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index fe31c73066..1a86691c11 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,7 +1,7 @@
{ fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keyup.enter) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); + // dispatch the (keydown.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); }); it('should call activateSection on the menuService', () => { @@ -104,8 +104,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keyup.enter) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); + // dispatch the (keydown.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); }); it('should call deactivateSection on the menuService', () => { @@ -122,8 +122,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keyup.space) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); + // dispatch the (keydown.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); }); it('should call activateSection on the menuService', () => { @@ -140,8 +140,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keyup.space) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); + // dispatch the (keydown.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); }); it('should call deactivateSection on the menuService', () => { From b4732f3c31b787e10abd5f82eb9003d6882a25ba Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 4 Oct 2021 10:05:43 -0500 Subject: [PATCH 7/7] Move actions to keyup while having preventDefault on keydown --- .../expandable-navbar-section.component.html | 5 +++-- .../expandable-navbar-section.component.spec.ts | 16 ++++++++-------- .../expandable-navbar-section.component.ts | 2 -- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index 1a86691c11..08d1b54fab 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,7 +1,8 @@
{ fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keydown.enter) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); + // dispatch the (keyup.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); }); it('should call activateSection on the menuService', () => { @@ -104,8 +104,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keydown.enter) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); + // dispatch the (keyup.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); }); it('should call deactivateSection on the menuService', () => { @@ -122,8 +122,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keydown.space) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); + // dispatch the (keyup.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); }); it('should call activateSection on the menuService', () => { @@ -140,8 +140,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keydown.space) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); + // dispatch the (keyup.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); }); it('should call deactivateSection on the menuService', () => { diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts index cebfa083ff..a0c0240159 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts @@ -42,7 +42,6 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp * @param {Event} event The user event that triggered this function */ activateSection(event): void { - event.preventDefault(); this.windowService.isXsOrSm().pipe( first() ).subscribe((isMobile) => { @@ -58,7 +57,6 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp * @param {Event} event The user event that triggered this function */ deactivateSection(event): void { - event.preventDefault(); this.windowService.isXsOrSm().pipe( first() ).subscribe((isMobile) => {