Skip to content

fix: use virtual-modal-container #1709

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
elibosley merged 3 commits into from
Sep 15, 2025
Merged

fix: use virtual-modal-container #1709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9175bed
chore: ensure teleport container
elibosley Sep 15, 2025
4f0e0fa
refactor(useTeleport): simplify modal container initialization by alw…
elibosley Sep 15, 2025
dd2f2f9
test(useTeleport): enhance tests for teleportTarget and virtual conta…
elibosley Sep 15, 2025
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
206 changes: 57 additions & 149 deletions unraid-ui/src/composables/useTeleport.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,169 +1,77 @@
import useTeleport from '@/composables/useTeleport';
import { mount } from '@vue/test-utils';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

// Mock Vue's computed while preserving other exports
vi.mock('vue', async () => ({
...(await vi.importActual('vue')),
computed: vi.fn((fn) => {
const result = { value: fn() };
return result;
}),
}));
import { defineComponent } from 'vue';

describe('useTeleport', () => {
beforeEach(() => {
// Clear the DOM before each test
document.body.innerHTML = '';
document.head.innerHTML = '';
vi.clearAllMocks();
});

afterEach(() => {
vi.restoreAllMocks();
});

it('should return teleportTarget computed property', () => {
const { teleportTarget } = useTeleport();
expect(teleportTarget).toBeDefined();
expect(teleportTarget).toHaveProperty('value');
});

it('should return #modals when element with id="modals" exists', () => {
// Create element with id="modals"
const modalsDiv = document.createElement('div');
modalsDiv.id = 'modals';
document.body.appendChild(modalsDiv);

const { teleportTarget } = useTeleport();
expect(teleportTarget.value).toBe('#modals');
});

it('should prioritize #modals id over mounted unraid-modals', () => {
// Create both elements
const modalsDiv = document.createElement('div');
modalsDiv.id = 'modals';
document.body.appendChild(modalsDiv);

const unraidModals = document.createElement('unraid-modals');
unraidModals.setAttribute('data-vue-mounted', 'true');
document.body.appendChild(unraidModals);

const { teleportTarget } = useTeleport();
expect(teleportTarget.value).toBe('#modals');
});

it('should return mounted unraid-modals with inner #modals div', () => {
// Create mounted unraid-modals with inner modals div
const unraidModals = document.createElement('unraid-modals');
unraidModals.setAttribute('data-vue-mounted', 'true');

const innerModals = document.createElement('div');
innerModals.id = 'modals';
unraidModals.appendChild(innerModals);

document.body.appendChild(unraidModals);

const { teleportTarget } = useTeleport();
expect(teleportTarget.value).toBe('#modals');
});

it('should add id to mounted unraid-modals when no inner modals div exists', () => {
// Create mounted unraid-modals without inner div
const unraidModals = document.createElement('unraid-modals');
unraidModals.setAttribute('data-vue-mounted', 'true');
document.body.appendChild(unraidModals);

const { teleportTarget } = useTeleport();
expect(unraidModals.id).toBe('unraid-modals-teleport-target');
expect(teleportTarget.value).toBe('#unraid-modals-teleport-target');
});

it('should use existing id of mounted unraid-modals if present', () => {
// Create mounted unraid-modals with existing id
const unraidModals = document.createElement('unraid-modals');
unraidModals.setAttribute('data-vue-mounted', 'true');
unraidModals.id = 'custom-modals-id';
document.body.appendChild(unraidModals);

const { teleportTarget } = useTeleport();
expect(teleportTarget.value).toBe('#custom-modals-id');
// Clean up virtual container if it exists
const virtualContainer = document.getElementById('unraid-api-modals-virtual');
if (virtualContainer) {
virtualContainer.remove();
}
// Reset the module to clear the virtualModalContainer variable
vi.resetModules();
});

it('should ignore unmounted unraid-modals elements', () => {
// Create unmounted unraid-modals (without data-vue-mounted attribute)
const unraidModals = document.createElement('unraid-modals');
document.body.appendChild(unraidModals);

it('should return teleportTarget ref with correct value', () => {
const { teleportTarget } = useTeleport();
expect(teleportTarget.value).toBe('body');
expect(teleportTarget.value).toBe('#unraid-api-modals-virtual');
});

it('should ignore unraid-modals with data-vue-mounted="false"', () => {
// Create unraid-modals with data-vue-mounted="false"
const unraidModals = document.createElement('unraid-modals');
unraidModals.setAttribute('data-vue-mounted', 'false');
document.body.appendChild(unraidModals);

const { teleportTarget } = useTeleport();
expect(teleportTarget.value).toBe('body');
it('should create virtual container element on mount with correct properties', () => {
const TestComponent = defineComponent({
setup() {
const { teleportTarget } = useTeleport();
return { teleportTarget };
},
template: '<div>{{ teleportTarget }}</div>',
});

// Initially, virtual container should not exist
expect(document.getElementById('unraid-api-modals-virtual')).toBeNull();

// Mount the component
mount(TestComponent);

// After mount, virtual container should be created with correct properties
const virtualContainer = document.getElementById('unraid-api-modals-virtual');
expect(virtualContainer).toBeTruthy();
expect(virtualContainer?.className).toBe('unapi');
expect(virtualContainer?.style.position).toBe('relative');
expect(virtualContainer?.style.zIndex).toBe('999999');
expect(virtualContainer?.parentElement).toBe(document.body);
});

it('should return body as fallback when no suitable target exists', () => {
// No elements in DOM
const { teleportTarget } = useTeleport();
expect(teleportTarget.value).toBe('body');
});

it('should handle multiple unraid-modals elements correctly', () => {
// Create multiple unraid-modals, only one mounted
const unmountedModals1 = document.createElement('unraid-modals');
document.body.appendChild(unmountedModals1);

const mountedModals = document.createElement('unraid-modals');
mountedModals.setAttribute('data-vue-mounted', 'true');
mountedModals.id = 'mounted-modals';
document.body.appendChild(mountedModals);

const unmountedModals2 = document.createElement('unraid-modals');
document.body.appendChild(unmountedModals2);

const { teleportTarget } = useTeleport();
expect(teleportTarget.value).toBe('#mounted-modals');
});

it('should handle nested modal elements correctly', () => {
// Create nested structure
const container = document.createElement('div');

const unraidModals = document.createElement('unraid-modals');
unraidModals.setAttribute('data-vue-mounted', 'true');

const innerDiv = document.createElement('div');
const innerModals = document.createElement('div');
innerModals.id = 'modals';

innerDiv.appendChild(innerModals);
unraidModals.appendChild(innerDiv);
container.appendChild(unraidModals);
document.body.appendChild(container);

const { teleportTarget } = useTeleport();
expect(teleportTarget.value).toBe('#modals');
});

it('should be reactive to DOM changes', () => {
const { teleportTarget } = useTeleport();

// Initially should be body
expect(teleportTarget.value).toBe('body');

// Add modals element
const modalsDiv = document.createElement('div');
modalsDiv.id = 'modals';
document.body.appendChild(modalsDiv);

// Recreate the composable to test updated DOM state
const { teleportTarget: newTeleportTarget } = useTeleport();
expect(newTeleportTarget.value).toBe('#modals');
it('should reuse existing virtual container within same test', () => {
// Manually create the container first
const manualContainer = document.createElement('div');
manualContainer.id = 'unraid-api-modals-virtual';
manualContainer.className = 'unapi';
manualContainer.style.position = 'relative';
manualContainer.style.zIndex = '999999';
document.body.appendChild(manualContainer);

const TestComponent = defineComponent({
setup() {
const { teleportTarget } = useTeleport();
return { teleportTarget };
},
template: '<div>{{ teleportTarget }}</div>',
});

// Mount component - should not create a new container
mount(TestComponent);

// Should still have only one container
const containers = document.querySelectorAll('#unraid-api-modals-virtual');
expect(containers.length).toBe(1);
expect(containers[0]).toBe(manualContainer);
});
});
43 changes: 18 additions & 25 deletions unraid-ui/src/composables/useTeleport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,24 @@
import { computed } from 'vue';
import { onMounted, ref } from 'vue';

@coderabbitai coderabbitai Bot Sep 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Public API regression: ComputedRef → Ref breaks consumers. Keep ComputedRef for BC.

Call sites typed as ComputedRef<string> will fail now. Use computed to retain the previous type while keeping the new container behavior.

Apply this diff:

-import { onMounted, ref } from 'vue';
+import { onMounted, computed } from 'vue';
@@
-const useTeleport = () => {
-  const teleportTarget = ref<string>('#unraid-api-modals-virtual');
+const useTeleport = () => {
+  const teleportTarget = computed<string>(() => VIRTUAL_SELECTOR);
@@
   return {
     teleportTarget,
   };
 };

Also applies to: 17-27

🤖 Prompt for AI Agents 
In unraid-ui/src/composables/useTeleport.ts around lines 1 and 17-27, the
function currently returns a Ref<string> which breaks backward compatibility for
callers expecting ComputedRef<string>; wrap the internal ref with Vue's computed
and return a ComputedRef instead (keep the reactive inner ref for mutability but
expose a computed getter/setter) so the public API remains ComputedRef<string>
while preserving the new container behavior.


const useTeleport = () => {
// Computed property that finds the correct teleport target
const teleportTarget = computed(() => {
// #modals should be unique (id), but let's be defensive
const modalsElement = document.getElementById('modals');
if (modalsElement) return `#modals`;
let virtualModalContainer: HTMLDivElement | null = null;

const ensureVirtualContainer = () => {
if (!virtualModalContainer) {
virtualModalContainer = document.createElement('div');
virtualModalContainer.id = 'unraid-api-modals-virtual';
virtualModalContainer.className = 'unapi';
virtualModalContainer.style.position = 'relative';
virtualModalContainer.style.zIndex = '999999';
document.body.appendChild(virtualModalContainer);
}
return virtualModalContainer;
};
Comment on lines +5 to 15

@coderabbitai coderabbitai Bot Sep 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Prevent duplicate IDs and HMR leaks: reuse existing node if present.

If this module is reloaded (HMR) or used alongside another bundle, you can end up with multiple divs sharing the same id. Query and reuse an existing element before creating a new one.

Apply this diff:

 const ensureVirtualContainer = () => {
-  if (!virtualModalContainer) {
-    virtualModalContainer = document.createElement('div');
-    virtualModalContainer.id = 'unraid-api-modals-virtual';
-    virtualModalContainer.className = 'unapi';
-    virtualModalContainer.style.position = 'relative';
-    virtualModalContainer.style.zIndex = '999999';
-    document.body.appendChild(virtualModalContainer);
-  }
+  if (!virtualModalContainer || !virtualModalContainer.isConnected) {
+    // Reuse existing node if present (HMR, multiple mounts, etc.)
+    const existing = document.getElementById(VIRTUAL_CONTAINER_ID) as HTMLDivElement | null;
+    if (existing) {
+      virtualModalContainer = existing;
+      return virtualModalContainer;
+    }
+    virtualModalContainer = document.createElement('div');
+    virtualModalContainer.id = VIRTUAL_CONTAINER_ID;
+    virtualModalContainer.className = 'unapi unraid-api-modals-virtual';
+    virtualModalContainer.style.position = 'relative';
+    virtualModalContainer.style.zIndex = VIRTUAL_Z_INDEX;
+    document.body.appendChild(virtualModalContainer);
+  }
   return virtualModalContainer;
 };

Optional dev-only cleanup for Vite HMR (requires vite/client types enabled in tsconfig or via a triple-slash ref):

/// <reference types="vite/client" />
if (import.meta.hot) {
  import.meta.hot.dispose(() => {
    const el = document.getElementById(VIRTUAL_CONTAINER_ID);
    if (el?.parentNode) el.parentNode.removeChild(el);
    virtualModalContainer = null;
  });
}
🤖 Prompt for AI Agents 
In unraid-ui/src/composables/useTeleport.ts around lines 5 to 15, the function
always creates a new div with the same id which can produce duplicate IDs during
HMR or when multiple bundles run; instead first query
document.getElementById(VIRTUAL_CONTAINER_ID) and, if found, reuse and assign it
to virtualModalContainer (ensuring className, style and positioning are set if
needed), otherwise create and append the element; additionally, add an optional
dev-only import.meta.hot.dispose handler to remove the element and null out
virtualModalContainer on HMR disposal.


// Find only mounted unraid-modals components (data-vue-mounted="true")
// This ensures we don't target unmounted or duplicate elements
const mountedModals = document.querySelector('unraid-modals[data-vue-mounted="true"]');
if (mountedModals) {
// Check if it has the inner #modals div
const innerModals = mountedModals.querySelector('#modals');
if (innerModals && innerModals.id) {
return `#${innerModals.id}`;
}
// Use the mounted component itself as fallback
// Add a unique identifier if it doesn't have one
if (!mountedModals.id) {
mountedModals.id = 'unraid-modals-teleport-target';
}
return `#${mountedModals.id}`;
}
const useTeleport = () => {
const teleportTarget = ref<string>('#unraid-api-modals-virtual');

// Final fallback to body - modals component not mounted yet
return 'body';
onMounted(() => {
ensureVirtualContainer();
});

return {
Expand Down
Loading