Skip to content

Description component improve #579

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 16 commits into from
Feb 10, 2021
Merged

Description component improve #579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7897d76
feat(description): create description component
Jan 29, 2021
16dd167
feat(description): add text truncation recalculate on resize
Feb 1, 2021
6986dac
fix(description): use cahngedetctionref
Feb 1, 2021
09afd65
fix(description): linting
DmitiryPotychkin Feb 1, 2021
aeb9df5
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into ht-de…
DmitiryPotychkin Feb 1, 2021
8f6040a
fix(description): resize event
Feb 1, 2021
c84c1b0
Merge main
Feb 1, 2021
9fd514c
fix(description): linting
DmitiryPotychkin Feb 1, 2021
58aa4fc
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into ht-de…
DmitiryPotychkin Feb 2, 2021
d0db5bf
Merge branch 'main' into ht-description
anandtiwary Feb 4, 2021
73fb491
fix(description): stop event bubbling, add option to pass content in …
Feb 8, 2021
5e0ea2f
fix(description): testing
Feb 8, 2021
bd0f11e
fix(description): resolve merge conflicts
Feb 8, 2021
53cfff7
Merge branch 'ht-description' of github.com:DmitiryPotychkin/hypertra…
Feb 8, 2021
5c132a9
Merge branch 'main' of github.com:hypertrace/hypertrace-ui into ht-de…
Feb 10, 2021
a1ea721
fix(ht-description): refactoring, remove input
Feb 10, 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
19 changes: 6 additions & 13 deletions projects/components/src/description/description.component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,18 @@ describe('Description Component', () => {
});

test('should render the description', () => {
spectator = createHost('<ht-description [description] = "description"> </ht-description>', {
hostProps: {
description: 'Description text'
}
});
spectator = createHost('<ht-description> Description text </ht-description>');

expect(spectator.query('.description')).toExist();
expect(spectator.query('.description-text')).toHaveText('Description text');
});

test('should show full description text if pressed button show more', () => {
spectator = createHost('<ht-description [description] = "description"> </ht-description>', {
hostProps: {
description: 'Description text'
}
});
spectator = createHost('<ht-description> Description text </ht-description>');

expect(spectator.component.isDescriptionTextToggled).toBeFalsy();
spectator.component.toggleDescriptionText();
expect(spectator.component.isDescriptionTextToggled).toBeTruthy();
expect(spectator.query('.description-text')).toHaveClass('description-text truncated-text');
spectator.component.isDescriptionTextToggled = true;
spectator.component.cdRef.detectChanges();
expect(spectator.query('.description-text')).not.toHaveClass('truncated-text');
});
});
15 changes: 6 additions & 9 deletions projects/components/src/description/description.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
ChangeDetectorRef,
Component,
ElementRef,
Input,
OnChanges,
ViewChild
} from '@angular/core';
Expand All @@ -21,18 +20,18 @@ import {
data-sensitive-pii
#eventDescriptionText
>
{{ description }}
<ng-content></ng-content>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not against using content to pass in the description, but it's an alternative API to passing in description as an input - we shouldn't have both. If we have use cases that need more than a simple string passed in, let's make this the sole API to pass in content and remove the description input.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

Curious to know why we need ng-content? Is their an example usage I could see? Is it suppose to wrap any other component apart from a string?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the main use cases are links and formatted strings

Copy link
Contributor

Choose a reason for hiding this comment

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

okay.. Links we should handle from outside, but i see the need for formatted strings.

<span
*ngIf="isDescriptionTruncated && isDescriptionTextToggled"
(click)="toggleDescriptionText()"
(click)="toggleDescriptionText($event)"
class="description-button"
>show less</span
>
</div>
<div
class="description-button description-button-more"
*ngIf="isDescriptionTruncated && !isDescriptionTextToggled"
(click)="toggleDescriptionText()"
(click)="toggleDescriptionText($event)"
>
show more
</div>
Expand All @@ -50,10 +49,7 @@ export class DescriptionComponent implements OnChanges, AfterViewInit {
@ViewChild('eventDescriptionContainer', { read: ElementRef })
public readonly eventDescriptionContainer!: ElementRef;

@Input()
public description!: string;

public constructor(private readonly cdRef: ChangeDetectorRef) {}
public constructor(public readonly cdRef: ChangeDetectorRef) {}

public ngOnChanges(): void {
if (this.isInitialized) {
Expand All @@ -66,7 +62,8 @@ export class DescriptionComponent implements OnChanges, AfterViewInit {
this.remeasure();
}

public toggleDescriptionText(): void {
public toggleDescriptionText(event: Event): void {
event.stopPropagation();
this.isDescriptionTextToggled = !this.isDescriptionTextToggled;
}

Expand Down