Skip to content

feat: adding an attachedTrigger with sheet overlay #1382

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
anandtiwary merged 5 commits into from
Jan 19, 2022
Merged

feat: adding an attachedTrigger with sheet overlay #1382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
754aaaf
feat: adding an attachedTrigger with sheet overlay
anandtiwary Jan 18, 2022
5fc6508
refactor: addressing review comments
anandtiwary Jan 19, 2022
d60e6f7
refactor: setting default trigger collapsed view and updating width
anandtiwary Jan 19, 2022
a305028
Merge branch 'main' into sheet-overlay-anand
anandtiwary Jan 19, 2022
47691f0
Merge branch 'main' into sheet-overlay-anand
anandtiwary Jan 19, 2022
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
46 changes: 25 additions & 21 deletions projects/components/src/overlay/sheet/sheet-overlay.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
@import 'color-palette';
@import 'font';

$box-shadow: -3px 0px 24px rgba(0, 0, 0, 0.12), -1px 0px 8px rgba(0, 0, 0, 0.08);

.sheet-overlay {
box-shadow: -3px 1px 24px rgba(0, 0, 0, 0.12), -1px -1px 8px rgba(0, 0, 0, 0.08);
box-shadow: $box-shadow;
border-radius: 16px 0 0 16px;
height: 100%;
width: 100%;
overflow: hidden;

padding: 24px;
Expand All @@ -13,26 +16,6 @@
flex-direction: column;
z-index: 1;

&.sheet-size-responsive-extra-large {
Copy link
Copy Markdown
Contributor

@palbizu palbizu Jan 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Why this size is not necessary anymore?

Edit: Moved to the component

Copy link
Copy Markdown
Contributor Author

@anandtiwary anandtiwary Jan 19, 2022

Choose a reason for hiding this comment

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

Adding it programmatically in TS

width: 100%;
}

&.sheet-size-small {
width: 320px;
}

&.sheet-size-medium {
width: 600px;
}

&.sheet-size-large {
width: 840px;
}

&.sheet-size-extra-large {
width: 1280px;
}

.header {
display: flex;
justify-content: space-between;
Expand Down Expand Up @@ -62,3 +45,24 @@
}
}
}

.attached-trigger {
cursor: pointer;
box-shadow: $box-shadow;
clip-path: inset(-20px -20px 0px -20px);
background: white;
width: 200px;
Copy link
Copy Markdown
Contributor

@itssharmasandeep itssharmasandeep Jan 19, 2022

Choose a reason for hiding this comment

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

Should we avoid this? I know it works for our current use case.

Copy link
Copy Markdown
Contributor Author

@anandtiwary anandtiwary Jan 19, 2022

Choose a reason for hiding this comment

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

Let's keep this for now. I want to put some restrictions on the content.

height: 40px;
transform: translate(calc(-50% - 20px), 0px) rotate(270deg);
position: relative;
top: -50%;
border-radius: 6px;

display: flex;
Copy link
Copy Markdown
Contributor

@itssharmasandeep itssharmasandeep Jan 19, 2022

Choose a reason for hiding this comment

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

nit: @include center-contents

Copy link
Copy Markdown
Contributor Author

@anandtiwary anandtiwary Jan 19, 2022

Choose a reason for hiding this comment

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

I prefer putting flex properties directly :)

align-items: center;
padding: 0px 12px;

.trigger-icon {
margin-right: 14px;
}
}
33 changes: 31 additions & 2 deletions projects/components/src/overlay/sheet/sheet-overlay.component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ describe('Sheet Overlay component', () => {
provide: PopoverRef,
useValue: {
height: jest.fn(),
width: jest.fn(),
close: jest.fn()
}
},
Expand Down Expand Up @@ -86,11 +87,39 @@ describe('Sheet Overlay component', () => {
expect(spectator.query('.header')).toHaveText('expected title');
});

test('uses the requested size', () => {
test('uses the requested size for small', () => {
spectator = createConfiguredHost({
size: SheetSize.Small
});
expect(spectator.inject(PopoverRef)?.width).toHaveBeenCalledWith('320px');
});

test('uses the requested size for medium', () => {
spectator = createConfiguredHost({
size: SheetSize.Medium
});
expect(spectator.inject(PopoverRef)?.width).toHaveBeenCalledWith('600px');
});

test('uses the requested size for large', () => {
spectator = createConfiguredHost({
size: SheetSize.Large
});
expect(spectator.query('.sheet-overlay')).toHaveClass('sheet-size-large');
expect(spectator.inject(PopoverRef)?.width).toHaveBeenCalledWith('840px');
});

test('uses the requested size for extra large', () => {
spectator = createConfiguredHost({
size: SheetSize.ExtraLarge
});
expect(spectator.inject(PopoverRef)?.width).toHaveBeenCalledWith('1280px');
});

test('uses the requested size for responsive extra large', () => {
spectator = createConfiguredHost({
size: SheetSize.ResponsiveExtraLarge
});
expect(spectator.inject(PopoverRef)?.width).toHaveBeenCalledWith('60%');
});

test('closes on close button click', () => {
Expand Down
93 changes: 68 additions & 25 deletions projects/components/src/overlay/sheet/sheet-overlay.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ChangeDetectionStrategy, Component, HostListener, Inject, Injector, Tem
import { IconType } from '@hypertrace/assets-library';
import { GLOBAL_HEADER_HEIGHT, LayoutChangeService } from '@hypertrace/common';
import { ButtonStyle } from '../../button/button';
import { IconSize } from '../../icon/icon-size';
import { PopoverFixedPositionLocation, POPOVER_DATA } from '../../popover/popover';
import { PopoverRef } from '../../popover/popover-ref';
import { SheetConstructionData } from '../overlay.service';
Expand All @@ -12,29 +13,42 @@ import { SheetOverlayConfig, SheetSize } from './sheet';
styleUrls: ['./sheet-overlay.component.scss'],
changeDetection: ChangeDetectionStrategy.OnPush,
template: `
<div *ngIf="this.visible" class="sheet-overlay" [ngClass]="'sheet-size-' + this.size">
<div *ngIf="this.showHeader" class="header">
<h3 class="header-title">{{ sheetTitle }}</h3>
<ht-button
class="close-button"
icon="${IconType.CloseCircle}"
display="${ButtonStyle.Outlined}"
htTooltip="Close Sheet"
(click)="this.close()"
>
</ht-button>
<ng-container *ngIf="this.visible">
<div class="sheet-overlay">
<ng-container *ngIf="!this.isViewCollapsed">
<div *ngIf="this.showHeader" class="header">
<h3 class="header-title">{{ sheetTitle }}</h3>
<ht-button
class="close-button"
icon="${IconType.CloseCircle}"
display="${ButtonStyle.Outlined}"
htTooltip="Close Sheet"
(click)="this.close()"
>
</ht-button>
</div>
<div class="content-wrapper">
<div class="content">
<ng-container *ngIf="this.isComponentSheet; else templateRenderer">
<ng-container *ngComponentOutlet="this.renderer; injector: this.rendererInjector"></ng-container>
</ng-container>
<ng-template #templateRenderer>
<ng-container *ngTemplateOutlet="this.renderer"></ng-container>
</ng-template>
</div>
</div>
</ng-container>
</div>
<div class="content-wrapper">
<div class="content">
<ng-container *ngIf="this.isComponentSheet; else templateRenderer">
<ng-container *ngComponentOutlet="this.renderer; injector: this.rendererInjector"></ng-container>
</ng-container>
<ng-template #templateRenderer>
<ng-container *ngTemplateOutlet="this.renderer"></ng-container>
</ng-template>
</div>
<div class="attached-trigger" *ngIf="!!this.attachedTriggerTemplate" (click)="this.toggleCollapseExpand()">
Copy link
Copy Markdown
Contributor

@itssharmasandeep itssharmasandeep Jan 19, 2022

Choose a reason for hiding this comment

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

should we also include chevron-up and chevron-down icon included in this? To give prover collapse and expand trigger behaviour.

Copy link
Copy Markdown
Contributor Author

@anandtiwary anandtiwary Jan 19, 2022

Choose a reason for hiding this comment

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

added

<ht-icon
class="trigger-icon"
icon="{{ this.isViewCollapsed ? '${IconType.ChevronUp}' : '${IconType.ChevronDown}' }}"
size="${IconSize.Small}"
htTooltip="{{ this.isViewCollapsed ? 'Expand Sheet' : 'Collapse Sheet' }}"
></ht-icon>
<ng-container *ngTemplateOutlet="this.attachedTriggerTemplate"></ng-container>
</div>
</div>
</ng-container>
`
})
export class SheetOverlayComponent {
Expand All @@ -46,6 +60,8 @@ export class SheetOverlayComponent {
public readonly rendererInjector: Injector;
public visible: boolean = true;
public readonly closeOnEscape: boolean;
public readonly attachedTriggerTemplate?: TemplateRef<unknown>;
public isViewCollapsed: boolean;
Copy link
Copy Markdown
Contributor

@arjunlalb arjunlalb Jan 19, 2022

Choose a reason for hiding this comment

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

can we repurpose the visible property for this? both seems to be doing similar things.

Copy link
Copy Markdown
Contributor Author

@anandtiwary anandtiwary Jan 19, 2022

Choose a reason for hiding this comment

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

Visible is actually hiding the entire content (including the trigger) before the popover is closed. So, it is different than isViewCollapsed and I can't combine them. Also, keeping them separate keeps the blast radius small


public constructor(
private readonly popoverRef: PopoverRef,
Expand All @@ -58,13 +74,13 @@ export class SheetOverlayComponent {
this.sheetTitle = sheetConfig.title === undefined ? '' : sheetConfig.title;
this.size = sheetConfig.size;
this.closeOnEscape = sheetConfig.closeOnEscapeKey ?? true;
this.attachedTriggerTemplate = sheetConfig.attachedTriggerTemplate;
this.isViewCollapsed = !!this.attachedTriggerTemplate;

this.isComponentSheet = !(sheetConfig.content instanceof TemplateRef);
this.renderer = sheetConfig.content;
this.popoverRef.height(this.getHeightForPopover(globalHeaderHeight, sheetConfig.position));

if (this.size === SheetSize.ResponsiveExtraLarge) {
this.popoverRef.width('60%');
}
this.setWidth();

this.rendererInjector = Injector.create({
providers: [
Expand All @@ -89,6 +105,33 @@ export class SheetOverlayComponent {
this.popoverRef.close();
}

public toggleCollapseExpand(): void {
this.isViewCollapsed = !this.isViewCollapsed;

this.setWidth();
}

private setWidth(): void {
this.popoverRef.width(this.isViewCollapsed ? '0px' : this.getWidthForPopover());
}

private getWidthForPopover(): string {
switch (this.size) {
case SheetSize.Small:
return '320px';
case SheetSize.Medium:
return '600px';
case SheetSize.Large:
return '840px';
case SheetSize.ExtraLarge:
return '1280px';
case SheetSize.ResponsiveExtraLarge:
return '60%';
default:
return '100%';
}
}

private getHeightForPopover(globalHeaderHeight: string, position?: PopoverFixedPositionLocation): string {
return position === PopoverFixedPositionLocation.Right ? '100vh' : `calc(100vh - ${globalHeaderHeight})`;
}
Expand Down
3 changes: 2 additions & 1 deletion projects/components/src/overlay/sheet/sheet-overlay.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';
import { ButtonModule } from '../../button/button.module';
import { IconModule } from '../../icon/icon.module';
import { TooltipModule } from '../../tooltip/tooltip.module';
import { SheetOverlayComponent } from './sheet-overlay.component';

@NgModule({
imports: [CommonModule, ButtonModule, TooltipModule],
imports: [CommonModule, ButtonModule, TooltipModule, IconModule],
declarations: [SheetOverlayComponent],
exports: [SheetOverlayComponent]
})
Expand Down
3 changes: 2 additions & 1 deletion projects/components/src/overlay/sheet/sheet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { InjectionToken } from '@angular/core';
import { InjectionToken, TemplateRef } from '@angular/core';
import { Observable } from 'rxjs';
import { PopoverFixedPositionLocation } from '../../popover/popover';
import { OverlayConfig } from './../overlay';
Expand All @@ -8,6 +8,7 @@ export interface SheetOverlayConfig<TData = unknown> extends OverlayConfig {
data?: TData;
position?: PopoverFixedPositionLocation.Right | PopoverFixedPositionLocation.RightUnderHeader;
closeOnEscapeKey?: boolean;
attachedTriggerTemplate?: TemplateRef<unknown>;
}

export const enum SheetSize {
Expand Down