Skip to content

Commit

Permalink
fix: Remove tooltip from dialog header to fix circular dependency iss…
Browse files Browse the repository at this point in the history
…ue; feat: Create an lcc-icon-button class to wrap icons in a button tag with no default styling added; feat: Remove need for full-screen overlay for Photo Viewer buttons, allowing click events to be handled as expected now
  • Loading branch information
mwiraszka committed Dec 27, 2024
1 parent 954a1da commit 4902994
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 103 deletions.
6 changes: 1 addition & 5 deletions src/app/components/dialog/dialog.component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,11 @@ $header-border-bottom-thickness: 2px;
border-bottom: $header-border-bottom-thickness solid
var(--lcc-color--dialog-headerBorderBottom);

.close-icon {
.close-button {
margin: 0 4px 0 auto;
background-color: unset;
height: calc($header-height - 4px);
width: calc($header-height - 4px);
color: var(--lcc-color--dialog-closeIcon);

&:hover {
cursor: pointer;
color: var(--lcc-color--dialog-closeIconOnHover);
}
}
Expand Down
16 changes: 8 additions & 8 deletions src/app/components/dialog/dialog.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
ViewContainerRef,
} from '@angular/core';

import { TooltipDirective } from '@app/components/tooltip/tooltip.directive';
import IconsModule from '@app/icons';
import { DIALOG_CONFIG_TOKEN } from '@app/services';
import type { DialogConfig, DialogOutput } from '@app/types';
Expand All @@ -18,18 +17,19 @@ import type { DialogConfig, DialogOutput } from '@app/types';
selector: 'lcc-dialog',
template: `
<header>
<i-feather
name="x"
class="close-icon"
[tooltip]="'Close'"
(click)="result.emit('close')">
</i-feather>
<button class="close-button lcc-icon-button">
<i-feather
name="x"
class="close-icon"
(click)="result.emit('close')">
</i-feather>
</button>
</header>
<ng-template #contentContainer></ng-template>
`,
styleUrl: './dialog.component.scss',
imports: [IconsModule, TooltipDirective],
imports: [IconsModule],
})
export class DialogComponent<TComponent extends DialogOutput<TResult>, TResult>
implements AfterViewInit
Expand Down
36 changes: 15 additions & 21 deletions src/app/components/photo-viewer/photo-viewer.component.html
Original file line number Diff line number Diff line change
@@ -1,25 +1,19 @@
<div
#navigationButtonsContainer
class="navigation-buttons-container">
<div class="button-wrapper">
<button
#previousPhotoButton
class="previous-photo-button lcc-secondary-button lcc-light-button"
[tooltip]="'Previous photo'"
(click)="onPreviousPhoto()">
<i-feather name="chevron-left"></i-feather>
</button>
</div>
<div class="previous-photo-button-wrapper">
<button
class="lcc-secondary-button lcc-light-button"
tooltip="Previous photo"
(click)="onPreviousPhoto()">
<i-feather name="chevron-left"></i-feather>
</button>
</div>

<div class="button-wrapper">
<button
#nextPhotoButton
class="next-photo-button lcc-secondary-button lcc-light-button"
[tooltip]="'Next photo'"
(click)="onNextPhoto()">
<i-feather name="chevron-right"></i-feather>
</button>
</div>
<div class="next-photo-button-wrapper">
<button
class="lcc-secondary-button lcc-light-button"
tooltip="Next photo"
(click)="onNextPhoto()">
<i-feather name="chevron-right"></i-feather>
</button>
</div>

<figure>
Expand Down
27 changes: 12 additions & 15 deletions src/app/components/photo-viewer/photo-viewer.component.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,26 @@ figcaption {
border-radius: var(--lcc-borderRadius--small);
line-height: 20px;
text-align: center;

// TODO: No idea why this is needed
translate: 0 -2px;
}

.navigation-buttons-container {
justify-self: center;
width: calc(100% - 16px);
max-width: calc(var(--lcc-width--appContent) + 100px);
.previous-photo-button-wrapper,
.next-photo-button-wrapper {
position: absolute;
top: 0;
bottom: 0;
left: 0;
right: 0;
display: flex;
align-items: center;
justify-content: space-between;
margin: 8px;
top: 50%;

.previous-photo-button,
.next-photo-button {
button {
display: flex;
padding: 8px;
background-color: var(--lcc-color--photoViewer-buttonBackground) !important;
}
}

.previous-photo-button-wrapper {
left: 40px;
}

.next-photo-button-wrapper {
right: 40px;
}
31 changes: 2 additions & 29 deletions src/app/components/photo-viewer/photo-viewer.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ import { CommonModule } from '@angular/common';
import {
AfterViewInit,
Component,
ElementRef,
EventEmitter,
Input,
Output,
Renderer2,
ViewChild,
} from '@angular/core';

import { TooltipDirective } from '@app/components/tooltip/tooltip.directive';
Expand All @@ -23,12 +21,6 @@ import { ImagePreloadDirective } from '../image-preload/image-preload.directive'
imports: [CommonModule, IconsModule, ImagePreloadDirective, TooltipDirective],
})
export class PhotoViewerComponent implements AfterViewInit, DialogOutput<null> {
@ViewChild('navigationButtonsContainer')
navigationButtonsContainer?: ElementRef<HTMLDivElement>;
@ViewChild('previousPhotoButton') previousPhotoButton?: ElementRef<HTMLButtonElement>;
@ViewChild('nextPhotoButton') nextPhotoButton?: ElementRef<HTMLButtonElement>;

private clickListener?: () => void;
private keyListener?: () => void;

@Input() index = 0;
Expand All @@ -39,11 +31,10 @@ export class PhotoViewerComponent implements AfterViewInit, DialogOutput<null> {
constructor(private readonly renderer: Renderer2) {}

ngAfterViewInit(): void {
setTimeout(() => this.initEventListeners());
setTimeout(() => this.initKeyListener());
}

ngOnDestroy(): void {
this.clickListener?.();
this.keyListener?.();
}

Expand All @@ -55,25 +46,7 @@ export class PhotoViewerComponent implements AfterViewInit, DialogOutput<null> {
this.index = this.index < this.photos.length - 1 ? this.index + 1 : 0;
}

private initEventListeners(): void {
this.clickListener = this.renderer.listen(
this.navigationButtonsContainer?.nativeElement,
'click',
(event: MouseEvent) => {
if (
event.target instanceof Node &&
!this.previousPhotoButton?.nativeElement.contains(event.target) &&
!this.nextPhotoButton?.nativeElement.contains(event.target)
) {
// Due to absolute positioning on navigation buttons, default backdrop event listener in
// dialog service is overridden, and unable to easily gain access to DOM elements behind
// the buttons overlay to set up new click listeners. So instead, this is set up to close
// when anything but the previous and next buttons (and for reason the label) are clicked
this.dialogResult.emit('close');
}
},
);

private initKeyListener(): void {
this.keyListener = this.renderer.listen(
'document',
'keydown',
Expand Down
1 change: 1 addition & 0 deletions src/app/components/tooltip/tooltip.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export class TooltipDirective implements OnDestroy {
],
});

// TODO: Prevent unstyled tooltip text from displaying briefly on rapid mousemove events
const componentPortal = new ComponentPortal(
TooltipComponent,
this.viewContainerRef,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
<b>{{ nextEvent.eventDate | formatDate: 'short' }}</b>
</span>
</a>
<i-feather
name="x"
class="close-icon"
(click)="onClearBanner()">
</i-feather>

<button class="lcc-icon-button">
<i-feather
name="x"
class="close-icon"
(click)="onClearBanner()">
</i-feather>
</button>
</div>
</div>
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,9 @@
}

.close-icon {
height: 18px;
width: 18px;
color: var(--lcc-color--upcomingEventBanner-closeIcon);

&:hover {
cursor: pointer;
color: var(--lcc-color--upcomingEventBanner-closeIconOnHover);
}
}
4 changes: 2 additions & 2 deletions src/app/screens/schedule/schedule-screen.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ export class ScheduleScreenComponent implements OnInit {
);

constructor(
private readonly store: Store,
private readonly metaAndTitleService: MetaAndTitleService,
@Inject(DOCUMENT) private _document: Document,
private readonly metaAndTitleService: MetaAndTitleService,
private readonly store: Store,
) {}

ngOnInit(): void {
Expand Down
25 changes: 10 additions & 15 deletions src/app/services/dialog.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,21 +60,16 @@ export class DialogService<TComponent extends DialogOutput<TResult>, TResult> {
providers: [{ provide: DIALOG_CONFIG_TOKEN, useValue: dialogConfig }],
});

// const dialogComponentPortal = new ComponentPortal(
// DialogComponent<TComponent, TResult>,
// null,
// injector,
// );
// this.dialogComponentRef = this.overlayRef.attach(dialogComponentPortal);

// return firstValueFrom(
// this.dialogComponentRef.instance.result.pipe(tap(() => this.close())),
// );
return new Promise(resolve => {
setTimeout(() => {
resolve('close');
}, 300);
});
const dialogComponentPortal = new ComponentPortal(
DialogComponent<TComponent, TResult>,
null,
injector,
);
this.dialogComponentRef = this.overlayRef.attach(dialogComponentPortal);

return firstValueFrom(
this.dialogComponentRef.instance.result.pipe(tap(() => this.close())),
);
}

public close(): void {
Expand Down
17 changes: 17 additions & 0 deletions src/styles/components/_button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,20 @@
background-color: var(--lcc-color--button-warningButtonBackgroundOnHover);
}
}

.lcc-icon-button {
background-color: unset;
border: none;
display: flex;
justify-content: center;
align-items: center;

&:hover {
cursor: pointer;
}

i-feather {
height: 18px;
width: 18px;
}
}

0 comments on commit 4902994

Please sign in to comment.