fix(editor): Place the restricted node popover per host, keep one open, and stack the admin dialog (#40678)

This commit is contained in:
Theodore Nastos 2026-10-09 13:12:49 +00:00 committed by GitHub
parent a7cba90a85
commit b28dceb462
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 207 additions and 19 deletions

View File

@ -228,6 +228,7 @@ function handleConnect() {
:node-type-name="item.title"
:scope="restriction.scope"
:anchor="rowRef"
:side-offset="48"
/>
<N8nTooltip
v-else-if="isDisabled"

View File

@ -59,6 +59,7 @@ function mailtoHref(email: string): string {
:header="i18n.baseText('typeAvailabilityPolicies.contactAdmin.title')"
:description="description"
size="medium"
stacked
data-test-id="contact-instance-admin-modal"
>
<N8nLoading v-if="isLoading" :rows="2" variant="p" :class="$style.body" />

View File

@ -14,8 +14,9 @@ const renderComponent = createComponentRenderer(RestrictedNodePopover, {
stubs: {
ContactInstanceAdminModal: {
props: ['open', 'nodeTypeName'],
emits: ['update:open'],
template:
'<div v-if="open" data-test-id="contact-instance-admin-modal">{{ nodeTypeName }}</div>',
'<div v-if="open" data-test-id="contact-instance-admin-modal">{{ nodeTypeName }}<button data-test-id="contact-instance-admin-close" @click="$emit(\'update:open\', false)" /></div>',
},
},
},
@ -52,6 +53,14 @@ describe('RestrictedNodePopover', () => {
expect(screen.getByText(DESCRIPTION)).toBeInTheDocument();
});
it('places the popover where the host asks', async () => {
renderPopover({ active: true, side: 'top', align: 'end' });
const content = (await screen.findByTestId('node-restricted-popover')).closest('[data-side]');
expect(content).toHaveAttribute('data-side', 'top');
expect(content).toHaveAttribute('data-align', 'end');
});
it('names the project scope', async () => {
renderPopover({ active: true, scope: 'project' });
@ -75,6 +84,68 @@ describe('RestrictedNodePopover', () => {
expect(screen.getByTestId('node-restricted-popover')).toBeInTheDocument();
});
it('closes at once when the pointer moves to another restricted row', async () => {
const otherAnchor = document.createElement('div');
document.body.appendChild(otherAnchor);
renderPopover({ anchor, nodeTypeName: 'Gmail' });
renderPopover({ anchor: otherAnchor, nodeTypeName: 'Slack' });
await userEvent.hover(anchor);
expect(await screen.findByText('Gmail')).toBeInTheDocument();
await userEvent.hover(otherAnchor);
expect(await screen.findByText('Slack')).toBeInTheDocument();
expect(screen.queryByText('Gmail')).not.toBeInTheDocument();
otherAnchor.remove();
});
it('keeps every popover closed while a dialog opened from another row is open', async () => {
const otherAnchor = document.createElement('div');
document.body.appendChild(otherAnchor);
renderPopover({ anchor, nodeTypeName: 'Gmail' });
renderPopover({ anchor: otherAnchor, nodeTypeName: 'Slack' });
await userEvent.hover(anchor);
await screen.findByText('Gmail');
await userEvent.hover(otherAnchor);
const popover = await screen.findByText('Slack');
await userEvent.hover(popover);
await userEvent.click(screen.getByTestId('node-restricted-contact-admin'));
expect(screen.getByTestId('contact-instance-admin-modal')).toHaveTextContent('Slack');
await waitFor(() =>
expect(screen.queryByTestId('node-restricted-popover')).not.toBeInTheDocument(),
);
otherAnchor.remove();
});
it('reopens on hover after the contact-admin dialog was opened from the popover and closed', async () => {
const otherAnchor = document.createElement('div');
document.body.appendChild(otherAnchor);
renderPopover({ anchor, nodeTypeName: 'Gmail' });
renderPopover({ anchor: otherAnchor, nodeTypeName: 'Slack' });
await userEvent.hover(anchor);
await userEvent.hover(await screen.findByTestId('node-restricted-popover'));
await userEvent.unhover(anchor);
await userEvent.click(screen.getByTestId('node-restricted-contact-admin'));
await userEvent.click(screen.getByTestId('contact-instance-admin-close'));
await waitFor(() =>
expect(screen.queryByTestId('node-restricted-popover')).not.toBeInTheDocument(),
);
await userEvent.hover(otherAnchor);
expect(await screen.findByText('Slack')).toBeInTheDocument();
await userEvent.hover(anchor);
expect(await screen.findByText('Gmail')).toBeInTheDocument();
expect(screen.queryByText('Slack')).not.toBeInTheDocument();
otherAnchor.remove();
});
it('opens while focus is inside the anchor row and closes when focus leaves', async () => {
const button = document.createElement('button');
anchor.appendChild(button);

View File

@ -2,20 +2,27 @@
import type { NodeTypeAvailabilityScope } from '@n8n/api-types';
import { N8nButton, N8nIcon, N8nPopover, N8nText } from '@n8n/design-system';
import { useI18n, type BaseTextKey } from '@n8n/i18n';
import { unrefElement, useElementHover, useFocusWithin, type MaybeElement } from '@vueuse/core';
import { computed, ref } from 'vue';
import { unrefElement, useActiveElement, useElementHover, type MaybeElement } from '@vueuse/core';
import { computed, ref, watch } from 'vue';
import { useExclusiveOpen } from '../composables/useExclusiveOpen';
import { SCOPE_LABEL_KEY } from '../type-availability-policies.constants';
import ContactInstanceAdminModal from './ContactInstanceAdminModal.vue';
const props = defineProps<{
nodeTypeName: string;
scope?: NodeTypeAvailabilityScope;
/** The list row the popover explains. It opens beside this element, not beside the lock. */
anchor?: MaybeElement;
/** Keyboard-active without DOM focus, such as a virtual list selection. */
active?: boolean;
}>();
const props = withDefaults(
defineProps<{
nodeTypeName: string;
scope?: NodeTypeAvailabilityScope;
/** The list row the popover explains. It opens beside this element, not beside the lock. */
anchor?: MaybeElement;
/** Keyboard-active without DOM focus, such as a virtual list selection. */
active?: boolean;
side?: 'top' | 'right' | 'bottom' | 'left';
align?: 'start' | 'center' | 'end';
sideOffset?: number;
}>(),
{ scope: undefined, anchor: undefined, side: 'left', align: 'center', sideOffset: 24 },
);
/** Leaving waits this long before closing, so the pointer can cross the gap to the popover. */
const HOVER_GRACE_MS = 200;
@ -26,12 +33,18 @@ const anchorElement = computed(() => unrefElement(props.anchor) ?? undefined);
const contentRef = ref<HTMLElement | null>(null);
const anchorHovered = useElementHover(anchorElement, { delayLeave: HOVER_GRACE_MS });
const contentHovered = useElementHover(contentRef, { delayLeave: HOVER_GRACE_MS });
watch(contentRef, (content) => {
if (!content) contentHovered.value = false;
});
const isContactAdminOpen = ref(false);
// Content is teleported, so focus in it is outside the anchor.
// Close for the contact-admin dialog, which this would otherwise cover.
const { focused: anchorFocused } = useFocusWithin(anchorElement);
const { focused: contentFocused } = useFocusWithin(contentRef);
const open = computed(
const activeElement = useActiveElement();
const anchorFocused = computed(() =>
Boolean(anchorElement.value?.contains(activeElement.value ?? null)),
);
const contentFocused = computed(() =>
Boolean(contentRef.value?.contains(activeElement.value ?? null)),
);
const wantsOpen = computed(
() =>
!isContactAdminOpen.value &&
(anchorHovered.value ||
@ -40,6 +53,7 @@ const open = computed(
contentFocused.value ||
props.active),
);
const open = useExclusiveOpen(wantsOpen);
const scopeKey = computed<BaseTextKey>(
() =>
@ -53,9 +67,9 @@ const scopeKey = computed<BaseTextKey>(
<!-- The tool pickers render this inside a modal. -->
<N8nPopover
:open="open"
side="left"
align="center"
:side-offset="24"
:side="props.side"
:align="props.align"
:side-offset="props.sideOffset"
:reference="anchorElement"
:suppress-auto-focus="true"
:content-class="$style.card"

View File

@ -0,0 +1,78 @@
import { describe, it, expect } from 'vitest';
import { effectScope, nextTick, ref } from 'vue';
import { useExclusiveOpen } from './useExclusiveOpen';
function instance() {
const wantsOpen = ref(false);
const scope = effectScope();
const open = scope.run(() => useExclusiveOpen(wantsOpen))!;
return { wantsOpen, open, scope };
}
describe('useExclusiveOpen', () => {
it('closes the previous owner as soon as another instance wants to open', async () => {
const a = instance();
const b = instance();
a.wantsOpen.value = true;
await nextTick();
expect(a.open.value).toBe(true);
b.wantsOpen.value = true;
await nextTick();
expect(a.open.value).toBe(false);
expect(b.open.value).toBe(true);
a.scope.stop();
b.scope.stop();
});
it('keeps a displaced instance closed until it wants to open again', async () => {
const a = instance();
const b = instance();
a.wantsOpen.value = true;
b.wantsOpen.value = true;
await nextTick();
expect(b.open.value).toBe(true);
b.wantsOpen.value = false;
await nextTick();
expect(a.open.value).toBe(false);
a.wantsOpen.value = false;
await nextTick();
a.wantsOpen.value = true;
await nextTick();
expect(a.open.value).toBe(true);
a.scope.stop();
b.scope.stop();
});
it('opens an instance that already wants to open when it is created', async () => {
const wantsOpen = ref(true);
const scope = effectScope();
const open = scope.run(() => useExclusiveOpen(wantsOpen))!;
expect(open.value).toBe(true);
scope.stop();
});
it('releases the slot when the owner is disposed', async () => {
const a = instance();
const b = instance();
b.wantsOpen.value = true;
await nextTick();
b.scope.stop();
a.wantsOpen.value = true;
await nextTick();
expect(a.open.value).toBe(true);
a.scope.stop();
});
});

View File

@ -0,0 +1,23 @@
import { computed, onScopeDispose, ref, watch, type ComputedRef, type Ref } from 'vue';
const activeOwner = ref<symbol | null>(null);
export function useExclusiveOpen(wantsOpen: Ref<boolean>): ComputedRef<boolean> {
const owner = Symbol('exclusive-open');
const release = () => {
if (activeOwner.value === owner) activeOwner.value = null;
};
watch(
wantsOpen,
(wants) => {
if (wants) activeOwner.value = owner;
else release();
},
{ immediate: true },
);
onScopeDispose(release);
return computed(() => wantsOpen.value && activeOwner.value === owner);
}