Skip to content

Commit

Permalink
Revert "Create Angular mixins for fractional-width columns and groupa…
Browse files Browse the repository at this point in the history
…ble columns (#1591)

# Pull Request

## 🤨 Rationale
Turns out the pattern introduced in #1587 for [using TypeScript mixins
does not work for Angular
directives](angular/angular#42594 (comment))
and resulted in [build failures for a built library used outside the
workspace](https://dev.azure.com/ni/DevCentral/_build/results?buildId=6361598&view=logs&j=5cb0c4d9-f36d-5fb3-bf2e-85320ef8bd68&t=51824763-f056-5c1e-0120-b664705fb575&l=71).

Reading the linked comment it sounds like we are seeing the exact
behavior described. `@Input` bindings used in the base class (column-id)
are failing to resolve. Even the extra sinister part where it works in
the workspace (our tests do [test bindings to base class props like
column-id](https://github.com/ni/nimble/blob/main/angular-workspace/projects/ni/nimble-angular/table-column/anchor/tests/nimble-table-column-anchor.directive.spec.ts#L169))
but not in apps using the published library 😵.

## 👩‍💻 Implementation

Revert the refactor

## 🧪 Testing

Rely on CI

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed. Nada
  • Loading branch information
rajsite authored Oct 6, 2023
1 parent 007f12c commit 9ebd4e1
Show file tree
Hide file tree
Showing 11 changed files with 272 additions and 115 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Directive, ElementRef, Input, Renderer2 } from '@angular/core';
import { type TableColumnAnchor, tableColumnAnchorTag } from '@ni/nimble-components/dist/esm/table-column/anchor';
import { AnchorAppearance } from '@ni/nimble-components/dist/esm/anchor/types';
import { BooleanValueOrAttribute, toBooleanProperty } from '@ni/nimble-angular/internal-utilities';
import { NimbleTableColumnBaseDirective, mixinFractionalWidthColumnAPI, mixinGroupableColumnAPI } from '@ni/nimble-angular/table-column';
import { BooleanValueOrAttribute, NumberValueOrAttribute, toBooleanProperty, toNullableNumberProperty } from '@ni/nimble-angular/internal-utilities';
import { NimbleTableColumnBaseDirective } from '@ni/nimble-angular/table-column';

export type { TableColumnAnchor };
export { tableColumnAnchorTag };
Expand All @@ -14,7 +14,7 @@ export { AnchorAppearance };
@Directive({
selector: 'nimble-table-column-anchor'
})
export class NimbleTableColumnAnchorDirective extends mixinFractionalWidthColumnAPI(mixinGroupableColumnAPI(NimbleTableColumnBaseDirective<TableColumnAnchor>)) {
export class NimbleTableColumnAnchorDirective extends NimbleTableColumnBaseDirective<TableColumnAnchor> {
public get labelFieldName(): string | undefined {
return this.elementRef.nativeElement.labelFieldName;
}
Expand Down Expand Up @@ -109,6 +109,46 @@ export class NimbleTableColumnAnchorDirective extends mixinFractionalWidthColumn
this.renderer.setProperty(this.elementRef.nativeElement, 'download', value);
}

public get fractionalWidth(): number | null | undefined {
return this.elementRef.nativeElement.fractionalWidth;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('fractional-width') public set fractionalWidth(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'fractionalWidth', toNullableNumberProperty(value));
}

public get minPixelWidth(): number | null | undefined {
return this.elementRef.nativeElement.minPixelWidth;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('min-pixel-width') public set minPixelWidth(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'minPixelWidth', toNullableNumberProperty(value));
}

public get groupIndex(): number | null | undefined {
return this.elementRef.nativeElement.groupIndex;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('group-index') public set groupIndex(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'groupIndex', toNullableNumberProperty(value));
}

public get groupingDisabled(): boolean {
return this.elementRef.nativeElement.groupingDisabled;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('grouping-disabled') public set groupingDisabled(value: BooleanValueOrAttribute) {
this.renderer.setProperty(this.elementRef.nativeElement, 'groupingDisabled', toBooleanProperty(value));
}

public constructor(renderer: Renderer2, elementRef: ElementRef<TableColumnAnchor>) {
super(renderer, elementRef);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Directive, ElementRef, Input, Renderer2 } from '@angular/core';
import { type TableColumnDateText, tableColumnDateTextTag } from '@ni/nimble-components/dist/esm/table-column/date-text';
import { NimbleTableColumnBaseDirective, mixinFractionalWidthColumnAPI, mixinGroupableColumnAPI } from '@ni/nimble-angular/table-column';
import { BooleanValueOrAttribute, NumberValueOrAttribute, toBooleanProperty, toNullableNumberProperty } from '@ni/nimble-angular/internal-utilities';
import { NimbleTableColumnBaseDirective } from '@ni/nimble-angular/table-column';
import {
DateTextFormat,
DateStyle,
Expand Down Expand Up @@ -45,7 +46,7 @@ export { tableColumnDateTextTag };
@Directive({
selector: 'nimble-table-column-date-text'
})
export class NimbleTableColumnDateTextDirective extends mixinFractionalWidthColumnAPI(mixinGroupableColumnAPI(NimbleTableColumnBaseDirective<TableColumnDateText>)) {
export class NimbleTableColumnDateTextDirective extends NimbleTableColumnBaseDirective<TableColumnDateText> {
public get fieldName(): string | undefined {
return this.elementRef.nativeElement.fieldName;
}
Expand Down Expand Up @@ -262,6 +263,46 @@ export class NimbleTableColumnDateTextDirective extends mixinFractionalWidthColu
this.renderer.setProperty(this.elementRef.nativeElement, 'customHourCycle', value);
}

public get fractionalWidth(): number | null | undefined {
return this.elementRef.nativeElement.fractionalWidth;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('fractional-width') public set fractionalWidth(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'fractionalWidth', toNullableNumberProperty(value));
}

public get minPixelWidth(): number | null | undefined {
return this.elementRef.nativeElement.minPixelWidth;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('min-pixel-width') public set minPixelWidth(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'minPixelWidth', toNullableNumberProperty(value));
}

public get groupIndex(): number | null | undefined {
return this.elementRef.nativeElement.groupIndex;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('group-index') public set groupIndex(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'groupIndex', toNullableNumberProperty(value));
}

public get groupingDisabled(): boolean {
return this.elementRef.nativeElement.groupingDisabled;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('grouping-disabled') public set groupingDisabled(value: BooleanValueOrAttribute) {
this.renderer.setProperty(this.elementRef.nativeElement, 'groupingDisabled', toBooleanProperty(value));
}

public constructor(renderer: Renderer2, elementRef: ElementRef<TableColumnDateText>) {
super(renderer, elementRef);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Directive, ElementRef, Input, Renderer2 } from '@angular/core';
import { type TableColumnEnumText, tableColumnEnumTextTag } from '@ni/nimble-components/dist/esm/table-column/enum-text';
import { NimbleTableColumnBaseDirective, mixinFractionalWidthColumnAPI, mixinGroupableColumnAPI } from '@ni/nimble-angular/table-column';
import { BooleanValueOrAttribute, NumberValueOrAttribute, toBooleanProperty, toNullableNumberProperty } from '@ni/nimble-angular/internal-utilities';
import { NimbleTableColumnBaseDirective } from '@ni/nimble-angular/table-column';
import { MappingKeyType } from '@ni/nimble-components/dist/esm/table-column/enum-base/types';

export { MappingKeyType };
Expand All @@ -13,7 +14,7 @@ export { tableColumnEnumTextTag };
@Directive({
selector: 'nimble-table-column-enum-text'
})
export class NimbleTableColumnEnumTextDirective extends mixinFractionalWidthColumnAPI(mixinGroupableColumnAPI(NimbleTableColumnBaseDirective<TableColumnEnumText>)) {
export class NimbleTableColumnEnumTextDirective extends NimbleTableColumnBaseDirective<TableColumnEnumText> {
public get fieldName(): string | undefined {
return this.elementRef.nativeElement.fieldName;
}
Expand All @@ -34,6 +35,46 @@ export class NimbleTableColumnEnumTextDirective extends mixinFractionalWidthColu
this.renderer.setProperty(this.elementRef.nativeElement, 'keyType', value);
}

public get fractionalWidth(): number | null | undefined {
return this.elementRef.nativeElement.fractionalWidth;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('fractional-width') public set fractionalWidth(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'fractionalWidth', toNullableNumberProperty(value));
}

public get minPixelWidth(): number | null | undefined {
return this.elementRef.nativeElement.minPixelWidth;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('min-pixel-width') public set minPixelWidth(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'minPixelWidth', toNullableNumberProperty(value));
}

public get groupIndex(): number | null | undefined {
return this.elementRef.nativeElement.groupIndex;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('group-index') public set groupIndex(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'groupIndex', toNullableNumberProperty(value));
}

public get groupingDisabled(): boolean {
return this.elementRef.nativeElement.groupingDisabled;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('grouping-disabled') public set groupingDisabled(value: BooleanValueOrAttribute) {
this.renderer.setProperty(this.elementRef.nativeElement, 'groupingDisabled', toBooleanProperty(value));
}

public constructor(renderer: Renderer2, elementRef: ElementRef<TableColumnEnumText>) {
super(renderer, elementRef);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Directive, ElementRef, Input, Renderer2 } from '@angular/core';
import { type TableColumnIcon, tableColumnIconTag } from '@ni/nimble-components/dist/esm/table-column/icon';
import { NimbleTableColumnBaseDirective, mixinFractionalWidthColumnAPI, mixinGroupableColumnAPI } from '@ni/nimble-angular/table-column';
import { BooleanValueOrAttribute, NumberValueOrAttribute, toBooleanProperty, toNullableNumberProperty } from '@ni/nimble-angular/internal-utilities';
import { NimbleTableColumnBaseDirective } from '@ni/nimble-angular/table-column';
import type { MappingKeyType } from '@ni/nimble-components/dist/esm/table-column/enum-base/types';

export type { TableColumnIcon };
Expand All @@ -12,7 +13,7 @@ export { tableColumnIconTag };
@Directive({
selector: 'nimble-table-column-icon'
})
export class NimbleTableColumnIconDirective extends mixinFractionalWidthColumnAPI(mixinGroupableColumnAPI(NimbleTableColumnBaseDirective<TableColumnIcon>)) {
export class NimbleTableColumnIconDirective extends NimbleTableColumnBaseDirective<TableColumnIcon> {
public get fieldName(): string | undefined {
return this.elementRef.nativeElement.fieldName;
}
Expand All @@ -33,6 +34,46 @@ export class NimbleTableColumnIconDirective extends mixinFractionalWidthColumnAP
this.renderer.setProperty(this.elementRef.nativeElement, 'keyType', value);
}

public get fractionalWidth(): number | null | undefined {
return this.elementRef.nativeElement.fractionalWidth;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('fractional-width') public set fractionalWidth(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'fractionalWidth', toNullableNumberProperty(value));
}

public get minPixelWidth(): number | null | undefined {
return this.elementRef.nativeElement.minPixelWidth;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('min-pixel-width') public set minPixelWidth(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'minPixelWidth', toNullableNumberProperty(value));
}

public get groupIndex(): number | null | undefined {
return this.elementRef.nativeElement.groupIndex;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('group-index') public set groupIndex(value: NumberValueOrAttribute | null | undefined) {
this.renderer.setProperty(this.elementRef.nativeElement, 'groupIndex', toNullableNumberProperty(value));
}

public get groupingDisabled(): boolean {
return this.elementRef.nativeElement.groupingDisabled;
}

// Renaming because property should have camel casing, but attribute should not
// eslint-disable-next-line @angular-eslint/no-input-rename
@Input('grouping-disabled') public set groupingDisabled(value: BooleanValueOrAttribute) {
this.renderer.setProperty(this.elementRef.nativeElement, 'groupingDisabled', toBooleanProperty(value));
}

public constructor(renderer: Renderer2, elementRef: ElementRef<TableColumnIcon>) {
super(renderer, elementRef);
}
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,7 @@ export class NimbleTableColumnBaseDirective<T extends TableColumn> {
this.renderer.setProperty(this.elementRef.nativeElement, 'sortIndex', toNullableNumberProperty(value));
}

/** @internal */
public readonly renderer: Renderer2;
/** @internal */
public readonly elementRef: ElementRef<T>;

public constructor(renderer: Renderer2, elementRef: ElementRef<T>) {
this.renderer = renderer;
this.elementRef = elementRef;
}
public constructor(protected readonly renderer: Renderer2, protected readonly elementRef: ElementRef<T>) {}

public checkValidity(): boolean {
return this.elementRef.nativeElement.checkValidity();
Expand Down
Loading

0 comments on commit 9ebd4e1

Please sign in to comment.