Skip to content

Commit

Permalink
refactor(workbench/popup): configure contextual reference(s) via cont…
Browse files Browse the repository at this point in the history
…ext object

BREAKING CHANGE: Changed popup config for passing contextual reference(s)

To migrate: Set a popup's view reference via `PopupConfig#context#viewId` instead of `PopupConfig#viewRef`.
  • Loading branch information
danielwiehl authored and simoneggler committed Feb 12, 2021
1 parent e031f96 commit 0591e7a
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
</select>
</sci-form-field>

<sci-form-field label="ViewRef">
<input [formControlName]="VIEW_REF" class="e2e-view-ref">
<sci-form-field label="Contextual View ID">
<input [formControlName]="CONTEXTUAL_VIEW_ID" class="e2e-contextual-view-id">
</sci-form-field>

<sci-form-field label="Align">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { defer, Observable, Subject } from 'rxjs';
const POPUP_COMPONENT = 'popupComponent';
const ANCHOR = 'anchor';
const BINDING = 'binding';
const VIEW_REF = 'viewRef';
const CONTEXTUAL_VIEW_ID = 'contextualViewId';
const ALIGN = 'align';
const INPUT = 'input';
const CSS_CLASS = 'cssClass';
Expand All @@ -47,7 +47,7 @@ export class PopupOpenerPageComponent implements OnDestroy, AfterViewInit {
public readonly POPUP_COMPONENT = POPUP_COMPONENT;
public readonly ANCHOR = ANCHOR;
public readonly BINDING = BINDING;
public readonly VIEW_REF = VIEW_REF;
public readonly CONTEXTUAL_VIEW_ID = CONTEXTUAL_VIEW_ID;
public readonly ALIGN = ALIGN;
public readonly INPUT = INPUT;
public readonly CSS_CLASS = CSS_CLASS;
Expand Down Expand Up @@ -93,7 +93,7 @@ export class PopupOpenerPageComponent implements OnDestroy, AfterViewInit {
[WIDTH]: formBuilder.control('0'),
[HEIGHT]: formBuilder.control('0'),
}),
[VIEW_REF]: formBuilder.control({disabled: true, value: ''}),
[CONTEXTUAL_VIEW_ID]: formBuilder.control({disabled: true, value: ''}),
[ALIGN]: formBuilder.control(''),
[CSS_CLASS]: formBuilder.control(''),
[INPUT]: formBuilder.control(''),
Expand All @@ -110,7 +110,7 @@ export class PopupOpenerPageComponent implements OnDestroy, AfterViewInit {
[MAX_WIDTH]: formBuilder.control(''),
}),
});
this.installViewRefEnabler();
this.installContextualViewIdEnabler();

this._coordinateAnchor$ = defer(() => this.form.get(ANCHOR)
.valueChanges
Expand Down Expand Up @@ -140,7 +140,6 @@ export class PopupOpenerPageComponent implements OnDestroy, AfterViewInit {

await this._popupService.open<string>({
component: this.parsePopupComponentInput(),
viewRef: this.parseViewRefInput(),
input: this.form.get(INPUT).value || undefined,
anchor: this.form.get([ANCHOR, BINDING]).value === 'element' ? this._openButton : this._coordinateAnchor$,
align: this.form.get(ALIGN).value || undefined,
Expand All @@ -157,6 +156,9 @@ export class PopupOpenerPageComponent implements OnDestroy, AfterViewInit {
minHeight: this.form.get([SIZE, MIN_HEIGHT]).value || undefined,
maxHeight: this.form.get([SIZE, MAX_HEIGHT]).value || undefined,
}),
context: {
viewId: this.parseContextualViewIdInput(),
},
})
.then(result => this.returnValue = result)
.catch(error => this.popupError = error ?? 'Popup was closed with an error');
Expand All @@ -173,31 +175,31 @@ export class PopupOpenerPageComponent implements OnDestroy, AfterViewInit {
}
}

private parseViewRefInput(): string | null | undefined {
const viewRef = this.form.get(VIEW_REF).value;
switch (viewRef) {
private parseContextualViewIdInput(): string | null | undefined {
const viewId = this.form.get(CONTEXTUAL_VIEW_ID).value;
switch (viewId) {
case '':
return undefined;
case '<null>':
return null;
default:
return viewRef;
return viewId;
}
}

/**
* Enables the field for setting a view reference when choosing not to close the popup on focus loss.
* Enables the field for setting a contextual view reference when choosing not to close the popup on focus loss.
*/
private installViewRefEnabler(): void {
private installContextualViewIdEnabler(): void {
this.form.get([CLOSE_STRATEGY, ON_FOCUS_LOST]).valueChanges
.pipe(takeUntil(this._destroy$))
.subscribe(closeOnFocusLost => {
if (closeOnFocusLost) {
this.form.get(VIEW_REF).setValue('');
this.form.get(VIEW_REF).disable();
this.form.get(CONTEXTUAL_VIEW_ID).setValue('');
this.form.get(CONTEXTUAL_VIEW_ID).disable();
}
else {
this.form.get(VIEW_REF).enable();
this.form.get(CONTEXTUAL_VIEW_ID).enable();
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,10 @@ export class PopupOpenerPagePO {
}
}

public async enterViewRef(viewRef: string | '<null>'): Promise<void> {
public async enterContextualViewId(viewId: string | '<null>'): Promise<void> {
await WebdriverExecutionContexts.switchToDefault();
await assertPageToDisplay(this._pageFinder);
await enterText(viewRef, this._pageFinder.$('input.e2e-view-ref'));
await enterText(viewId, this._pageFinder.$('input.e2e-contextual-view-id'));
}

public async clickOpen(): Promise<void> {
Expand Down
4 changes: 2 additions & 2 deletions projects/scion/e2e-testing/src/workbench/popup.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ describe('Workbench Popup', () => {

const popupOpenerPagePO = await PopupOpenerPagePO.openInNewTab();
await popupOpenerPagePO.enterCssClass('testee');
await popupOpenerPagePO.enterViewRef(startPagePO.viewId);
await popupOpenerPagePO.enterContextualViewId(startPagePO.viewId);
await popupOpenerPagePO.selectAnchor('coordinate');
await popupOpenerPagePO.enterCloseStrategy({closeOnFocusLost: false});
await popupOpenerPagePO.clickOpen();
Expand Down Expand Up @@ -457,7 +457,7 @@ describe('Workbench Popup', () => {
const popupOpenerPagePO = await PopupOpenerPagePO.openInNewTab();
await popupOpenerPagePO.enterCssClass('testee');
await popupOpenerPagePO.enterCloseStrategy({closeOnFocusLost: false});
await popupOpenerPagePO.enterViewRef('<null>');
await popupOpenerPagePO.enterContextualViewId('<null>');
await popupOpenerPagePO.selectAnchor('coordinate');
await popupOpenerPagePO.clickOpen();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,7 @@ export interface ɵWorkbenchPopupCommand { // tslint:disable-line:class-name
params: Map<string, any>;
align?: 'east' | 'west' | 'north' | 'south';
closeStrategy?: CloseStrategy;
viewId?: string;
context?: {
viewId?: string;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ export class WorkbenchPopupService {
...Maps.coerce(config.params),
...Maps.coerce(qualifier),
]),
viewId: Beans.opt(WorkbenchView)?.viewId,
context: {
viewId: Beans.opt(WorkbenchView)?.viewId,
},
};
const popupOriginPublisher = this.observePopupOrigin$(config).subscribe(origin => {
Beans.get(MessageClient).publish<ClientRect>(ɵWorkbenchCommands.popupOriginTopic(popupCommand.popupId), origin, {retain: true});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class MicrofrontendPopupCommandHandler {
component: MicrofrontendPopupComponent,
input: popupContext,
anchor: this.observePopupOrigin$(command),
viewRef: command.viewId,
context: command.context,
align: command.align,
size: popupCapability.properties?.size,
closeStrategy: {
Expand All @@ -62,16 +62,17 @@ export class MicrofrontendPopupCommandHandler {
}

private observePopupOrigin$(command: ɵWorkbenchPopupCommand): Observable<PopupOrigin> {
const contextualViewId = command.context?.viewId;
return combineLatest([
command.viewId ? this.observeViewBoundingBox$(command.viewId) : of(undefined),
contextualViewId ? this.observeViewBoundingBox$(contextualViewId) : of(undefined),
this.observeMicrofrontendPopupOrigin$(command.popupId),
])
.pipe(
filter(([viewBoundingBox, popupOrigin]) => {
// Swallow emissions until both sources report a non-empty dimension. For example, when deactivating
// the popup's contextual view, the view reports an empty bounding box, causing the popup to flicker
// when activating it again.
return !isNullClientRect(viewBoundingBox) && !isNullClientRect(popupOrigin);
return (!viewBoundingBox || !isNullClientRect(viewBoundingBox)) && !isNullClientRect(popupOrigin);
}),
map(([viewBoundingBox, popupOrigin]: [ClientRect | undefined, ClientRect]) => {
return {
Expand Down
20 changes: 10 additions & 10 deletions projects/scion/workbench/src/lib/popup/popup.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,17 @@ export abstract class PopupConfig {
*/
public readonly closeStrategy?: CloseStrategy;
/**
* Allows sticking the popup to the lifecycle of a workbench view so that it is only displayed when the referenced view is the active view
* in its viewpart, or closed when closing the view.
*
* If you open the popup directly from a workbench view component, you do not need to set the view reference - it is already set by the popup
* service. You only need to set the view reference if you open the popup from a service that is not provided at the view component level, i.e.,
* a service provided at the root injector, e.g., if you annotate the service with `@Injectable({providedIn: 'root'})`.
*
* Note that binding the popup to a workbench view only makes sense if it is opened from a view, and only if the popup does not close on focus loss.
* You can set the closing strategy via {@link closeStrategy} property.
* Specifies the context in which to display the popup.
*/
public readonly viewRef?: string;
public readonly context?: {
/**
* Allows sticking the popup to the lifecycle of a workbench view so that it is only displayed when the referenced view is the active view
* in its viewpart, or closed when closing the view.
*
* By default, when opening the popup in the context of a view, that view is used as the popup's contextual view.
*/
viewId?: string;
};
/**
* Specifies the preferred popup size. If not specified, the popup adjusts its size to the content size.
*/
Expand Down
10 changes: 5 additions & 5 deletions projects/scion/workbench/src/lib/popup/popup.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ export class PopupService {
.subscribe(() => popupHandle.close());
}
// If in the context of a view, hide the popup when inactivating the view, or close it when closing the view.
else if (config.viewRef || (config.viewRef === undefined /* undefined, not null */ && this._view)) {
const view = this.resolveViewFromContext(config.viewRef);
else if (config.context?.viewId || (config.context?.viewId === undefined /* undefined, not null */ && this._view)) {
const view = this.resolveViewFromContext(config.context?.viewId);
overlayRef.overlayElement.classList.add('wb-view-context');

// Hide the popup when inactivating the view.
Expand Down Expand Up @@ -244,9 +244,9 @@ export class PopupService {
}
}

private resolveViewFromContext(viewRef: string | undefined): WorkbenchView {
if (viewRef) {
return this._viewRegistry.getElseThrow(viewRef);
private resolveViewFromContext(contextualViewId: string | undefined): WorkbenchView {
if (contextualViewId) {
return this._viewRegistry.getElseThrow(contextualViewId);
}

if (!this._view) {
Expand Down

0 comments on commit 0591e7a

Please sign in to comment.