Skip to content
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
@import 'font';
@import 'color-palette';

:host {
overflow: hidden;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this needed?

}

.string-enum-cell {
@include ellipsis-overflow();
@include body-1-regular($gray-7);

&.first-column {
@include body-1-medium($gray-9);
}
}

.clickable {
@include link-hover();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { FormattingModule } from '@hypertrace/common';
import { byText, createComponentFactory } from '@ngneat/spectator/jest';
import { TableCellStringParser } from '../../data-parsers/table-cell-string-parser';
import { tableCellColumnProvider, tableCellDataProvider, tableCellProviders } from '../../test/cell-providers';
import { StringEnumTableCellRendererComponent } from './string-enum-table-cell-renderer.component';

describe('String Enum table cell renderer component', () => {
const buildComponent = createComponentFactory({
component: StringEnumTableCellRendererComponent,
imports: [FormattingModule],
providers: [
tableCellProviders(
{
id: 'test'
},
new TableCellStringParser(undefined!)
)
],
shallow: true
});

test('should render a plain string', () => {
const spectator = buildComponent({
providers: [tableCellDataProvider('test-text')]
});

expect(spectator.element).toHaveText('Test text');
});

test('should render a missing string', () => {
const spectator = buildComponent();

expect(spectator.element).toHaveText('');
});

test('should add clickable class for clickable columns', () => {
const spectator = buildComponent({
providers: [
tableCellDataProvider('test-text'),
tableCellColumnProvider({
id: 'test',
onClick: () => {
/* NOOP */
}
})
]
});

expect(spectator.query(byText('Test text'))).toHaveClass('clickable');
});

test('should not add clickable class for columns without a click handler', () => {
const spectator = buildComponent({
providers: [tableCellDataProvider('TEST_TEXT')]
});

expect(spectator.query(byText('Test text'))).not.toHaveClass('clickable');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { ChangeDetectionStrategy, Component } from '@angular/core';
import { TableCellRenderer } from '../../table-cell-renderer';
import { TableCellRendererBase } from '../../table-cell-renderer-base';
import { CoreTableCellParserType } from '../../types/core-table-cell-parser-type';
import { CoreTableCellRendererType } from '../../types/core-table-cell-renderer-type';
import { TableCellAlignmentType } from '../../types/table-cell-alignment-type';

@Component({
selector: 'ht-string-enum-table-cell-renderer',
styleUrls: ['./string-enum-table-cell-renderer.component.scss'],
changeDetection: ChangeDetectionStrategy.OnPush,
template: `
<div
[ngClass]="{ clickable: this.clickable, 'first-column': this.isFirstColumn }"
Copy link
Contributor

Choose a reason for hiding this comment

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

what advantage do we get compared to directly supplying the startcased enum value to a regular string cell renderer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing other than this does the text presentation for you. It has been coming up more and more, so this is a convenience, but it also feels (at least to me) more correct to have the presentation logic in the cell renderer rather than pre-formatting it before passing it in.

class="string-enum-cell"
[htTooltip]="this.value"
>
{{ this.value | htDisplayStringEnum }}
</div>
`
})
@TableCellRenderer({
type: CoreTableCellRendererType.StringEnum,
alignment: TableCellAlignmentType.Left,
parser: CoreTableCellParserType.String
})
export class StringEnumTableCellRendererComponent extends TableCellRendererBase<string> {}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: IMO, the name 'String Enum' is not very clear. Or we can at least add similar comments from htDisplayStringEnum here too

  // This removes dashes and underscores and gives all words initial caps

  // We only want the first word capitalized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I always struggle with names, but also always open to suggestions.

I've added the comment in my local. That should help some. Not worth holding up this PR for, but I'll commit them in whatever next HT PR I have.

3 changes: 3 additions & 0 deletions projects/components/src/table/cells/table-cells.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { TableCellNumberParser } from './data-parsers/table-cell-number-parser';
import { TableCellStringParser } from './data-parsers/table-cell-string-parser';
import { TableCellTimestampParser } from './data-parsers/table-cell-timestamp-parser';
import { CodeTableCellRendererComponent } from './data-renderers/code/code-table-cell-renderer.component';
import { StringEnumTableCellRendererComponent } from './data-renderers/enum/string-enum-table-cell-renderer.component';
import { IconTableCellRendererComponent } from './data-renderers/icon/icon-table-cell-renderer.component';
import { NumericTableCellRendererComponent } from './data-renderers/numeric/numeric-table-cell-renderer.component';
import { StringArrayTableCellRendererComponent } from './data-renderers/string-array/string-array-table-cell-renderer.component';
Expand Down Expand Up @@ -70,6 +71,7 @@ export const TABLE_CELL_PARSERS = new InjectionToken<unknown[][]>('TABLE_CELL_PA
TimeAgoTableCellRendererComponent,
CodeTableCellRendererComponent,
StringArrayTableCellRendererComponent,
StringEnumTableCellRendererComponent,
TextWithCopyActionTableCellRendererComponent
],
providers: [
Expand All @@ -85,6 +87,7 @@ export const TABLE_CELL_PARSERS = new InjectionToken<unknown[][]>('TABLE_CELL_PA
TimeAgoTableCellRendererComponent,
CodeTableCellRendererComponent,
StringArrayTableCellRendererComponent,
StringEnumTableCellRendererComponent,
TextWithCopyActionTableCellRendererComponent
],
multi: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export const enum CoreTableCellRendererType {
Number = 'number',
RowExpander = 'row-expander',
StringArray = 'string-array',
StringEnum = 'string-enum',
Text = 'text',
TextWithCopyAction = 'text-with-copy-action',
Timestamp = 'timestamp',
Expand Down
15 changes: 15 additions & 0 deletions src/styles/main.scss
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,21 @@ h1 {
box-sizing: border-box;
}

strong {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to this change, but needed for another.

font-weight: 500;
}

em {
font-style: normal;
background: $gray-2;
border-radius: 4px;
padding: 2px 4px;
}

q {
Copy link
Contributor

Choose a reason for hiding this comment

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

what are these for?

color: $gray-5;
}

mark {
background-color: $color-text-highlight;
color: $gray-9;
Expand Down