Summary list component #706
Conversation
Codecov Report
@@ Coverage Diff @@
## main #706 +/- ##
==========================================
+ Coverage 85.15% 85.17% +0.01%
==========================================
Files 782 784 +2
Lines 16036 16057 +21
Branches 2046 2047 +1
==========================================
+ Hits 13656 13677 +21
Misses 2347 2347
Partials 33 33
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
|
Could you please share the screenshot? |
| import { SummaryItem } from './summary-list-api'; | ||
|
|
||
| @Component({ | ||
| selector: 'ht-summary-list', |
There was a problem hiding this comment.
I don't love the name (when do I ever?). We already have a list-view, card-list, summary-values, table - so from the names, it's not clear how this one is different and when to use which.
There was a problem hiding this comment.
Yeah, I was scanning through our components and thinking the same thing. Open to suggestions. :-)
There was a problem hiding this comment.
Ah, of course - my own words turned against me. In that case, carry on.
projects/observability/src/shared/components/cartesian/d3/chart/cartesian-chart.ts
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
|
|
||
| public getFormattedLabel(item: SummaryItem): string { | ||
| return startCase(item.label.trim().replace('-', ' ').replace('_', ' ').toLowerCase()); | ||
| return startCase(item.label.trim().toLowerCase()); |
There was a problem hiding this comment.
trim is definitely still taken care of by startCase, the most intelligent of all cases. lower case may not be though.
aaron-steinfeld
left a comment
aaron-steinfeld
left a comment
There was a problem hiding this comment.
Please add a test for the new component, hopefully someone can come up with a more inspired name in the meantime and if not we'll go with this.
This comment has been minimized.
This comment has been minimized.
| let spectator: Spectator<SummaryListComponent>; | ||
|
|
||
| const createComponent = createComponentFactory({ | ||
| component: SummaryListComponent, |
There was a problem hiding this comment.
For a component with inputs and ouputs, using a hostfactory allows you to provide inputs and then verify it rendered correctly. This test only verifies the logic directly rather than for a given set of inputs it renders the html we want - that's what we're generally looking for in a component's tests - the functions are implementation details.
This comment has been minimized.
This comment has been minimized.
| expect(spectator.component.getValuesArray(true)).toEqual([true]); | ||
| expect(spectator.component.getValuesArray([true, false])).toEqual([true, false]); | ||
| const values = spectator.queryAll('li').map(e => e.textContent); | ||
| expect(values).toEqual(['0', '0', '1', '2', 'zero', 'zero', 'one', 'two', 'three', 'true', 'true', 'false']); |
There was a problem hiding this comment.
There was a problem hiding this comment.
It was worth updating this test just to see that video.
This comment has been minimized.
This comment has been minimized.
Description
This adds a summary list component that can be used to list out labelled data.
A few other miscellaneous things (bad, Jake!):