Skip to content

Commit

Permalink
fix(popover): remove unnecesary effect-refs asignments + more (#187)
Browse files Browse the repository at this point in the history
  • Loading branch information
pawel-twardziak authored Dec 11, 2024
1 parent 4335989 commit f2c61f7
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 69 deletions.
51 changes: 29 additions & 22 deletions packages/primitives/popover/src/popover-arrow.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ export class RdxPopoverArrowDirective implements AfterViewInit {
/** @ignore */
private triggerRect: DOMRect;

constructor() {
this.onArrowSvgElementChangeEffect();
this.onContentPositionChangeEffect();
}

/** @ignore */
ngAfterViewInit() {
if (this.elementRef.nativeElement.parentElement) {
Expand Down Expand Up @@ -97,30 +102,32 @@ export class RdxPopoverArrowDirective implements AfterViewInit {
}

/** @ignore */
private readonly onArrowSvgElementChangeEffect = effect(() => {
const arrowElement = this.arrowSvgElement();

untracked(() => {
const currentArrowSvgElement = this.currentArrowSvgElement();
if (currentArrowSvgElement) {
this.renderer.removeChild(this.elementRef.nativeElement, currentArrowSvgElement);
}
this.currentArrowSvgElement.set(arrowElement);
this.renderer.setStyle(this.elementRef.nativeElement, 'width', `${this.width()}px`);
this.renderer.setStyle(this.elementRef.nativeElement, 'height', `${this.height()}px`);
this.renderer.appendChild(this.elementRef.nativeElement, this.currentArrowSvgElement());
private onArrowSvgElementChangeEffect() {
effect(() => {
const arrowElement = this.arrowSvgElement();
untracked(() => {
const currentArrowSvgElement = this.currentArrowSvgElement();
if (currentArrowSvgElement) {
this.renderer.removeChild(this.elementRef.nativeElement, currentArrowSvgElement);
}
this.currentArrowSvgElement.set(arrowElement);
this.renderer.setStyle(this.elementRef.nativeElement, 'width', `${this.width()}px`);
this.renderer.setStyle(this.elementRef.nativeElement, 'height', `${this.height()}px`);
this.renderer.appendChild(this.elementRef.nativeElement, this.currentArrowSvgElement());
});
});
});
}

/** @ignore */
private readonly OnContentPositionChange = effect(() => {
const position = this.position();

untracked(() => {
if (!position) {
return;
}
this.setPosition(position);
private onContentPositionChangeEffect() {
effect(() => {
const position = this.position();
untracked(() => {
if (!position) {
return;
}
this.setPosition(position);
});
});
});
}
}
8 changes: 0 additions & 8 deletions packages/primitives/popover/src/popover-arrow.inject.ts

This file was deleted.

20 changes: 15 additions & 5 deletions packages/primitives/popover/src/popover-close.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,22 @@ export class RdxPopoverCloseDirective {
/** @ignore */
private readonly renderer = inject(Renderer2);

constructor() {
this.onIsControlledExternallyEffect();
}

/** @ignore */
private readonly onIsControlledExternallyEffect = effect(() => {
const isControlledExternally = this.popoverRoot.controlledExternally()();
private onIsControlledExternallyEffect() {
effect(() => {
const isControlledExternally = this.popoverRoot.controlledExternally()();

untracked(() => {
this.renderer.setStyle(this.elementRef.nativeElement, 'display', isControlledExternally ? 'none' : null);
untracked(() => {
this.renderer.setStyle(
this.elementRef.nativeElement,
'display',
isControlledExternally ? 'none' : null
);
});
});
});
}
}
39 changes: 24 additions & 15 deletions packages/primitives/popover/src/popover-content.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ export class RdxPopoverContentDirective implements OnInit {
*/
readonly onHide = output<void>();

constructor() {
this.onPositionChangeEffect();
this.onControlledExternallyChangeEffect();
}

/** @ignore */
ngOnInit() {
this.setOrigin();
Expand Down Expand Up @@ -241,24 +246,28 @@ export class RdxPopoverContentDirective implements OnInit {
}

/** @ignore */
private OnPositionChange = effect(() => {
const positions = this.positions();
this.disableAlternatePositions();
untracked(() => {
const prevPositions = this.connectedOverlay.positions;
this.connectedOverlay.positions = positions;
this.connectedOverlay.ngOnChanges({
positions: new SimpleChange(prevPositions, this.connectedOverlay.positions, false)
private onPositionChangeEffect() {
effect(() => {
const positions = this.positions();
this.disableAlternatePositions();
untracked(() => {
const prevPositions = this.connectedOverlay.positions;
this.connectedOverlay.positions = positions;
this.connectedOverlay.ngOnChanges({
positions: new SimpleChange(prevPositions, this.connectedOverlay.positions, false)
});
this.connectedOverlay.overlayRef?.updatePosition();
});
this.connectedOverlay.overlayRef?.updatePosition();
});
});
}

/** @ignore */
private OnControlledExternallyChange = effect(() => {
this.popoverRoot.controlledExternally()();
untracked(() => {
this.setDisableClose();
private onControlledExternallyChangeEffect() {
effect(() => {
this.popoverRoot.controlledExternally()();
untracked(() => {
this.setDisableClose();
});
});
});
}
}
43 changes: 26 additions & 17 deletions packages/primitives/popover/src/popover-root.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ export class RdxPopoverRootDirective implements OnInit {
/** @ignore */
private isControlledExternally = computed(() => signal(this.open() !== void 0));

constructor() {
this.onOpenChangeEffect();
this.onIsOpenChangeEffect();
}

/** @ignore */
ngOnInit(): void {
if (this.defaultOpen()) {
Expand Down Expand Up @@ -128,27 +133,31 @@ export class RdxPopoverRootDirective implements OnInit {
}

/** @ignore */
private readonly onIsOpenChangeEffect = effect(() => {
const isOpen = this.isOpen();
private onIsOpenChangeEffect() {
effect(() => {
const isOpen = this.isOpen();

untracked(() => {
if (isOpen) {
this.show();
} else {
this.hide();
}
untracked(() => {
if (isOpen) {
this.show();
} else {
this.hide();
}
});
});
});
}

/** @ignore */
private readonly onOpenChangeEffect = effect(() => {
const currentOpen = this.open();
private onOpenChangeEffect() {
effect(() => {
const currentOpen = this.open();

untracked(() => {
this.isControlledExternally().set(currentOpen !== void 0);
if (this.isControlledExternally()()) {
this.setOpen(currentOpen);
}
untracked(() => {
this.isControlledExternally().set(currentOpen !== void 0);
if (this.isControlledExternally()()) {
this.setOpen(currentOpen);
}
});
});
});
}
}
4 changes: 2 additions & 2 deletions packages/primitives/popover/src/popover-root.inject.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { assertInInjectionContext, inject } from '@angular/core';
import { assertInInjectionContext, inject, isDevMode } from '@angular/core';
import { RdxPopoverRootDirective } from './popover-root.directive';
import { RdxPopoverRootToken } from './popover-root.token';

export function injectPopoverRoot(): RdxPopoverRootDirective {
assertInInjectionContext(injectPopoverRoot);
isDevMode() && assertInInjectionContext(injectPopoverRoot);
return inject(RdxPopoverRootToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ import { RdxPopoverAlign, RdxPopoverSide } from '../src/popover.types';
</select>
SideOffset:
<input class="no-reset" [ngModel]="sideOffset()" (ngModelChange)="sideOffset.set($event)" type="number" />
AlignOffset:
<input class="no-reset" [ngModel]="alignOffset()" (ngModelChange)="alignOffset.set($event)" type="number" />
Alternate positions:
<input
class="no-reset"
Expand All @@ -236,6 +238,7 @@ import { RdxPopoverAlign, RdxPopoverSide } from '../src/popover.types';
<ng-template
[sideOffset]="sideOffset()"
[alignOffset]="alignOffset()"
[side]="selectedSide()"
[align]="selectedAlign()"
[disableAlternatePositions]="disableAlternatePositions()"
Expand Down Expand Up @@ -278,6 +281,7 @@ export class RdxPopoverPositioningComponent {
selectedSide = signal(RdxPopoverSide.Top);
selectedAlign = signal(RdxPopoverAlign.Center);
sideOffset = signal(8);
alignOffset = signal(8);
disableAlternatePositions = signal(false);

readonly sides = RdxPopoverSide;
Expand Down

0 comments on commit f2c61f7

Please sign in to comment.