Merge pull request #19337 from guerler/consistent_popper

Refactor and add tests for Popovers

client/src/components/ActivityBar/ActivityItem.vue

@@ -65,7 +65,7 @@ const meta = computed(() => store.metaForId(props.id));
 </script>
 
 <template>
-    <Popper reference-is="span" popper-is="span" :placement="tooltipPlacement">
+    <Popper :placement="tooltipPlacement">
         <template v-slot:reference>
             <b-nav-item
                 :id="`activity-${id}`"

client/src/components/Common/ContextMenu.vue

@@ -36,10 +36,10 @@ watch(
         <Popper
             placement="right"
             class="context-menu"
-            :style="placeContextMenu"
-            :force-show="true"
+            mode="light"
+            trigger="manual"
             :arrow="false"
-            mode="light">
+            :style="placeContextMenu">
             <div class="context-menu-slot">
                 <slot />
             </div>

client/src/components/Popper/Popper.test.js

@@ -0,0 +1,123 @@
+import { createPopper } from "@popperjs/core";
+import { mount } from "@vue/test-utils";
+
+import PopperComponent from "./Popper.vue";
+
+jest.mock("@popperjs/core", () => ({
+    createPopper: jest.fn(() => ({
+        destroy: jest.fn(),
+        update: jest.fn(),
+    })),
+}));
+
+function mountTarget(trigger = "click") {
+    return mount(PopperComponent, {
+        propsData: {
+            title: "Test Title",
+            placement: "bottom",
+            trigger: trigger,
+        },
+        slots: {
+            reference: "<button>Reference</button>",
+            default: "<p>Popper Content</p>",
+        },
+    });
+}
+
+describe("PopperComponent.vue", () => {
+    afterEach(() => {
+        jest.clearAllMocks();
+    });
+
+    test("renders component with default props", async () => {
+        const wrapper = mountTarget();
+        expect(wrapper.find(".popper-element").exists()).toBe(true);
+        expect(wrapper.find(".popper-element").isVisible()).toBe(false);
+        const reference = wrapper.find("button");
+        await reference.trigger("click");
+        expect(wrapper.find(".popper-header").exists()).toBe(true);
+        expect(wrapper.find(".popper-header").text()).toContain("Test Title");
+    });
+
+    test("opens and closes popper on click trigger", async () => {
+        const wrapper = mountTarget();
+        const reference = wrapper.find("button");
+        expect(wrapper.find(".popper-element").isVisible()).toBe(false);
+        await reference.trigger("click");
+        expect(wrapper.find(".popper-element").isVisible()).toBe(true);
+        await wrapper.find(".popper-close").trigger("click");
+        expect(wrapper.find(".popper-element").isVisible()).toBe(false);
+    });
+
+    test("disables popper when `disabled` prop is true", async () => {
+        const wrapper = mountTarget();
+        await wrapper.setProps({ disabled: true });
+        const reference = wrapper.find("button");
+        await reference.trigger("click");
+        expect(wrapper.find(".popper-element").isVisible()).toBe(false);
+    });
+
+    test("renders the arrow when `arrow` prop is true", () => {
+        const wrapper = mountTarget();
+        expect(wrapper.find(".popper-arrow").exists()).toBe(true);
+    });
+
+    test("does not render the arrow when `arrow` prop is false", async () => {
+        const wrapper = mountTarget();
+        await wrapper.setProps({ arrow: false });
+        expect(wrapper.find(".popper-arrow").exists()).toBe(false);
+    });
+
+    test("applies correct mode class", async () => {
+        const wrapper = mountTarget();
+        await wrapper.setProps({ mode: "light" });
+        expect(wrapper.find(".popper-element").classes()).toContain("popper-element-light");
+        await wrapper.setProps({ mode: "dark" });
+        expect(wrapper.find(".popper-element").classes()).toContain("popper-element-dark");
+    });
+
+    test("uses correct placement prop", () => {
+        mountTarget();
+        expect(createPopper).toHaveBeenCalledWith(
+            expect.anything(),
+            expect.anything(),
+            expect.objectContaining({ placement: "bottom" })
+        );
+    });
+
+    test("updates visibility when props or watchers change", async () => {
+        const wrapper = mountTarget();
+        await wrapper.setProps({ disabled: false });
+        const reference = wrapper.find("button");
+        await reference.trigger("click");
+        expect(wrapper.find(".popper-element").isVisible()).toBe(true);
+        await wrapper.setProps({ disabled: true });
+        expect(wrapper.find(".popper-element").isVisible()).toBe(false);
+    });
+
+    test("shows and hides popper on hover trigger", async () => {
+        const wrapper = mountTarget("hover");
+        const reference = wrapper.find("button");
+        const popperElement = wrapper.find(".popper-element");
+        expect(popperElement.isVisible()).toBe(false);
+        await reference.trigger("mouseover");
+        expect(popperElement.isVisible()).toBe(true);
+        await reference.trigger("mouseout");
+        expect(popperElement.isVisible()).toBe(false);
+    });
+
+    test("popper remains visible when clicked inside of popper", async () => {
+        const wrapper = mountTarget("click");
+        const reference = wrapper.find("button");
+        const popperElement = wrapper.find(".popper-element");
+        expect(popperElement.isVisible()).toBe(false);
+        await reference.trigger("click");
+        expect(popperElement.isVisible()).toBe(true);
+        await popperElement.trigger("mouseover");
+        expect(popperElement.isVisible()).toBe(true);
+        await popperElement.trigger("mouseout");
+        expect(popperElement.isVisible()).toBe(true);
+        await popperElement.trigger("click");
+        expect(popperElement.isVisible()).toBe(true);
+    });
+});

client/src/components/Popper/Popper.vue

@@ -1,15 +1,9 @@
 <template>
     <span>
-        <component :is="referenceIs" v-bind="referenceProps" ref="reference">
+        <span ref="reference">
             <slot name="reference" />
-        </component>
-        <component
-            :is="popperIs"
-            v-show="visible"
-            v-bind="popperProps"
-            ref="popper"
-            class="popper-element mt-1"
-            :class="`popper-element-${mode}`">
+        </span>
+        <div v-show="visible" ref="popper" class="popper-element mt-1" :class="`popper-element-${mode}`">
             <div v-if="arrow" class="popper-arrow" data-popper-arrow />
             <div v-if="title" class="popper-header px-2 py-1 rounded-top d-flex justify-content-between">
                 <span class="px-1">{{ title }}</span>
@@ -18,120 +12,53 @@
                 </span>
             </div>
             <slot />
-        </component>
+        </div>
     </span>
 </template>
 
-<script lang="ts">
+<script setup lang="ts">
 import { library } from "@fortawesome/fontawesome-svg-core";
 import { faTimesCircle } from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/vue-fontawesome";
-import type { PropType, UnwrapRef } from "vue";
-import { defineComponent, ref, toRef, watch } from "vue";
+import { type Placement } from "@popperjs/core";
+import type { PropType } from "vue";
+import { ref, watch } from "vue";
 
-import { usePopperjs } from "./usePopper";
+import { type Trigger, usePopper } from "./usePopper";
 
 library.add(faTimesCircle);
 
-export default defineComponent({
-    components: { FontAwesomeIcon },
+const props = defineProps({
+    arrow: { type: Boolean, default: true },
+    disabled: { type: Boolean, default: false },
+    mode: { type: String, default: "dark" },
+    placement: String as PropType<Placement>,
+    title: String,
+    trigger: String as PropType<Trigger>,
+});
 
-    props: {
-        // hook options
-        delayOnMouseout: Number,
-        delayOnMouseover: Number,
-        trigger: String as PropType<
-            Exclude<UnwrapRef<Required<Parameters<typeof usePopperjs>>["2"]["trigger"]>, "manual">
-        >,
-        forceShow: Boolean,
-        modifiers: Array as PropType<Required<Parameters<typeof usePopperjs>>["2"]["modifiers"]>,
-        onFirstUpdate: Function as PropType<Required<Parameters<typeof usePopperjs>>["2"]["onFirstUpdate"]>,
-        placement: String as PropType<Required<Parameters<typeof usePopperjs>>["2"]["placement"]>,
-        strategy: String as PropType<Required<Parameters<typeof usePopperjs>>["2"]["strategy"]>,
+const reference = ref();
+const popper = ref();
 
-        // component props
-        popperIs: {
-            default: "div",
-            type: String,
-        },
-        popperProps: {
-            type: Object,
-        },
-        referenceIs: {
-            default: "span",
-            type: String,
-        },
-        referenceProps: {
-            type: Object,
-        },
-        arrow: {
-            type: Boolean,
-            default: true,
-        },
-        disabled: {
-            type: Boolean,
-            default: false,
-        },
-        mode: {
-            type: String,
-            default: "dark",
-        },
-        title: {
-            type: String,
-            default: null,
-        },
+const { visible } = usePopper(reference, popper, {
+    placement: props.placement,
+    trigger: props.trigger,
+});
+
+watch(
+    () => [visible.value, props.disabled],
+    () => {
+        if (props.disabled && visible.value) {
+            visible.value = false;
+        }
     },
+    { flush: "sync" }
+);
 
-    emits: [
-        "show",
-        "hide",
-        "before-enter",
-        "enter",
-        "after-enter",
-        "enter-cancelled",
-        "before-leave",
-        "leave",
-        "after-leave",
-        "leave-cancelled",
-    ],
-
-    setup(props, { emit }) {
-        const reference = ref();
-        const popper = ref();
-        const { visible } = usePopperjs(reference, popper, {
-            ...props,
-            trigger: toRef(props, "trigger"),
-            forceShow: toRef(props, "forceShow"),
-            disabled: toRef(props, "disabled"),
-            delayOnMouseover: toRef(props, "delayOnMouseover"),
-            delayOnMouseout: toRef(props, "delayOnMouseout"),
-            onShow: () => emit("show"),
-            onHide: () => emit("hide"),
-        });
-
-        watch(
-            () => [visible.value, props.disabled],
-            () => {
-                if (props.disabled && visible.value) {
-                    visible.value = false;
-                }
-            },
-            { flush: "sync" }
-        );
-
-        const handle =
-            (event: Parameters<typeof emit>[0]) =>
-            (...args: any[]) => {
-                return emit(event, ...args);
-            };
-
-        return {
-            visible,
-            reference,
-            popper,
-            handle,
-        };
-    },
+defineExpose({
+    visible,
+    reference,
+    popper,
 });
 </script>
 

client/src/components/Popper/usePopper.test.js

@@ -0,0 +1,69 @@
+import { createPopper } from "@popperjs/core";
+import { mount } from "@vue/test-utils";
+import { nextTick, ref } from "vue";
+
+import { usePopper } from "./usePopper";
+
+jest.mock("@popperjs/core", () => ({
+    createPopper: jest.fn(() => ({
+        destroy: jest.fn(),
+        update: jest.fn(),
+    })),
+}));
+
+describe("usePopper", () => {
+    let referenceElement;
+    let popperElement;
+
+    beforeEach(() => {
+        referenceElement = document.createElement("div");
+        document.body.appendChild(referenceElement);
+
+        popperElement = document.createElement("div");
+        document.body.appendChild(popperElement);
+    });
+
+    afterEach(() => {
+        document.body.innerHTML = "";
+        jest.clearAllMocks();
+    });
+
+    const createTestComponent = (trigger = "none") => {
+        return mount({
+            template: "<div></div>",
+            setup() {
+                const reference = ref(referenceElement);
+                const popper = ref(popperElement);
+                const options = { placement: "bottom", trigger };
+                const { visible, instance } = usePopper(reference, popper, options);
+                return { visible, instance };
+            },
+        });
+    };
+
+    test("should initialize Popper instance on mount", () => {
+        createTestComponent();
+        expect(createPopper).toHaveBeenCalledWith(referenceElement, popperElement, {
+            placement: "bottom",
+            strategy: "absolute",
+        });
+    });
+
+    test("should destroy Popper instance on unmount", () => {
+        const wrapper = createTestComponent();
+        const popperInstance = createPopper.mock.results[0].value;
+        wrapper.destroy();
+        expect(popperInstance.destroy).toHaveBeenCalled();
+    });
+
+    test("should not change visibility for trigger 'none'", async () => {
+        const wrapper = createTestComponent("none");
+        const { visible } = wrapper.vm;
+
+        expect(visible).toBe(false);
+
+        referenceElement.dispatchEvent(new Event("click"));
+        await nextTick();
+        expect(visible).toBe(false);
+    });
+});

client/src/components/Popper/usePopper.ts

@@ -1,252 +1,55 @@
-import { createPopper } from "@popperjs/core";
-import { onMounted, onUnmounted, onUpdated, type Ref, ref, unref, watch } from "vue";
+import { createPopper, type Placement } from "@popperjs/core";
+import { onMounted, onUnmounted, type Ref, ref, watch } from "vue";
 
-export type MaybeRef<T> = T | Ref<T>;
-
-export type Trigger = "hover" | "focus" | "click-to-open" | "click-to-toggle" | "manual";
-
-export type EventOptions = {
-    onShow: () => void;
-    onHide: () => void;
-};
-
-function on(element: Element, event: string, handler: EventListenerOrEventListenerObject) {
-    if (element && event && handler) {
-        element.addEventListener(event, handler, false);
-    }
-}
-function off(element: Element, event: string, handler: EventListenerOrEventListenerObject) {
-    if (element && event) {
-        element.removeEventListener(event, handler, false);
-    }
-}
+export type Trigger = "click" | "hover" | "none";
 
 const defaultTrigger: Trigger = "hover";
 
-export function usePopperjs(
-    reference: MaybeRef<Parameters<typeof createPopper>["0"]>,
-    popper: MaybeRef<Parameters<typeof createPopper>["1"]>,
-    options?: Partial<
-        Parameters<typeof createPopper>["2"] &
-            EventOptions & {
-                trigger: MaybeRef<Trigger | undefined>;
-                delayOnMouseover: MaybeRef<number | undefined>;
-                delayOnMouseout: MaybeRef<number | undefined>;
-                disabled: MaybeRef<boolean | undefined>;
-                forceShow: MaybeRef<boolean | undefined>;
-            }
-    >
+export function usePopper(
+    reference: Ref<HTMLElement>,
+    popper: Ref<HTMLElement>,
+    options: { placement?: Placement; trigger?: Trigger }
 ) {
-    const isMounted = ref(false);
-    onMounted(() => {
-        isMounted.value = true;
-    });
-    onUnmounted(() => {
-        isMounted.value = false;
-        destroy();
-    });
-
-    const updatedFlag = ref(true);
-    onUpdated(() => {
-        updatedFlag.value = !updatedFlag.value;
-    });
-
-    const referenceRef = ref<Element>();
-    const popperRef = ref<HTMLElement>();
-    watch(
-        () => [isMounted.value, updatedFlag.value],
-        () => {
-            if (!isMounted.value) {
-                return;
-            }
-            if ((unref(reference) as any)?.$el) {
-                referenceRef.value = (unref(reference) as any).$el;
-            } else {
-                referenceRef.value = unref(reference) as Element;
-            }
-
-            if ((unref(popper) as any)?.$el) {
-                popperRef.value = (unref(popper) as any).$el;
-            } else {
-                popperRef.value = unref(popper);
-            }
-        }
-    );
-
     const instance = ref<ReturnType<typeof createPopper>>();
-    watch(
-        () => [referenceRef.value, popperRef.value],
-        () => {
-            destroy();
-            if (!referenceRef.value) {
-                return;
-            }
-            if (!popperRef.value) {
-                return;
-            }
-            concrete();
-        }
-    );
-    const concrete = () => {
-        instance.value = createPopper(referenceRef.value!, popperRef.value!, {
-            placement: options?.placement ?? "bottom",
-            modifiers: options?.modifiers ?? [],
-            strategy: options?.strategy ?? "absolute",
-            onFirstUpdate: options?.onFirstUpdate ?? undefined,
-        });
-    };
-    const destroy = () => {
-        instance.value?.destroy();
-        instance.value = undefined;
-    };
-
     const visible = ref(false);
-    const doToggle = () => (visible.value = !visible.value);
+    const listeners: Array<{ target: EventTarget; event: string; handler: EventListener }> = [];
+
     const doOpen = () => (visible.value = true);
     const doClose = () => (visible.value = false);
-    watch(
-        () => [instance.value, unref(options?.trigger), unref(options?.forceShow)],
-        () => {
-            if (!instance.value) {
-                return;
-            }
-
-            if (unref(options?.forceShow)) {
-                visible.value = true;
-                doOff();
-                return;
-            }
-
-            doOn();
-        }
-    );
-
-    watch(
-        () => unref(options?.forceShow),
-        () => {
-            if (unref(options?.forceShow)) {
-                return;
-            }
-            if (unref(options?.trigger) === "manual") {
-                return;
-            }
+    const doCloseForDocument = (e: Event) => {
+        if (!reference.value?.contains(e.target as Node) && !popper.value?.contains(e.target as Node)) {
             visible.value = false;
         }
-    );
-
-    watch(
-        () => unref(options?.disabled),
-        () => {
-            if (unref(options?.disabled)) {
-                doOff();
-            } else {
-                doOn();
-            }
-        }
-    );
-
-    const timer = ref<any>();
-    const doMouseover = () => {
-        if (unref(options?.delayOnMouseover) === 0) {
-            doOpen();
-        } else {
-            clearTimeout(timer.value);
-            timer.value = setTimeout(() => {
-                doOpen();
-            }, unref(options?.delayOnMouseover) ?? 100);
-        }
-    };
-    const doMouseout = () => {
-        if (unref(options?.delayOnMouseout) === 0) {
-            doClose();
-        } else {
-            clearTimeout(timer.value);
-            timer.value = setTimeout(() => {
-                doClose();
-            }, unref(options?.delayOnMouseout) ?? 100);
-        }
     };
 
-    const doOn = () => {
-        doOff();
-
-        switch (unref(options?.trigger) ?? defaultTrigger) {
-            case "click-to-open": {
-                on(referenceRef.value!, "click", doOpen);
-                on(document as any, "click", doCloseForDocument);
-                break;
-            }
-
-            case "click-to-toggle": {
-                on(referenceRef.value!, "click", doToggle);
-                on(document as any, "click", doCloseForDocument);
-                break;
-            }
-
-            case "hover": {
-                on(referenceRef.value!, "mouseover", doMouseover);
-                on(referenceRef.value!, "mouseout", doMouseout);
-                on(referenceRef.value!, "mousedown", doMouseout);
-                break;
-            }
-
-            case "focus": {
-                on(referenceRef.value!, "focus", doOpen);
-                on(referenceRef.value!, "blur", doClose);
-                break;
-            }
-
-            case "manual": {
-                break;
-            }
-
-            default: {
-                throw TypeError();
-            }
-        }
-    };
-    const doOff = () => {
-        off(referenceRef.value!, "click", doOpen);
-        off(document as any, "click", doCloseForDocument);
-
-        off(referenceRef.value!, "click", doToggle);
-
-        off(referenceRef.value!, "mouseover", doMouseover);
-        off(referenceRef.value!, "mouseout", doMouseout);
-        off(referenceRef.value!, "mousedown", doMouseout);
-
-        off(referenceRef.value!, "focus", doOpen);
-        off(referenceRef.value!, "blur", doClose);
-    };
-    const doCloseForDocument = (e: Event) => {
-        if (referenceRef.value?.contains(e.target as Element)) {
-            return;
-        }
-        if (popperRef.value?.contains(e.target as Element)) {
-            return;
-        }
-        doClose();
+    const addEventListener = (target: EventTarget, event: string, handler: EventListener) => {
+        target.addEventListener(event, handler);
+        listeners.push({ target, event, handler });
     };
 
-    watch(
-        () => [instance.value, visible.value],
-        () => {
-            if (!instance.value) {
-                return;
-            }
-            if (visible.value || unref(options?.forceShow)) {
-                popperRef.value?.classList.remove("vue-use-popperjs-none");
-                options?.onShow?.();
-                instance.value?.update();
-            } else {
-                popperRef.value?.classList.add("vue-use-popperjs-none");
-                options?.onHide?.();
-            }
-        }
-    );
+    onMounted(() => {
+        instance.value = createPopper(reference.value, popper.value, {
+            placement: options.placement ?? "bottom",
+            strategy: "absolute",
+        });
 
-    return {
-        instance,
-        visible,
-    };
+        const trigger = options.trigger ?? defaultTrigger;
+        if (trigger === "click") {
+            addEventListener(reference.value, "click", doOpen);
+            addEventListener(document, "click", doCloseForDocument);
+        } else if (trigger === "hover") {
+            addEventListener(reference.value, "mouseover", doOpen);
+            addEventListener(reference.value, "mouseout", doClose);
+        }
+    });
+
+    onUnmounted(() => {
+        instance.value?.destroy();
+        listeners.forEach(({ target, event, handler }) => target.removeEventListener(event, handler));
+        listeners.length = 0;
+    });
+
+    watch([instance, visible], () => instance.value?.update());
+
+    return { instance, visible };
 }

client/src/components/Upload/UploadSettings.vue

@@ -31,7 +31,7 @@ const emit = defineEmits(["input"]);
 </script>
 
 <template>
-    <Popper placement="bottom" title="Upload Configuration" mode="primary-title" trigger="click-to-open">
+    <Popper placement="bottom" title="Upload Configuration" mode="primary-title" trigger="click">
         <template v-slot:reference>
             <FontAwesomeIcon class="cursor-pointer" icon="fa-cog" />
         </template>