Skip to content

feat(popover-edit): Adds support for spanning multiple columns and se… #15724

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/cdk-experimental/popover-edit/edit-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {Injectable, OnDestroy, Self} from '@angular/core';
import {ControlContainer} from '@angular/forms';
import {Subject} from 'rxjs';
import {Observable, Subject} from 'rxjs';
import {take} from 'rxjs/operators';

import {EditEventDispatcher} from './edit-event-dispatcher';
Expand All @@ -21,7 +21,7 @@ import {EditEventDispatcher} from './edit-event-dispatcher';
export class EditRef<FormValue> implements OnDestroy {
/** Emits the final value of this edit instance before closing. */
private readonly _finalValueSubject = new Subject<FormValue>();
readonly finalValue = this._finalValueSubject.asObservable();
readonly finalValue: Observable<FormValue> = this._finalValueSubject.asObservable();

/** The value to set the form back to on revert. */
private _revertFormValue: FormValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,27 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ElementRef, Injectable} from '@angular/core';
import {Overlay, PositionStrategy} from '@angular/cdk/overlay';
import {Directionality} from '@angular/cdk/bidi';
import {Overlay, OverlaySizeConfig, PositionStrategy} from '@angular/cdk/overlay';
import {Injectable} from '@angular/core';

/**
* Overridable factory responsible for configuring how cdkPopoverEdit popovers are positioned.
* Overridable factory responsible for configuring how cdkPopoverEdit popovers are positioned
* and sized.
*/
@Injectable()
export abstract class PopoverEditPositionStrategyFactory {
abstract forElementRef(elementRef: ElementRef): PositionStrategy;
/**
* Creates a PositionStrategy based on the specified table cells.
* The cells will be provided in DOM order.
*/
abstract positionStrategyForCells(cells: HTMLElement[]): PositionStrategy;

/**
* Creates an OverlaySizeConfig based on the specified table cells.
* The cells will be provided in DOM order.
*/
abstract sizeConfigForCells(cells: HTMLElement[]): OverlaySizeConfig;
}

/**
Expand All @@ -24,19 +36,42 @@ export abstract class PopoverEditPositionStrategyFactory {
*/
@Injectable()
export class DefaultPopoverEditPositionStrategyFactory extends PopoverEditPositionStrategyFactory {
constructor(protected readonly overlay: Overlay) {
constructor(protected readonly direction: Directionality, protected readonly overlay: Overlay) {
Copy link
Member

Choose a reason for hiding this comment

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

The readonly part here seems a little redudant. If somebody decides to change these, they either know what they're doing or they're shooting themselves in the foot.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I disagree - if someone wants to override these they can do it via the DI system.

Maybe this comes down to philosophy, but IMO readonly and const should be used everywhere that mutability is not a requirement.

super();
}

forElementRef(elementRef: ElementRef): PositionStrategy {
return this.overlay.position().flexibleConnectedTo(elementRef)
positionStrategyForCells(cells: HTMLElement[]): PositionStrategy {
return this.overlay.position()
.flexibleConnectedTo(cells[0])
.withGrowAfterOpen()
.withPush()
.withViewportMargin(16)
.withPositions([{
originX: 'start',
originY: 'top',
overlayX: 'start',
overlayY: 'top',
}]);
}

sizeConfigForCells(cells: HTMLElement[]): OverlaySizeConfig {
if (cells.length === 0) {
return {};
}

if (cells.length === 1) {
return {width: cells[0].getBoundingClientRect().width};
}

let firstCell, lastCell;
if (this.direction.value === 'ltr') {
firstCell = cells[0];
lastCell = cells[cells.length - 1];
} else {
lastCell = cells[0];
firstCell = cells[cells.length - 1];
}

return {width: lastCell.getBoundingClientRect().right - firstCell.getBoundingClientRect().left};
}
}
94 changes: 81 additions & 13 deletions src/cdk-experimental/popover-edit/popover-edit.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {BehaviorSubject} from 'rxjs';
import {Component, ElementRef, Type, ViewChild} from '@angular/core';
import {CommonModule} from '@angular/common';
import {FormsModule, NgForm} from '@angular/forms';
import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
import {DataSource} from '@angular/cdk/collections';
import {CdkTableModule} from '@angular/cdk/table';
import {CdkPopoverEditModule, PopoverEditClickOutBehavior} from './index';
import {CommonModule} from '@angular/common';
import {Component, ElementRef, Type, ViewChild} from '@angular/core';
import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
import {FormsModule, NgForm} from '@angular/forms';
import {BehaviorSubject} from 'rxjs';

import {CdkPopoverEditColspan, CdkPopoverEditModule, PopoverEditClickOutBehavior} from './index';

const EDIT_TEMPLATE = `
<div style="background-color: white;">
Expand Down Expand Up @@ -33,6 +34,8 @@ const CELL_TEMPLATE = `
</span>
`;

const POPOVER_EDIT_DIRECTIVE = `[cdkPopoverEdit]="nameEdit" [cdkPopoverEditColspan]="colspan"`;

interface PeriodicElement {
name: string;
weight: number;
Expand All @@ -45,6 +48,7 @@ abstract class BaseTestComponent {

ignoreSubmitUnlessValid = true;
clickOutBehavior: PopoverEditClickOutBehavior = 'close';
colspan: CdkPopoverEditColspan = {};

onSubmit(element: PeriodicElement, form: NgForm) {
if (!form.valid) { return; }
Expand All @@ -62,7 +66,7 @@ abstract class BaseTestComponent {

getEditCell(rowIndex = 0) {
const row = getRows(this.table.nativeElement)[rowIndex];
return getCells(row)[0];
return getCells(row)[1];
}

focusEditCell(rowIndex = 0) {
Expand All @@ -84,6 +88,10 @@ abstract class BaseTestComponent {
flush();
}

getEditPane() {
return document.querySelector('.cdk-edit-pane');
}

getInput() {
return document.querySelector('input') as HTMLInputElement|null;
}
Expand Down Expand Up @@ -125,7 +133,10 @@ abstract class BaseTestComponent {
</ng-template>

<tr *ngFor="let element of elements">
<td [cdkPopoverEdit]="nameEdit" [cdkPopoverEditContext]="element">
<td> just a cell </td>

<td ${POPOVER_EDIT_DIRECTIVE}
[cdkPopoverEditContext]="element">
${CELL_TEMPLATE}
</td>

Expand All @@ -142,7 +153,9 @@ class VanillaTableOutOfCell extends BaseTestComponent {
template: `
<table #table editable>
<tr *ngFor="let element of elements">
<td [cdkPopoverEdit]="nameEdit">
<td> just a cell </td>

<td ${POPOVER_EDIT_DIRECTIVE}>
${CELL_TEMPLATE}

<ng-template #nameEdit>
Expand Down Expand Up @@ -175,9 +188,15 @@ class ElementDataSource extends DataSource<PeriodicElement> {
template: `
<div #table>
<cdk-table cdk-table editable [dataSource]="dataSource">
<ng-container cdkColumnDef="before">
<cdk-cell *cdkCellDef="let element">
just a cell
</cdk-cell>
</ng-container>

<ng-container cdkColumnDef="name">
<cdk-cell *cdkCellDef="let element"
[cdkPopoverEdit]="nameEdit">
${POPOVER_EDIT_DIRECTIVE}>
${CELL_TEMPLATE}

<ng-template #nameEdit>
Expand All @@ -202,17 +221,23 @@ class ElementDataSource extends DataSource<PeriodicElement> {
`
})
class CdkFlexTableInCell extends BaseTestComponent {
displayedColumns = ['name', 'weight'];
displayedColumns = ['before', 'name', 'weight'];
dataSource = new ElementDataSource();
}

@Component({
template: `
<div #table>
<table cdk-table editable [dataSource]="dataSource">
<ng-container cdkColumnDef="before">
<td cdk-cell *cdkCellDef="let element">
just a cell
</td>
</ng-container>

<ng-container cdkColumnDef="name">
<td cdk-cell *cdkCellDef="let element"
[cdkPopoverEdit]="nameEdit">
${POPOVER_EDIT_DIRECTIVE}>
${CELL_TEMPLATE}

<ng-template #nameEdit>
Expand All @@ -237,7 +262,7 @@ class CdkFlexTableInCell extends BaseTestComponent {
`
})
class CdkTableInCell extends BaseTestComponent {
displayedColumns = ['name', 'weight'];
displayedColumns = ['before', 'name', 'weight'];
dataSource = new ElementDataSource();
}

Expand Down Expand Up @@ -316,6 +341,49 @@ describe('CDK Popover Edit', () => {
expect(component.getInput()!.value).toBe('Hydrogen');
}));

it('positions the lens at the top left corner and spans the full width of the cell',
fakeAsync(() => {
component.openLens();

const paneRect = component.getEditPane()!.getBoundingClientRect();
const cellRect = component.getEditCell().getBoundingClientRect();

expect(paneRect.width).toBe(cellRect.width);
expect(paneRect.left).toBe(cellRect.left);
expect(paneRect.top).toBe(cellRect.top);
}));

it('adjusts the positioning of the lens based on colspan', fakeAsync(() => {
const cellRects = getCells(getRows(component.table.nativeElement)[0])
.map(cell => cell.getBoundingClientRect());

component.colspan = {before: 1};
fixture.detectChanges();

component.openLens();

let paneRect = component.getEditPane()!.getBoundingClientRect();
expect(paneRect.top).toBe(cellRects[0].top);
expect(paneRect.left).toBe(cellRects[0].left);
expect(paneRect.right).toBe(cellRects[1].right);

component.colspan = {after: 1};
fixture.detectChanges();

paneRect = component.getEditPane()!.getBoundingClientRect();
expect(paneRect.top).toBe(cellRects[1].top);
expect(paneRect.left).toBe(cellRects[1].left);
expect(paneRect.right).toBe(cellRects[2].right);

component.colspan = {before: 1, after: 1};
fixture.detectChanges();

paneRect = component.getEditPane()!.getBoundingClientRect();
expect(paneRect.top).toBe(cellRects[0].top);
expect(paneRect.left).toBe(cellRects[0].left);
expect(paneRect.right).toBe(cellRects[2].right);
}));

it('updates the form and submits, closing the lens', fakeAsync(() => {
component.openLens();

Expand Down
Loading