From abc5863f75b7f5b6e8dfd3b884025e2649b2d92a Mon Sep 17 00:00:00 2001 From: Xiao Gui <xgui3783@gmail.com> Date: Wed, 29 Apr 2020 19:06:33 +0200 Subject: [PATCH] chore: added named input for iav-switch chore: reworked how fixedmousecontexualcontainer works chore added unit and e2e for fixedMouseCxt tweak: by default layerbrowser is now collapsed tweak: reduced region name in ctx menu chore: removed unused import --- common/constants.js | 2 + e2e/src/layout/viewerCtxMenu.prod.e2e-spec.js | 53 +++++++ e2e/src/util.js | 9 ++ src/atlasViewer/atlasViewer.component.ts | 13 +- src/atlasViewer/atlasViewer.style.css | 5 + src/atlasViewer/atlasViewer.template.html | 8 +- .../atlasViewer.pluginService.service.ts | 2 - src/index.html | 1 - src/res/css/extra_styles.css | 5 - .../regionMenu/regionMenu.style.css | 18 +++ .../regionMenu/regionMenu.template.html | 12 +- .../searchSideNav/searchSideNav.component.ts | 2 - .../searchSideNav/searchSideNav.template.html | 13 +- ...extualContainerDirective.directive.spec.ts | 131 ++++++++++++++++++ ...eContextualContainerDirective.directive.ts | 3 + src/util/directives/switch.directive.ts | 2 +- 16 files changed, 246 insertions(+), 33 deletions(-) create mode 100644 e2e/src/layout/viewerCtxMenu.prod.e2e-spec.js create mode 100644 src/util/directives/FixedMouseContextualContainerDirective.directive.spec.ts diff --git a/common/constants.js b/common/constants.js index b3643c815..c5f7c0567 100644 --- a/common/constants.js +++ b/common/constants.js @@ -1,6 +1,8 @@ (function(exports){ exports.ARIA_LABELS = { + // overlay specific + CONTEXT_MENU: `Viewer context menu`, // sharing module SHARE_BTN: `Share this view`, diff --git a/e2e/src/layout/viewerCtxMenu.prod.e2e-spec.js b/e2e/src/layout/viewerCtxMenu.prod.e2e-spec.js new file mode 100644 index 000000000..f5f208a3f --- /dev/null +++ b/e2e/src/layout/viewerCtxMenu.prod.e2e-spec.js @@ -0,0 +1,53 @@ +const { AtlasPage } = require('../util') +const { ARIA_LABELS } = require('../../../common/constants') +const dict = { + "ICBM 2009c Nonlinear Asymmetric": { + "JuBrain Cytoarchitectonic Atlas": { + tests:[ + { + position: [550, 270], + expectedLabelName: 'Fastigial Nucleus (Cerebellum) - right hemisphere', + } + ] + } + } +} + +describe('> viewerCtxMenu', () => { + + for (const templateName in dict) { + for (const parcellationName in dict[templateName]) { + describe(`> on ${templateName} / ${parcellationName}`, () => { + beforeAll(async () => { + iavPage = new AtlasPage() + await iavPage.init() + await iavPage.goto() + await iavPage.selectTitleTemplateParcellation(templateName, parcellationName) + await iavPage.wait(500) + await iavPage.waitForAsync() + }) + + it('> does not appear on init', async () => { + const visible = await iavPage.isVisible(`[aria-label="${ARIA_LABELS.CONTEXT_MENU}"]`) + expect(visible).toBeFalse() + }) + + it('> appears on click', async () => { + const { tests } = dict[templateName][parcellationName] + const { position } = tests[0] + await iavPage.cursorMoveToAndClick({ position }) + await iavPage.wait(500) + const visible = await iavPage.isVisible(`[aria-label="${ARIA_LABELS.CONTEXT_MENU}"]`) + expect(visible).toBeTrue() + }) + + it('> disappear again on click of anywhere else', async () => { + await iavPage.cursorMoveToAndClick({ position: [10, 10] }) + await iavPage.wait(500) + const visible = await iavPage.isVisible(`[aria-label="${ARIA_LABELS.CONTEXT_MENU}"]`) + expect(visible).toBeFalse() + }) + }) + } + } +}) \ No newline at end of file diff --git a/e2e/src/util.js b/e2e/src/util.js index a5157471f..457dfcdd6 100644 --- a/e2e/src/util.js +++ b/e2e/src/util.js @@ -113,6 +113,15 @@ class WdBase{ return text } + async isVisible(cssSelector) { + + if (!cssSelector) throw new Error(`getText needs to define css selector`) + const el = await this._browser.findElement( By.css(cssSelector) ) + const isDisplayed = await el.isDisplayed() + + return isDisplayed + } + historyBack() { return this._browser.navigate().back() } diff --git a/src/atlasViewer/atlasViewer.component.ts b/src/atlasViewer/atlasViewer.component.ts index e51d012d4..b349816e4 100644 --- a/src/atlasViewer/atlasViewer.component.ts +++ b/src/atlasViewer/atlasViewer.component.ts @@ -27,7 +27,6 @@ import { isDefined, safeFilter, } from "../services/stateStore.service"; -import { AtlasViewerAPIServices } from "./atlasViewer.apiService.service"; import { AtlasViewerConstantsServices, UNSUPPORTED_INTERVAL, UNSUPPORTED_PREVIEW } from "./atlasViewer.constantService.service"; import { WidgetServices } from "./widgetUnit/widgetService.service"; @@ -44,6 +43,7 @@ import { colorAnimation } from "./atlasViewer.animation" import { MouseHoverDirective } from "src/util/directives/mouseOver.directive"; import {MatSnackBar, MatSnackBarRef} from "@angular/material/snack-bar"; import {MatDialog, MatDialogRef} from "@angular/material/dialog"; +import { ARIA_LABELS } from 'common/constants' export const NEHUBA_CLICK_OVERRIDE = 'NEHUBA_CLICK_OVERRIDE' @@ -67,11 +67,11 @@ const compareFn = (it, item) => it.name === item.name export class AtlasViewer implements OnDestroy, OnInit, AfterViewInit { + public CONTEXT_MENU_ARIA_LABEL = ARIA_LABELS.CONTEXT_MENU public compareFn = compareFn @ViewChild('cookieAgreementComponent', {read: TemplateRef}) public cookieAgreementComponent: TemplateRef<any> - private persistentStateNotifierMatDialogRef: MatDialogRef<any> @ViewChild('kgToS', {read: TemplateRef}) public kgTosComponent: TemplateRef<any> @ViewChild(LayoutMainSide) public layoutMainSide: LayoutMainSide @@ -382,11 +382,9 @@ export class AtlasViewer implements OnDestroy, OnInit, AfterViewInit { ) } - public mouseDownNehuba() { - this.rClContextualMenu.hide() - } + public mouseClickDocument(event) { - public mouseClickNehuba(event) { + const dismissRClCtxtMenu = this.rClContextualMenu.isShown const next = () => { @@ -396,7 +394,8 @@ export class AtlasViewer implements OnDestroy, OnInit, AfterViewInit { event.clientY, ] - this.rClContextualMenu.show() + if (dismissRClCtxtMenu) this.rClContextualMenu.hide() + else this.rClContextualMenu.show() } this.nehubaClickOverride(next) diff --git a/src/atlasViewer/atlasViewer.style.css b/src/atlasViewer/atlasViewer.style.css index bb25a9c67..4add13a9b 100644 --- a/src/atlasViewer/atlasViewer.style.css +++ b/src/atlasViewer/atlasViewer.style.css @@ -73,3 +73,8 @@ region-menu { display:inline-block; } + +.floating-container +{ + max-width: 350px; +} \ No newline at end of file diff --git a/src/atlasViewer/atlasViewer.template.html b/src/atlasViewer/atlasViewer.template.html index 21fc37f18..0dc97ab18 100644 --- a/src/atlasViewer/atlasViewer.template.html +++ b/src/atlasViewer/atlasViewer.template.html @@ -57,8 +57,8 @@ [currentOnHoverObs$]="iavMouseHoverEl.currentOnHoverObs$" [currentOnHover]="iavMouseHoverEl.currentOnHoverObs$ | async" iav-captureClickListenerDirective - (iav-captureClickListenerDirective-onMousedown)="mouseDownNehuba()" - (iav-captureClickListenerDirective-onClick)="mouseClickNehuba($event)"> + [iav-captureClickListenerDirective-captureDocument]="true" + (iav-captureClickListenerDirective-onClick)="mouseClickDocument($event)"> </ui-nehuba-container> <div class="z-index-10 position-absolute pe-none w-100 h-100"> @@ -165,7 +165,9 @@ <!-- TODO Potentially implementing plugin contextual info --> </div> - <div fixedMouseContextualContainerDirective + <div class="floating-container" + [attr.aria-label]="CONTEXT_MENU_ARIA_LABEL" + fixedMouseContextualContainerDirective #fixedContainer="iavFixedMouseCtxContainer"> <!-- on click segment menu --> diff --git a/src/atlasViewer/pluginUnit/atlasViewer.pluginService.service.ts b/src/atlasViewer/pluginUnit/atlasViewer.pluginService.service.ts index 756444017..22ed4cd0e 100644 --- a/src/atlasViewer/pluginUnit/atlasViewer.pluginService.service.ts +++ b/src/atlasViewer/pluginUnit/atlasViewer.pluginService.service.ts @@ -12,8 +12,6 @@ import { PluginHandler } from 'src/util/pluginHandler'; import { AtlasViewerConstantsServices } from "../atlasViewer.constantService.service"; import { WidgetUnit } from "../widgetUnit/widgetUnit.component"; -import './plugin_styles.css' - @Injectable({ providedIn : 'root', }) diff --git a/src/index.html b/src/index.html index 23558e394..9b001d637 100644 --- a/src/index.html +++ b/src/index.html @@ -8,7 +8,6 @@ <link href="https://stackpath.bootstrapcdn.com/bootstrap/4.3.1/css/bootstrap.min.css" rel="stylesheet" integrity="sha384-ggOyR0iXCbMQv3Xipma34MD+dH/1fQ784/j6cY/iJTQUOhcWr7x9JvoRxT2MZw1T" crossorigin="anonymous"> <link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.8.1/css/all.css" integrity="sha384-50oBUHEmvpQ+1lW4y57PTFmhCaXp0ML5d60M1M7uH2+nqUivzIebhndOJK28anvf" crossorigin="anonymous"> <link rel="stylesheet" href="extra_styles.css"> - <link rel="stylesheet" href="plugin_styles.css"> <link rel="stylesheet" href="theme.css"> <link rel="stylesheet" href="version.css"> diff --git a/src/res/css/extra_styles.css b/src/res/css/extra_styles.css index e2f9b3b56..fe7d4d4e3 100644 --- a/src/res/css/extra_styles.css +++ b/src/res/css/extra_styles.css @@ -699,11 +699,6 @@ kg-dataset-previewer > img margin-right: -1rem!important; } -.mr-8 -{ - margin-right: 4rem!important; -} - .mat-card-sm > mat-dialog-container { padding-left: 16px!important; diff --git a/src/ui/parcellationRegion/regionMenu/regionMenu.style.css b/src/ui/parcellationRegion/regionMenu/regionMenu.style.css index 278de0373..fd17d0f56 100644 --- a/src/ui/parcellationRegion/regionMenu/regionMenu.style.css +++ b/src/ui/parcellationRegion/regionMenu/regionMenu.style.css @@ -8,4 +8,22 @@ mat-list.sm mat-list-item mat-icon { transform: scale(0.75); +} + +.action-list +{ + margin-left: -16px; + margin-right: -16px; +} + +.action-list mat-icon +{ + padding-left: 0px; +} + +.region-name +{ + display: inherit; + font-size: 95%; + line-height: normal; } \ No newline at end of file diff --git a/src/ui/parcellationRegion/regionMenu/regionMenu.template.html b/src/ui/parcellationRegion/regionMenu/regionMenu.template.html index 8c7ccf7e9..29daefc54 100644 --- a/src/ui/parcellationRegion/regionMenu/regionMenu.template.html +++ b/src/ui/parcellationRegion/regionMenu/regionMenu.template.html @@ -1,9 +1,7 @@ <mat-card> <mat-card-title> - <div class="position-relative"> - <span class="mr-8"> - {{ region.name }} - </span> + <div class="position-relative region-name"> + {{ region.name }} </div> </mat-card-title> <mat-card-subtitle> @@ -28,7 +26,7 @@ [color]="isSelected ? 'primary' : 'default'" (click)="toggleRegionSelected()"> <i iav-v-button-icon class="far" [ngClass]="{'fa-check-square': isSelected, 'fa-square': !isSelected}"></i> - <span iav-v-button-text>{{isSelected ? 'Deselect' : 'Select'}}</span> + <span iav-v-button-text>Select</span> </iav-v-button> </mat-grid-tile> @@ -99,7 +97,7 @@ <mat-divider></mat-divider> - <mat-list class="sm"> + <mat-list class="action-list sm"> <!-- selected --> <mat-list-item @@ -109,7 +107,7 @@ (click)="toggleRegionSelected()"> <mat-icon scaled-down="" fontSet="far" [fontIcon]="isSelected ? 'fa-check-square' : 'fa-square'" mat-list-icon></mat-icon> <div mat-line> - {{ isSelected ? 'Selected' : 'Add to selected' }} + Select </div> </mat-list-item> diff --git a/src/ui/searchSideNav/searchSideNav.component.ts b/src/ui/searchSideNav/searchSideNav.component.ts index a0647a652..fe9bcb9dc 100644 --- a/src/ui/searchSideNav/searchSideNav.component.ts +++ b/src/ui/searchSideNav/searchSideNav.component.ts @@ -25,8 +25,6 @@ import { MatSnackBar } from "@angular/material/snack-bar"; export class SearchSideNav implements OnDestroy { public availableDatasets: number = 0 - public showLayerBrowser: boolean = true - private subscriptions: Subscription[] = [] private layerBrowserDialogRef: MatDialogRef<any> diff --git a/src/ui/searchSideNav/searchSideNav.template.html b/src/ui/searchSideNav/searchSideNav.template.html index 765869826..b0bd20fff 100644 --- a/src/ui/searchSideNav/searchSideNav.template.html +++ b/src/ui/searchSideNav/searchSideNav.template.html @@ -75,7 +75,10 @@ </div> <ng-template #layerBrowserTmpl> - <mat-dialog-content [hidden]="!showLayerBrowser"> + <mat-dialog-content + iav-switch + #showLayerBrowserSwitch="iavSwitch" + [hidden]="!showLayerBrowserSwitch.switchState"> <div class="d-flex flex-column"> <layer-browser></layer-browser> </div> @@ -84,12 +87,12 @@ <div class="d-flex justify-content-center position-static h-0 mt-4"> <button mat-mini-fab aria-label="Toggle expansion state of additional layer browser" - [matBadge]="showLayerBrowser ? null : (layerBrowser.nonBaseNgLayers$ | async).length" + [matBadge]="showLayerBrowserSwitch.switchState ? null : (layerBrowser.nonBaseNgLayers$ | async).length" class="position-absolute layerBrowserToggleBtn" matTooltip="Toggle the visiblity of the additional layer browser" - [attr.toggle-open]="showLayerBrowser" - (click)="showLayerBrowser = !showLayerBrowser"> - <i class="fas" [ngClass]="{'fa-chevron-down': !showLayerBrowser, 'fa-chevron-up': showLayerBrowser}"> + [attr.toggle-open]="showLayerBrowserSwitch.switchState" + (click)="showLayerBrowserSwitch.toggle()"> + <i class="fas" [ngClass]="{'fa-chevron-down': !showLayerBrowserSwitch.switchState, 'fa-chevron-up': showLayerBrowserSwitch.switchState}"> </i> </button> </div> diff --git a/src/util/directives/FixedMouseContextualContainerDirective.directive.spec.ts b/src/util/directives/FixedMouseContextualContainerDirective.directive.spec.ts new file mode 100644 index 000000000..0cf11aeb3 --- /dev/null +++ b/src/util/directives/FixedMouseContextualContainerDirective.directive.spec.ts @@ -0,0 +1,131 @@ +import { Component, ViewChild } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; +import { FixedMouseContextualContainerDirective } from "./FixedMouseContextualContainerDirective.directive"; +import { By } from "@angular/platform-browser"; + +@Component({ + template: '' +}) + +class TestCmp{ + @ViewChild(FixedMouseContextualContainerDirective) directive: FixedMouseContextualContainerDirective +} + +describe('FixedMouseContextualContainerDirective', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [ + + ], + declarations: [ + TestCmp, + FixedMouseContextualContainerDirective + ] + }) + }) + + it('> can instantiate directive properly', () => { + TestBed.overrideComponent(TestCmp, { + set: { + template: ` + <div fixedMouseContextualContainerDirective> + </div> + ` + } + }).compileComponents() + + const fixture = TestBed.createComponent(TestCmp) + fixture.detectChanges() + const directive = fixture.debugElement.query( By.directive(FixedMouseContextualContainerDirective) ) + expect(directive).toBeTruthy() + + expect(fixture.componentInstance.directive).toBeTruthy() + }) + + describe('> hides if no content', () => { + it('> on #show, if content exists, isShown will be true', () => { + + TestBed.overrideComponent(TestCmp, { + set: { + template: ` + <div fixedMouseContextualContainerDirective> + <span>Hello World</span> + </div> + ` + } + }).compileComponents() + + const fixture = TestBed.createComponent(TestCmp) + fixture.detectChanges() + + const cmp = fixture.componentInstance + cmp.directive.show() + fixture.detectChanges() + expect(cmp.directive.isShown).toBeTrue() + }) + + it('> on #show, if only comment exists, isShown will be false', () => { + + TestBed.overrideComponent(TestCmp, { + set: { + template: ` + <div fixedMouseContextualContainerDirective> + <!-- hello world --> + </div> + ` + } + }).compileComponents() + + const fixture = TestBed.createComponent(TestCmp) + fixture.detectChanges() + + const cmp = fixture.componentInstance + cmp.directive.show() + fixture.detectChanges() + expect(cmp.directive.isShown).toBeFalse() + }) + + it('> on #show, if only text exists, isShown will be false', () => { + + TestBed.overrideComponent(TestCmp, { + set: { + template: ` + <div fixedMouseContextualContainerDirective> + hello world + </div> + ` + } + }).compileComponents() + + const fixture = TestBed.createComponent(TestCmp) + fixture.detectChanges() + + const cmp = fixture.componentInstance + cmp.directive.show() + fixture.detectChanges() + expect(cmp.directive.isShown).toBeFalse() + }) + + it('> on #show, if nothing exists, isShown will be false', () => { + + TestBed.overrideComponent(TestCmp, { + set: { + template: ` + <div fixedMouseContextualContainerDirective> + </div> + ` + } + }).compileComponents() + + const fixture = TestBed.createComponent(TestCmp) + fixture.detectChanges() + + const cmp = fixture.componentInstance + cmp.directive.show() + fixture.detectChanges() + expect(cmp.directive.isShown).toBeFalse() + }) + }) + + // TODO complete tests for FixedMouseContextualContainerDirective +}) \ No newline at end of file diff --git a/src/util/directives/FixedMouseContextualContainerDirective.directive.ts b/src/util/directives/FixedMouseContextualContainerDirective.directive.ts index bf2293466..058a27918 100644 --- a/src/util/directives/FixedMouseContextualContainerDirective.directive.ts +++ b/src/util/directives/FixedMouseContextualContainerDirective.directive.ts @@ -43,6 +43,9 @@ export class FixedMouseContextualContainerDirective implements AfterContentCheck } ngAfterContentChecked(){ + if (this.el.nativeElement.childElementCount === 0) { + this.hide() + } this.recalculatePosition() this.cdr.markForCheck() } diff --git a/src/util/directives/switch.directive.ts b/src/util/directives/switch.directive.ts index cbdbe4dfc..adcb6a707 100644 --- a/src/util/directives/switch.directive.ts +++ b/src/util/directives/switch.directive.ts @@ -5,7 +5,7 @@ import { Directive, Input } from "@angular/core"; exportAs: 'iavSwitch' }) export class SwitchDirective{ - @Input() switchState: boolean = false + @Input('iav-switch-initstate') switchState: boolean = false toggle(){ this.switchState = !this.switchState -- GitLab