Skip to content

Conversation

jake-bassett
Copy link
Contributor

Description

Added table cell renderer to format string enums into more human readable text.

@jake-bassett jake-bassett requested a review from a team as a code owner April 20, 2021 22:04
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.

@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

Merging #787 (41354b5) into main (6a68582) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #787      +/-   ##
==========================================
+ Coverage   85.40%   85.42%   +0.01%     
==========================================
  Files         791      792       +1     
  Lines       16156    16161       +5     
  Branches     2060     2060              
==========================================
+ Hits        13798    13805       +7     
+ Misses       2327     2325       -2     
  Partials       31       31              
Impacted Files Coverage Δ
.../enum/string-enum-table-cell-renderer.component.ts 100.00% <100.00%> (ø)
...s/components/src/table/cells/table-cells.module.ts 100.00% <100.00%> (ø)
...lities/formatters/enum/display-string-enum.pipe.ts 100.00% <0.00%> (+40.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a68582...41354b5. Read the comment docs.

@github-actions

This comment has been minimized.

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.

@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?

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.

@github-actions

This comment has been minimized.

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?

@github-actions

This comment has been minimized.

@jake-bassett jake-bassett merged commit fa80f9a into main Apr 21, 2021
@jake-bassett jake-bassett deleted the string-enum-table-cell-renderer branch April 21, 2021 17:21
@github-actions
Copy link

Unit Test Results

    4 files  ±0  250 suites  +1   16m 25s ⏱️ + 1m 13s
897 tests +4  897 ✔️ +4  0 💤 ±0  0 ❌ ±0 
903 runs  +4  903 ✔️ +4  0 💤 ±0  0 ❌ ±0 

Results for commit fa80f9a. ± Comparison against base commit 6a68582.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants