fix: adding layout trigger to titled container (#352)

* fix: adding layout trigger to titled container

- Triggering layout initialization at the time of creation rather
 than after view init.

* refactor: remove delay from set Timeout

* refactor: initializing on init

* refactor: fixing lint and test

- moving dashboard provides to an exported variable
- This would make it easier to provide essential services without
  being too verbose in every test
- Needed to provide LayoutChangeService with me current change as
  we added it in the titled content component
- Also, this fixes the navigation$ pipe console warning which usually comes
  during test runs

Author: anandtiwary

projects/common/src/layout/layout-change.service.ts

@@ -20,13 +20,15 @@ export class LayoutChangeService implements OnDestroy {
     @SkipSelf() @Optional() private readonly parentLayoutChange?: LayoutChangeService
   ) {
     this.boundingElement = hostElementRef.nativeElement;
-    // Get layout changes from our parent LayoutChangeService, or if it's root, collect from window
-    const parentChange$ = this.parentLayoutChange
-      ? this.parentLayoutChange.layoutChangeSubject
-      : this.getWindowResizeEvents();
 
     // Filter out layout changes from our parent if we didn't change size
-    parentChange$.pipe(filter(event => this.hasLayoutChanged(event))).subscribe(this.layoutChangeSubject);
+    (this.parentLayoutChange?.getLayoutChangeEventObservable() ?? this.getWindowResizeEvents())
+      .pipe(filter(event => this.hasLayoutChanged(event)))
+      .subscribe(this.layoutChangeSubject);
+  }
+
+  public getLayoutChangeEventObservable(): Observable<LayoutChangeEvent> {
+    return this.layoutChangeSubject;
   }
 
   public publishLayoutChange(): void {

projects/components/src/layout/layout-change.directive.ts

@@ -1,19 +1,16 @@
-import { AfterViewInit, Directive, EventEmitter, Output } from '@angular/core';
+import { Directive, EventEmitter, Output } from '@angular/core';
 import { LayoutChangeService, SubscriptionLifecycle } from '@hypertrace/common';
 
 @Directive({
   selector: '[htLayoutChange]',
   providers: [SubscriptionLifecycle, LayoutChangeService]
 })
-export class LayoutChangeDirective implements AfterViewInit {
+export class LayoutChangeDirective {
   @Output('htLayoutChange')
   public readonly changeEmitter: EventEmitter<void> = new EventEmitter();
 
   public constructor(private readonly layoutChange: LayoutChangeService, subscriptionLifecycle: SubscriptionLifecycle) {
     subscriptionLifecycle.add(layoutChange.layout$.subscribe(this.changeEmitter));
-  }
-
-  public ngAfterViewInit(): void {
     this.layoutChange.initialize();
   }
 }

projects/components/src/titled-content/titled-content.component.scss

@@ -7,6 +7,7 @@
   width: 100%;
 
   .header {
+    max-height: 28px;
     display: flex;
     flex-direction: row;
     align-items: center;
@@ -31,7 +32,7 @@
   }
 
   .content {
-    flex: 1 1;
+    flex: 1 1 auto;
     overflow: hidden;
   }
 

projects/components/src/titled-content/titled-content.component.test.ts

@@ -1,6 +1,8 @@
 import { fakeAsync } from '@angular/core/testing';
-import { NavigationService } from '@hypertrace/common';
+import { LayoutChangeService, NavigationService } from '@hypertrace/common';
 import { createHostFactory, mockProvider, Spectator } from '@ngneat/spectator/jest';
+import { MockDirective } from 'ng-mocks';
+import { LayoutChangeTriggerDirective } from '../layout/layout-change-trigger.directive';
 import { TitledContentComponent } from './titled-content.component';
 import { TitledContentModule } from './titled-content.module';
 
@@ -11,6 +13,8 @@ describe('Titled content component', () => {
     declareComponent: false,
     component: TitledContentComponent,
     imports: [TitledContentModule],
+    declarations: [MockDirective(LayoutChangeTriggerDirective)],
+    componentProviders: [LayoutChangeService],
     providers: [mockProvider(NavigationService)]
   });
 

projects/components/src/titled-content/titled-content.component.ts

@@ -10,7 +10,7 @@ import { TitledHeaderControlDirective } from './header-controls/titled-header-co
   changeDetection: ChangeDetectionStrategy.OnPush,
   template: `
     <div class="titled-content-container">
-      <div class="header">
+      <div class="header" [htLayoutChangeTrigger]="this.shouldShowHeader">
         <ht-label *ngIf="this.shouldShowTitleInHeader" [label]="this.title" class="title"></ht-label>
         <ht-link [paramsOrUrl]="this.link" class="link" *ngIf="this.link">
           <ht-button
@@ -51,6 +51,10 @@ export class TitledContentComponent {
   @Input()
   public linkLabel?: string;
 
+  public get shouldShowHeader(): boolean {
+    return this.shouldShowTitleInHeader || this.headerControl !== undefined;
+  }
+
   private get shouldShowTitle(): boolean {
     return !isEmpty(this.title) && !this.hideTitle;
   }

projects/components/src/titled-content/titled-content.module.ts

@@ -2,13 +2,14 @@ import { CommonModule } from '@angular/common';
 import { NgModule } from '@angular/core';
 import { ButtonModule } from '../button/button.module';
 import { LabelModule } from '../label/label.module';
+import { LayoutChangeModule } from '../layout/layout-change.module';
 import { LinkModule } from '../link/link.module';
 import { TitledHeaderControlDirective } from './header-controls/titled-header-control.directive';
 import { TitledContentComponent } from './titled-content.component';
 
 @NgModule({
   declarations: [TitledContentComponent, TitledHeaderControlDirective],
   exports: [TitledContentComponent, TitledHeaderControlDirective],
-  imports: [LabelModule, CommonModule, LinkModule, ButtonModule]
+  imports: [LabelModule, CommonModule, LinkModule, ButtonModule, LayoutChangeModule]
 })
 export class TitledContentModule {}

projects/dashboards/src/test/dashboard-verification.ts

@@ -1,6 +1,8 @@
+import { StaticProvider } from '@angular/core';
 import { ActivatedRoute } from '@angular/router';
 import {
   ColorService,
+  LayoutChangeService,
   NavigationService,
   RelativeTimeRange,
   TimeDuration,
@@ -10,7 +12,7 @@ import {
 import { MetadataService } from '@hypertrace/distributed-tracing';
 import { GraphQlRequestService } from '@hypertrace/graphql-client';
 import { ModelJson } from '@hypertrace/hyperdash';
-import { DashboardManagerService, LoggerService } from '@hypertrace/hyperdash-angular';
+import { DashboardManagerService, LoggerService, RENDERER_API } from '@hypertrace/hyperdash-angular';
 import { getMockFlexLayoutProviders } from '@hypertrace/test-utils';
 import { mockProvider, Spectator } from '@ngneat/spectator/jest';
 import { EMPTY, of } from 'rxjs';
@@ -32,6 +34,9 @@ export const isValidModelJson = (
 export const mockDashboardProviders = [
   mockProvider(GraphQlRequestService),
   mockProvider(ColorService),
+  mockProvider(LayoutChangeService, {
+    layout$: of()
+  }),
   mockProvider(TimeRangeService, {
     getTimeRangeAndChanges: () => EMPTY,
     getCurrentTimeRange: jest.fn().mockReturnValue(new RelativeTimeRange(new TimeDuration(15, TimeUnit.Minute)))
@@ -53,3 +58,31 @@ export const mockDashboardProviders = [
   }),
   ...getMockFlexLayoutProviders()
 ];
+
+export const rendererApiFactoryBuilder = <TModel extends object>(model: TModel) => () => ({
+  getTimeRange: jest.fn(),
+  model: model,
+  change$: EMPTY,
+  dataRefresh$: EMPTY,
+  timeRangeChanged$: EMPTY
+});
+
+export const mockDashboardWidgetProviders: <T extends object>(model: T) => StaticProvider[] = model => [
+  {
+    provide: RENDERER_API,
+    useFactory: rendererApiFactoryBuilder(model)
+  },
+  mockProvider(GraphQlRequestService, {
+    query: jest.fn(() => EMPTY)
+  }),
+  mockProvider(ColorService),
+  mockProvider(LayoutChangeService, {
+    getLayoutChangeEventObservable: jest.fn().mockReturnValue(of({})),
+    layout$: of()
+  }),
+  mockProvider(NavigationService, {
+    navigation$: EMPTY,
+    getAllValuesForQueryParameter: () => []
+  }),
+  ...getMockFlexLayoutProviders()
+];

projects/dashboards/src/widgets/divider/divider-widget-renderer.component.test.ts

@@ -1,25 +1,13 @@
-import { RENDERER_API } from '@hypertrace/hyperdash-angular';
 import { createComponentFactory, Spectator } from '@ngneat/spectator/jest';
-import { EMPTY } from 'rxjs';
+import { mockDashboardWidgetProviders } from '../../test/dashboard-verification';
 import { DividerWidgetRendererComponent } from './divider-widget-renderer.component';
 
 describe('Divider Widget Renderer Component', () => {
   let spectator: Spectator<DividerWidgetRendererComponent>;
 
   const createComponent = createComponentFactory<DividerWidgetRendererComponent>({
     component: DividerWidgetRendererComponent,
-    providers: [
-      {
-        provide: RENDERER_API,
-        useFactory: () => ({
-          model: {},
-          getTimeRange: jest.fn(),
-          change$: EMPTY,
-          dataRefresh$: EMPTY,
-          timeRangeChanged$: EMPTY
-        })
-      }
-    ],
+    providers: [...mockDashboardWidgetProviders({})],
     shallow: true
   });
 

projects/dashboards/src/widgets/greeting-label/greeting-label-widget-renderer.component.test.ts

@@ -1,7 +1,6 @@
 import { GreetingLabelComponent } from '@hypertrace/components';
-import { RENDERER_API } from '@hypertrace/hyperdash-angular';
 import { createComponentFactory, Spectator } from '@ngneat/spectator/jest';
-import { EMPTY } from 'rxjs';
+import { mockDashboardWidgetProviders } from '../../test/dashboard-verification';
 import { GreetingLabelWidgetRendererComponent } from './greeting-label-widget-renderer.component';
 import { GreetingLabelWidgetModel } from './greeting-label-widget.model';
 
@@ -11,19 +10,7 @@ describe('Greeting label widget renderer component', () => {
   const createComponent = createComponentFactory<GreetingLabelWidgetRendererComponent>({
     component: GreetingLabelWidgetRendererComponent,
     entryComponents: [GreetingLabelComponent],
-    providers: [
-      {
-        provide: RENDERER_API,
-        useFactory: () => ({
-          model: mockModel,
-          getDataFromModelDataSource: EMPTY,
-          getTimeRange: jest.fn(),
-          change$: EMPTY,
-          dataRefresh$: EMPTY,
-          timeRangeChanged$: EMPTY
-        })
-      }
-    ]
+    providers: [...mockDashboardWidgetProviders(mockModel)]
   });
 
   beforeEach(() => {

projects/dashboards/src/widgets/link/link-widget-renderer.component.test.ts

@@ -1,6 +1,5 @@
-import { RENDERER_API } from '@hypertrace/hyperdash-angular';
 import { createComponentFactory, Spectator } from '@ngneat/spectator/jest';
-import { EMPTY } from 'rxjs';
+import { mockDashboardWidgetProviders } from '../../test/dashboard-verification';
 import { LinkWidgetRendererComponent } from './link-widget-renderer.component';
 import { LinkWidgetModel } from './link-widget.model';
 
@@ -9,18 +8,6 @@ describe('Link widget renderer component', () => {
   let mockModel: Partial<LinkWidgetModel> = {};
   const createComponent = createComponentFactory<LinkWidgetRendererComponent>({
     component: LinkWidgetRendererComponent,
-    providers: [
-      {
-        provide: RENDERER_API,
-        useFactory: () => ({
-          model: mockModel,
-          getTimeRange: jest.fn(),
-          change$: EMPTY,
-          dataRefresh$: EMPTY,
-          timeRangeChanged$: EMPTY
-        })
-      }
-    ],
     shallow: true
   });
 
@@ -31,15 +18,19 @@ describe('Link widget renderer component', () => {
   test('Link should be displayed as expected if url and displayText are defined', () => {
     mockModel.url = '#';
     mockModel.displayText = 'Test';
-    spectator = createComponent();
+    spectator = createComponent({
+      providers: [...mockDashboardWidgetProviders(mockModel)]
+    });
 
     expect(spectator.query('ht-link')).toExist();
     expect(spectator.component.getDisplayText()).toEqual('Test');
   });
 
   test('Link should use url as displayText if displayText is undefined', () => {
     mockModel.url = '#';
-    spectator = createComponent();
+    spectator = createComponent({
+      providers: [...mockDashboardWidgetProviders(mockModel)]
+    });
 
     expect(spectator.query('ht-link')).toExist();
     expect(spectator.component.getDisplayText()).toEqual(mockModel.url);