Skip to content

Commit

Permalink
Fix misaligned tooltip positions in modal dialogs
Browse files Browse the repository at this point in the history
This commit fixes a bug that causes tooltips to be slightly misaligned.

Tooltip positioning was incorrect during modal transitions due to their
initial movement, causing tooltips to align incorrectly at the start of
the animation rather than the end.

One way to solve this would be using `autoUpdate` from `floating-ui`
with `animationFrame: true`. However, this recalculates positions tens
of times per second, impacting performance. This is a monkey solution.

This commit adopts a more efficient approach by updating tooltip
positions only at the end of the transitions, which reduces calculations
and conserves resources.

Key changes:

- Addd transition end event listener for updating tooltip positions.
- Use throttling to eliminate excessive position recalculations.

Other supporting changes:

- Improve throttle function to support efficient recalculations of
  positions:
  - Add ability to optionally exclude the first execution (leading
    call).
  - Refactor to simplify it make it easier to follow and read.
  - Fix a bug where initial calls were incorrectly throttled if
    `dateNow()` returned `0`.
- Introduce and use a global hook for efficient DOM event management.
  This greatily introduce safety, reuse and testability of event
  listening.
  • Loading branch information
undergroundwires committed May 8, 2024
1 parent a334320 commit d7fae29
Show file tree
Hide file tree
Showing 26 changed files with 1,158 additions and 328 deletions.
144 changes: 128 additions & 16 deletions src/application/Common/Timing/Throttle.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,156 @@
/* eslint-disable max-classes-per-file */
import { PlatformTimer } from './PlatformTimer';
import type { Timer, TimeoutType } from './Timer';

export type CallbackType = (..._: readonly unknown[]) => void;

export interface ThrottleOptions {
/** Skip the immediate execution of the callback on the first invoke */
readonly excludeLeadingCall: boolean;
readonly timer: Timer;
}

const DefaultOptions: ThrottleOptions = {
excludeLeadingCall: false,
timer: PlatformTimer,
};

export function throttle(
callback: CallbackType,
waitInMs: number,
timer: Timer = PlatformTimer,
options: Partial<ThrottleOptions> = DefaultOptions,
): CallbackType {
const throttler = new Throttler(timer, waitInMs, callback);
const defaultedOptions: ThrottleOptions = {
...DefaultOptions,
...options,
};
const throttler = new Throttler(waitInMs, callback, defaultedOptions);
return (...args: unknown[]) => throttler.invoke(...args);
}

class Throttler {
private queuedExecutionId: TimeoutType | undefined;
private lastExecutionTime: number | null = null;

private previouslyRun: number;
private executionScheduler: DelayedCallbackScheduler;

constructor(
private readonly timer: Timer,
private readonly waitInMs: number,
private readonly callback: CallbackType,
private readonly options: ThrottleOptions,
) {
if (!waitInMs) { throw new Error('missing delay'); }
if (waitInMs < 0) { throw new Error('negative delay'); }
this.executionScheduler = new DelayedCallbackScheduler(options.timer);
}

public invoke(...args: unknown[]): void {
const now = this.timer.dateNow();
if (this.queuedExecutionId !== undefined) {
this.timer.clearTimeout(this.queuedExecutionId);
this.queuedExecutionId = undefined;
switch (true) {
case this.isLeadingCallWithinThrottlePeriod(): {
if (this.options.excludeLeadingCall) {
this.scheduleNext(args);
return;
}
this.executeNow(args);
return;
}
case this.isAlreadyScheduled(): {
this.updateNextScheduled(args);
return;
}
case !this.isThrottlePeriodPassed(): {
this.scheduleNext(args);
return;
}
default:
throw new Error('Throttle logical error: no conditions for execution or scheduling were met.');
}
}

private isLeadingCallWithinThrottlePeriod(): boolean {
return this.isThrottlePeriodPassed()
&& !this.isAlreadyScheduled();
}

private isThrottlePeriodPassed(): boolean {
if (this.lastExecutionTime === null) {
return true;
}
if (!this.previouslyRun || (now - this.previouslyRun >= this.waitInMs)) {
this.callback(...args);
this.previouslyRun = now;
} else {
const nextCall = () => this.invoke(...args);
const nextCallDelayInMs = this.waitInMs - (now - this.previouslyRun);
this.queuedExecutionId = this.timer.setTimeout(nextCall, nextCallDelayInMs);
const timeSinceLastExecution = this.options.timer.dateNow() - this.lastExecutionTime;
const isThrottleTimePassed = timeSinceLastExecution >= this.waitInMs;
return isThrottleTimePassed;
}

private isAlreadyScheduled(): boolean {
return this.executionScheduler.getNext() !== null;
}

private scheduleNext(args: unknown[]): void {
if (this.executionScheduler.getNext()) {
throw new Error('An execution is already scheduled.');
}
this.executionScheduler.resetNext(
() => this.executeNow(args),
this.waitInMs,
);
}

private updateNextScheduled(args: unknown[]): void {
const nextScheduled = this.executionScheduler.getNext();
if (!nextScheduled) {
throw new Error('A non-existent scheduled execution cannot be updated.');
}
const nextDelay = nextScheduled.scheduledTime - this.dateNow();
this.executionScheduler.resetNext(
() => this.executeNow(args),
nextDelay,
);
}

private executeNow(args: unknown[]): void {
this.callback(...args);
this.lastExecutionTime = this.dateNow();
}

private dateNow(): number {
return this.options.timer.dateNow();
}
}

interface ScheduledCallback {
readonly scheduleTimeoutId: TimeoutType;
readonly scheduledTime: number;
}

class DelayedCallbackScheduler {
private scheduledCallback: ScheduledCallback | null = null;

constructor(
private readonly timer: Timer,
) { }

public getNext(): ScheduledCallback | null {
return this.scheduledCallback;
}

public resetNext(
callback: () => void,
delayInMs: number,
) {
this.clear();
this.scheduledCallback = {
scheduledTime: this.timer.dateNow() + delayInMs,
scheduleTimeoutId: this.timer.setTimeout(() => {
this.clear();
callback();
}, delayInMs),
};
}

private clear() {
if (this.scheduledCallback === null) {
return;
}
this.timer.clearTimeout(this.scheduledCallback.scheduleTimeoutId);
this.scheduledCallback = null;
}
}
5 changes: 5 additions & 0 deletions src/presentation/bootstrapping/DependencyProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { useCodeRunner } from '@/presentation/components/Shared/Hooks/UseCodeRun
import { CurrentEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory';
import { useDialog } from '@/presentation/components/Shared/Hooks/Dialog/UseDialog';
import { useScriptDiagnosticsCollector } from '@/presentation/components/Shared/Hooks/UseScriptDiagnosticsCollector';
import { useAutoUnsubscribedEventListener } from '@/presentation/components/Shared/Hooks/UseAutoUnsubscribedEventListener';

export function provideDependencies(
context: IApplicationContext,
Expand Down Expand Up @@ -77,6 +78,10 @@ export function provideDependencies(
InjectionKeys.useScriptDiagnosticsCollector,
useScriptDiagnosticsCollector,
),
useAutoUnsubscribedEventListener: (di) => di.provide(
InjectionKeys.useAutoUnsubscribedEventListener,
useAutoUnsubscribedEventListener,
),
};
registerAll(Object.values(resolvers), api);
}
Expand Down
13 changes: 3 additions & 10 deletions src/presentation/components/Scripts/View/Cards/CardList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

<script lang="ts">
import {
defineComponent, ref, onMounted, onUnmounted, computed,
defineComponent, ref, computed,
} from 'vue';
import { injectKey } from '@/presentation/injectionSymbols';
import SizeObserver from '@/presentation/components/Shared/SizeObserver.vue';
Expand All @@ -54,6 +54,7 @@ export default defineComponent({
},
setup() {
const { currentState, onStateChange } = injectKey((keys) => keys.useCollectionState);
const { startListening } = injectKey((keys) => keys.useAutoUnsubscribedEventListener);
const width = ref<number | undefined>();
Expand All @@ -70,7 +71,7 @@ export default defineComponent({
collapseAllCards();
}, { immediate: true });
const outsideClickListener = (event: PointerEvent): void => {
startListening(document, 'click', (event) => {
if (areAllCardsCollapsed()) {
return;
}
Expand All @@ -79,14 +80,6 @@ export default defineComponent({
if (element && !element.contains(target)) {
onOutsideOfActiveCardClicked(target);
}
};
onMounted(() => {
document.addEventListener('click', outsideClickListener);
});
onUnmounted(() => {
document.removeEventListener('click', outsideClickListener);
});
function onOutsideOfActiveCardClicked(clickedElement: Element): void {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { ref, onMounted, onUnmounted } from 'vue';
import { ref } from 'vue';
import { useAutoUnsubscribedEventListener, type UseEventListener } from '@/presentation/components/Shared/Hooks/UseAutoUnsubscribedEventListener';

export function useKeyboardInteractionState(window: WindowWithEventListeners = globalThis.window) {
export function useKeyboardInteractionState(
eventTarget: EventTarget = DefaultEventSource,
useEventListener: UseEventListener = useAutoUnsubscribedEventListener,
) {
const { startListening } = useEventListener();
const isKeyboardBeingUsed = ref(false);

const enableKeyboardFocus = () => {
Expand All @@ -17,20 +22,10 @@ export function useKeyboardInteractionState(window: WindowWithEventListeners = g
isKeyboardBeingUsed.value = false;
};

onMounted(() => {
window.addEventListener('keydown', enableKeyboardFocus, true);
window.addEventListener('click', disableKeyboardFocus, true);
});

onUnmounted(() => {
window.removeEventListener('keydown', enableKeyboardFocus);
window.removeEventListener('click', disableKeyboardFocus);
});
startListening(eventTarget, 'keydown', enableKeyboardFocus);
startListening(eventTarget, 'click', disableKeyboardFocus);

return { isKeyboardBeingUsed };
}

export interface WindowWithEventListeners {
addEventListener: typeof global.window.addEventListener;
removeEventListener: typeof global.window.removeEventListener;
}
export const DefaultEventSource = document;
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { onMounted, onUnmounted, type Ref } from 'vue';
import { type Ref } from 'vue';
import { useAutoUnsubscribedEventListener, type UseEventListener } from '@/presentation/components/Shared/Hooks/UseAutoUnsubscribedEventListener';
import { TreeNodeCheckState } from './Node/State/CheckState';
import type { TreeNode } from './Node/TreeNode';
import type { TreeRoot } from './TreeRoot/TreeRoot';
Expand All @@ -10,8 +11,10 @@ type TreeNavigationKeyCodes = 'ArrowLeft' | 'ArrowUp' | 'ArrowRight' | 'ArrowDow
export function useTreeKeyboardNavigation(
treeRootRef: Readonly<Ref<TreeRoot>>,
treeElementRef: Readonly<Ref<HTMLElement | undefined>>,
useEventListener: UseEventListener = useAutoUnsubscribedEventListener,
) {
useKeyboardListener(treeElementRef, (event) => {
const { startListening } = useEventListener();
startListening(treeElementRef, 'keydown', (event) => {
if (!treeElementRef.value) {
return; // Not yet initialized?
}
Expand Down Expand Up @@ -40,19 +43,6 @@ export function useTreeKeyboardNavigation(
});
}

function useKeyboardListener(
elementRef: Readonly<Ref<HTMLElement | undefined>>,
handleKeyboardEvent: (event: KeyboardEvent) => void,
) {
onMounted(() => {
elementRef.value?.addEventListener('keydown', handleKeyboardEvent, true);
});

onUnmounted(() => {
elementRef.value?.removeEventListener('keydown', handleKeyboardEvent);
});
}

interface TreeNavigationContext {
readonly focus: SingleNodeFocusManager;
readonly nodes: QueryableNodes;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import {
onBeforeUnmount,
shallowRef,
watch,
type Ref,
} from 'vue';

export interface UseEventListener {
(): TargetEventListener;
}

export const useAutoUnsubscribedEventListener: UseEventListener = () => ({
startListening: (eventTargetSource, eventType, eventHandler) => {
const eventTargetRef = isEventTarget(eventTargetSource)
? shallowRef(eventTargetSource)
: eventTargetSource;
return startListeningRef(
eventTargetRef,
eventType,
eventHandler,
);
},
});

type EventTargetRef = Readonly<Ref<EventTarget | undefined>>;

type EventTargetOrRef = EventTargetRef | EventTarget;

function isEventTarget(obj: EventTargetOrRef): obj is EventTarget {
return obj instanceof EventTarget;
}

export interface TargetEventListener {
startListening<TEvent extends keyof HTMLElementEventMap>(
eventTargetSource: EventTargetOrRef,
eventType: TEvent,
eventHandler: (event: HTMLElementEventMap[TEvent]) => void,
): void;
}

function startListeningRef<TEvent extends keyof HTMLElementEventMap>(
eventTargetRef: Readonly<Ref<EventTarget | undefined>>,
eventType: TEvent,
eventHandler: (event: HTMLElementEventMap[TEvent]) => void,
): void {
const eventListenerManager = new EventListenerManager();
watch(() => eventTargetRef.value, (element) => {
eventListenerManager.removeListenerIfExists();
if (!element) {
return;
}
eventListenerManager.addListener(element, eventType, eventHandler);
}, { immediate: true });

onBeforeUnmount(() => {
eventListenerManager.removeListenerIfExists();
});
}

class EventListenerManager {
private removeListener: (() => void) | null = null;

public removeListenerIfExists() {
if (this.removeListener === null) {
return;
}
this.removeListener();
this.removeListener = null;
}

public addListener<TEvent extends keyof HTMLElementEventMap>(
eventTarget: EventTarget,
eventType: TEvent,
eventHandler: (event: HTMLElementEventMap[TEvent]) => void,
) {
eventTarget.addEventListener(eventType, eventHandler);
this.removeListener = () => eventTarget.removeEventListener(eventType, eventHandler);
}
}
Loading

0 comments on commit d7fae29

Please sign in to comment.