Add ht-description component #543
Conversation
Codecov Report
@@ Coverage Diff @@
## main #543 +/- ##
==========================================
+ Coverage 85.36% 85.38% +0.01%
==========================================
Files 763 765 +2
Lines 15674 15702 +28
Branches 1991 1992 +1
==========================================
+ Hits 13380 13407 +27
- Misses 2261 2262 +1
Partials 33 33
Continue to review full report at Codecov.
|
| @@ -0,0 +1,23 @@ | |||
| :host { | |||
There was a problem hiding this comment.
Is this doable without :host? we use it in some places, but it's a bad practice and we should try to get rid of it where possible (since the parent is generally responsible for styling the host, and will have greater precedence here)
There was a problem hiding this comment.
I'm try to do it without :host but for some reason style min-width: 0 for .description not affected parent component. Originally this min-width: 0 added for fix such case as on screenshots.
Without min-width: 0 for host
Add min-width: 0
anyway, I still work on this component and will try to figure way to fix this without using :host.
There was a problem hiding this comment.
There's usually alternatives, but always the easiest to find. Don't spend too much time on it - if we need to use :host so be it.
| </div> | ||
| <div | ||
| class="description-button" | ||
| *ngIf="eventDescription.offsetWidth < eventDescription.scrollWidth && !this.isDescriptionTextToggled" |
There was a problem hiding this comment.
Also I found this ngIf not recalculate on window resize, I tried to use @ViewChild decorator and several other ways but ref always return undefined, so it's another one issue I working on
There was a problem hiding this comment.
One option here is to use out layout change directive, which should give you a callback when the layout changes - either from a window resize, or a shift in other components. An example of this can be seen in LabelComponent
| } | ||
|
|
||
| ngAfterViewInit() { | ||
| this.fullDescriptionTextWidth = this.eventDescriptionText.nativeElement.scrollWidth; |
There was a problem hiding this comment.
This should be reassigned in ngOnChanges (or easier, just move it into remeasure and don't persist it) as the description text is based on an input and can change.
There was a problem hiding this comment.
Got it, so now method look this:
// Called in ngOnChanges
public remeasure(): void {
// This will add class for truncate text for get full width of text (without text wrapping)
this.isDescriptionTextToggled = false;
this.cdRef.detectChanges();
// Get width of parent element and text full width without text wrapping, compare it
this.isDescriptionTruncated =
this.eventDescriptionContainer.nativeElement.offsetWidth < this.eventDescriptionText.nativeElement.scrollWidth;
this.cdRef.detectChanges();
}
| } | ||
| } | ||
|
|
||
| ngAfterViewInit() { |
There was a problem hiding this comment.
Missing visibility/type declarations
| export class DescriptionComponent implements OnChanges, AfterViewInit { | ||
| public isInitialized: boolean = false; | ||
| public isDescriptionTextToggled: boolean = false; | ||
| public readonly isDescriptionTruncated$: Subject<boolean> = new Subject(); |
There was a problem hiding this comment.
Is there an advantage of using an async boolean here (vs a simple boolean that is reassigned)?
There was a problem hiding this comment.
The original problem was *ngIf don't re-rendered after the change condition, so I decide to use this method. But now I'm use ChangeDetectorRef for this purpose
|
|
||
| this.isDescriptionTruncated = | ||
| this.eventDescriptionContainer.nativeElement.offsetWidth < this.eventDescriptionText.nativeElement.scrollWidth; | ||
| this.cdRef.detectChanges(); |
There was a problem hiding this comment.
Only run once, at the end of the remeasure method.
There was a problem hiding this comment.
It will cause bug, value of isDescriptionTruncated will be wrong as we try to get this.eventDescriptionText.nativeElement.scrollWidth before it truncated and DOM changed.
There was a problem hiding this comment.
Hmm - that's one of the weird parts about using detectChanges explicitly instead of markForCheck (which just invalidates this node in the change detection tree to be picked up by angular's general change detection, which should ensure that the changes settle. Changing that here may be better - could you try that out, and if it's not working we'll go with this?
There was a problem hiding this comment.
I try to use markForCheck method but unfortunately it not work correctly
This comment has been minimized.
This comment has been minimized.
|
@DmitiryPotychkin we should now use this component at the two locations mentioned in the other PR. |
Add description component, which truncate long text and display buttons "show more" or "show less"