Skip to content

feat: header functionality for the list view #806

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.

Sign up for GitHub

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 8 commits into from
May 4, 2021
Merged

feat: header functionality for the list view #806

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c5148b0
feat: header functionality in the list view
itssharmasandeep Apr 30, 2021
9c9b6b0
Merge remote-tracking branch origin/main into list-view-header
itssharmasandeep Apr 30, 2021
f1403f0
fix: addressing review comments
itssharmasandeep Apr 30, 2021
b02a5a7
Merge branch 'main' into list-view-header
itssharmasandeep May 3, 2021
d547d83
fix: addressing review comments
itssharmasandeep May 3, 2021
6e799a6
Merge branch 'list-view-header' of github.com:hypertrace/hypertrace-u…
itssharmasandeep May 3, 2021
cd27f4f
fix: addressing review comment
itssharmasandeep May 4, 2021
add59cf
Merge branch 'main' into list-view-header
itssharmasandeep May 4, 2021
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
65 changes: 39 additions & 26 deletions projects/components/src/list-view/list-view.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,46 +1,59 @@
@import 'font';
@import 'color-palette';

$key-width: 40%;
$value-width: 60%;
$margin-left: 12px;

@mixin grid-view {
width: 100%;
padding: 8px 0;
display: grid;
grid-template-columns: $key-width $value-width;
align-content: center;
}

.list-view {
height: 100%;
width: 100%;
overflow: auto;
display: flex;
flex-direction: column;

.data-row {
flex: 0 0 auto;
min-height: 40px;
width: 100%;
word-break: break-word;
padding: 8px 0;
& > * {
Copy link
Contributor

@arjunlalb arjunlalb May 4, 2021

Choose a reason for hiding this comment

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

list-view could have a huge number of children. In such cases, this does us more harm than good. We should use a class name here instead of the wildcard.

@include grid-view();
Copy link
Contributor

@anandtiwary anandtiwary May 1, 2021

Choose a reason for hiding this comment

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

Nice work isolating column width as variables.

I think you can just add these grid properties to list-view class. No need to add it in both header and data row with flex column in list view.

Copy link
Contributor Author

@itssharmasandeep itssharmasandeep May 3, 2021

Choose a reason for hiding this comment

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

Fixed!!

Copy link
Contributor

@anandtiwary anandtiwary May 3, 2021

Choose a reason for hiding this comment

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

Oh I was thinking more like this

.list-view{
  height: 100%;
  width: 100%;
  overflow: auto;
  width: 100%;
  padding: 8px 0;
  display: grid;
  grid-template-columns: $key-width $value-width;
  align-content: center;
  
  
  .header-key-label{}
  
  .header-value-label{}
  
  .data-key{}
  
  .data-value{} 
}

Is this something we can do?

Copy link
Contributor Author

@itssharmasandeep itssharmasandeep May 3, 2021

Choose a reason for hiding this comment

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

But then we'll have to put some styles twice like

  1. Header shadow and background
  2. Alternate background for data row and column etc.

Copy link
Contributor Author

@itssharmasandeep itssharmasandeep May 4, 2021

Choose a reason for hiding this comment

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

if needed for this, we can take this as follow up work.
I am merging changes for now.

}

display: flex;
flex-direction: row;
align-items: center;
font-size: 15px;
font-style: normal;
line-height: 18px;
.header-row {
height: 32px;
background: $gray-1;
box-shadow: inset 0px -1px 0px $gray-2;
border-radius: 6px 6px 0px 0px;

.key {
flex: 1 1 auto;
height: 100%;
width: 40%;
display: flex;
align-items: center;
margin-left: 12px;
font-weight: 500;
.header-key-label,
.header-value-label {
@include overline;
margin-left: $margin-left;
width: 100%;
}
}

.data-row {
@include font-placeholder();
color: $gray-9;
min-height: 40px;
word-break: break-word;

.key,
.value {
flex: 1 1 auto;
height: 100%;
width: 60%;
width: 100%;
display: flex;
align-items: center;
margin-left: 12px;

/* identical to box height, or 120% */
letter-spacing: -0.01em;
margin-left: $margin-left;
}
.key {
font-weight: 500;
}
}

Expand Down
22 changes: 19 additions & 3 deletions projects/components/src/list-view/list-view.component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { CommonModule } from '@angular/common';
import { Component } from '@angular/core';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { ListViewComponent, ListViewRecord } from './list-view.component';
import { ListViewComponent, ListViewHeader, ListViewRecord } from './list-view.component';

describe('List View Component', () => {
@Component({
selector: 'ht-test-host-component',
template: ` <ht-list-view [records]="this.records"></ht-list-view> `
template: ` <ht-list-view [records]="this.records" [header]="this.header"></ht-list-view> `
})
class TestHostComponent {
public records: ListViewRecord[] = [
Expand All @@ -19,6 +19,11 @@ describe('List View Component', () => {
value: 'Response 2'
}
];

public header: ListViewHeader = {
keyLabel: 'key',
valueLabel: 'value'
};
}

let fixture: ComponentFixture<TestHostComponent>;
Expand All @@ -34,11 +39,22 @@ describe('List View Component', () => {
hostComp = fixture.componentInstance;
});

test('should display rows for each data', () => {
test('should display rows for each data and should display header', () => {
fixture.detectChanges();

const element: HTMLElement = fixture.nativeElement;

const headerElement = element.querySelector<HTMLElement>('.header-row');
expect(headerElement).not.toBeNull();

const headerKeyLabelElement = element.querySelector<HTMLElement>('.header-key-label');
expect(headerKeyLabelElement).not.toBeNull();
expect(headerKeyLabelElement?.textContent).toEqual(hostComp.header.keyLabel);

const headerValueLabelElement = element.querySelector<HTMLElement>('.header-value-label');
expect(headerValueLabelElement).not.toBeNull();
expect(headerValueLabelElement?.textContent).toEqual(hostComp.header.valueLabel);

const rowElements = element.querySelectorAll<HTMLElement>('.data-row');
expect(rowElements).not.toBeNull();
expect(rowElements.length).toBe(2);
Expand Down
16 changes: 16 additions & 0 deletions projects/components/src/list-view/list-view.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
changeDetection: ChangeDetectionStrategy.OnPush,
template: `
<div class="list-view">
<div *ngIf="this.header" class="header-row">
<div class="header-key-label">
<span>{{ this.header.keyLabel }}</span>
</div>
<div class="header-value-label">
<span>{{ this.header.valueLabel }}</span>
</div>
</div>
<div class="data-row" *ngFor="let record of this.records">
<div class="key">
<span>{{ record.key }}</span>
Expand All @@ -18,10 +26,18 @@ import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
`
})
export class ListViewComponent {
@Input()
public header?: ListViewHeader;

@Input()
public records?: ListViewRecord[];
}

export interface ListViewHeader {
keyLabel: string;
valueLabel: string;
}

export interface ListViewRecord {
key: string;
value: string | number;
Expand Down
2 changes: 1 addition & 1 deletion projects/components/src/public-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export * from './link/link.component';
export * from './link/link.module';

// List View
export { ListViewComponent, ListViewRecord } from './list-view/list-view.component';
export { ListViewComponent, ListViewHeader, ListViewRecord } from './list-view/list-view.component';
export { ListViewModule } from './list-view/list-view.module';

// Load Async
Expand Down