Skip to content

Commit

Permalink
fix(editor): Use web native <a> element in nav menus (#8385)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomi authored Jan 19, 2024
1 parent 6fcf5dd commit e606e84
Show file tree
Hide file tree
Showing 22 changed files with 343 additions and 289 deletions.
1 change: 1 addition & 0 deletions packages/design-system/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"sanitize-html": "2.10.0",
"vue": "^3.3.4",
"vue-boring-avatars": "^1.3.0",
"vue-router": "^4.2.2",
"xss": "^1.0.14"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<script setup lang="ts">
/**
* Component that renders either a RouterLink or a normal anchor tag or
* just the slot content based on whether the `to` or `href` prop is
* passed or not.
*/
import { useAttrs } from 'vue';
import type { RouterLinkProps } from 'vue-router';
import { RouterLink } from 'vue-router';
defineOptions({
name: 'ConditionalRouterLink',
inheritAttrs: false,
});
const props = defineProps({
// @ts-expect-error TS doesn't understand this but it works
...RouterLink.props,
// Make to optional
to: {
type: [String, Object] as unknown as () => string | RouterLinkProps['to'] | undefined,
default: undefined,
},
// <a> element "props" are passed as attributes
}) as Partial<RouterLinkProps>;
const attrs = useAttrs();
</script>

<template>
<div>
<RouterLink v-if="props.to" v-bind="props" :to="props.to">
<slot />
</RouterLink>
<a v-else-if="attrs.href" v-bind="attrs">
<slot />
</a>
<slot v-else />
</div>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { render } from '@testing-library/vue';
import { beforeAll, describe } from 'vitest';
import { createRouter, createWebHistory } from 'vue-router';
import CondtionalRouterLink from '../CondtionalRouterLink.vue';

const slots = {
default: 'Button',
};

const router = createRouter({
history: createWebHistory(),
routes: [
{
path: '/',
name: 'home',
redirect: '/home',
},
],
});

describe('CondtionalRouterLink', () => {
beforeAll(async () => {
await router.push('/');

await router.isReady();
});

it("renders router-link when 'to' prop is passed", () => {
const wrapper = render(CondtionalRouterLink, {
props: {
to: { name: 'home' },
},
slots,
global: {
plugins: [router],
},
});

expect(wrapper.html()).toMatchSnapshot();
});

it("renders <a> when 'href' attr is passed", () => {
const wrapper = render(CondtionalRouterLink, {
attrs: {
href: 'https://n8n.io',
target: '_blank',
},
slots,
global: {
plugins: [router],
},
});

expect(wrapper.html()).toMatchSnapshot();
});

it('renders only the slot when neither to nor href is given', () => {
const wrapper = render(CondtionalRouterLink, {
slots,
global: {
plugins: [router],
},
});

expect(wrapper.html()).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`CondtionalRouterLink > renders <a> when 'href' attr is passed 1`] = `"<div><a href="https://n8n.io" target="_blank">Button</a></div>"`;
exports[`CondtionalRouterLink > renders only the slot when neither to nor href is given 1`] = `"<div>Button</div>"`;
exports[`CondtionalRouterLink > renders router-link when 'to' prop is passed 1`] = `"<div><a href="/" class="">Button</a></div>"`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import CondtionalRouterLink from './CondtionalRouterLink.vue';

export default CondtionalRouterLink;
10 changes: 4 additions & 6 deletions packages/design-system/src/components/N8nMenu/Menu.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,9 @@ const menuItems = [
id: 'website',
icon: 'globe',
label: 'Website',
type: 'link',
properties: {
link: {
href: 'https://www.n8n.io',
newWindow: true,
target: '_blank',
},
position: 'bottom',
},
Expand All @@ -140,10 +139,9 @@ const menuItems = [
id: 'quickstart',
icon: 'video',
label: 'Quickstart',
type: 'link',
properties: {
link: {
href: 'https://www.youtube.com/watch?v=RpjQTGKm-ok',
newWindow: true,
target: '_blank',
},
},
],
Expand Down
26 changes: 5 additions & 21 deletions packages/design-system/src/components/N8nMenu/Menu.vue
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import N8nMenuItem from '../N8nMenuItem';
import type { PropType } from 'vue';
import { defineComponent } from 'vue';
import type { IMenuItem, RouteObject } from '../../types';
import { doesMenuItemMatchCurrentRoute } from '../N8nMenuItem/routerUtil';
export default defineComponent({
name: 'N8nMenu',
Expand Down Expand Up @@ -128,14 +129,10 @@ export default defineComponent({
},
mounted() {
if (this.mode === 'router') {
const found = this.items.find((item) => {
return (
(Array.isArray(item.activateOnRouteNames) &&
item.activateOnRouteNames.includes(this.currentRoute.name || '')) ||
(Array.isArray(item.activateOnRoutePaths) &&
item.activateOnRoutePaths.includes(this.currentRoute.path))
);
});
const found = this.items.find((item) =>
doesMenuItemMatchCurrentRoute(item, this.currentRoute),
);
this.activeTab = found ? found.id : '';
} else {
this.activeTab = this.items.length > 0 ? this.items[0].id : '';
Expand All @@ -145,19 +142,6 @@ export default defineComponent({
},
methods: {
onSelect(item: IMenuItem): void {
if (item && item.type === 'link' && item.properties) {
const href: string = item.properties.href;
if (!href) {
return;
}
if (item.properties.newWindow) {
window.open(href);
} else {
window.location.assign(item.properties.href);
}
}
if (this.mode === 'tabs') {
this.activeTab = item.id;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,9 @@ link.args = {
id: 'website',
icon: 'globe',
label: 'Website',
type: 'link',
properties: {
link: {
href: 'https://www.n8n.io',
newWindow: true,
target: '_blank',
},
},
};
Expand All @@ -96,10 +95,9 @@ withChildren.args = {
id: 'quickstart',
icon: 'video',
label: 'Quickstart',
type: 'link',
properties: {
link: {
href: 'https://www.youtube.com/watch?v=RpjQTGKm-ok',
newWindow: true,
target: '_blank',
},
},
],
Expand Down
80 changes: 38 additions & 42 deletions packages/design-system/src/components/N8nMenuItem/MenuItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -40,37 +40,39 @@
:disabled="!compact"
:show-after="tooltipDelay"
>
<ElMenuItem
:id="item.id"
:class="{
[$style.menuItem]: true,
[$style.item]: true,
[$style.disableActiveStyle]: !isItemActive(item),
[$style.active]: isItemActive(item),
[$style.compact]: compact,
}"
data-test-id="menu-item"
:index="item.id"
@click="handleSelect(item)"
>
<N8nIcon
v-if="item.icon"
:class="$style.icon"
:icon="item.icon"
:size="item.customIconSize || 'large'"
/>
<span :class="$style.label">{{ item.label }}</span>
<N8nTooltip
v-if="item.secondaryIcon"
:class="$style.secondaryIcon"
:placement="item.secondaryIcon?.tooltip?.placement || 'right'"
:content="item.secondaryIcon?.tooltip?.content"
:disabled="compact || !item.secondaryIcon?.tooltip?.content"
:show-after="tooltipDelay"
<ConditionalRouterLink v-bind="item.route ?? item.link">
<ElMenuItem
:id="item.id"
:class="{
[$style.menuItem]: true,
[$style.item]: true,
[$style.disableActiveStyle]: !isItemActive(item),
[$style.active]: isItemActive(item),
[$style.compact]: compact,
}"
data-test-id="menu-item"
:index="item.id"
@click="handleSelect(item)"
>
<N8nIcon :icon="item.secondaryIcon.name" :size="item.secondaryIcon.size || 'small'" />
</N8nTooltip>
</ElMenuItem>
<N8nIcon
v-if="item.icon"
:class="$style.icon"
:icon="item.icon"
:size="item.customIconSize || 'large'"
/>
<span :class="$style.label">{{ item.label }}</span>
<N8nTooltip
v-if="item.secondaryIcon"
:class="$style.secondaryIcon"
:placement="item.secondaryIcon?.tooltip?.placement || 'right'"
:content="item.secondaryIcon?.tooltip?.content"
:disabled="compact || !item.secondaryIcon?.tooltip?.content"
:show-after="tooltipDelay"
>
<N8nIcon :icon="item.secondaryIcon.name" :size="item.secondaryIcon.size || 'small'" />
</N8nTooltip>
</ElMenuItem>
</ConditionalRouterLink>
</N8nTooltip>
</div>
</template>
Expand All @@ -81,7 +83,9 @@ import N8nTooltip from '../N8nTooltip';
import N8nIcon from '../N8nIcon';
import type { PropType } from 'vue';
import { defineComponent } from 'vue';
import ConditionalRouterLink from '../ConditionalRouterLink';
import type { IMenuItem, RouteObject } from '../../types';
import { doesMenuItemMatchCurrentRoute } from './routerUtil';
export default defineComponent({
name: 'N8nMenuItem',
Expand All @@ -90,6 +94,7 @@ export default defineComponent({
ElMenuItem,
N8nIcon,
N8nTooltip,
ConditionalRouterLink,
},
props: {
item: {
Expand All @@ -115,9 +120,11 @@ export default defineComponent({
},
activeTab: {
type: String,
default: undefined,
},
handleSelect: {
type: Function as PropType<(item: IMenuItem) => void>,
default: undefined,
},
},
computed: {
Expand Down Expand Up @@ -151,18 +158,7 @@ export default defineComponent({
},
isActive(item: IMenuItem): boolean {
if (this.mode === 'router') {
if (item.activateOnRoutePaths) {
return (
Array.isArray(item.activateOnRoutePaths) &&
item.activateOnRoutePaths.includes(this.currentRoute.path)
);
} else if (item.activateOnRouteNames) {
return (
Array.isArray(item.activateOnRouteNames) &&
item.activateOnRouteNames.includes(this.currentRoute.name || '')
);
}
return false;
return doesMenuItemMatchCurrentRoute(item, this.currentRoute);
} else {
return item.id === this.activeTab;
}
Expand Down
42 changes: 42 additions & 0 deletions packages/design-system/src/components/N8nMenuItem/routerUtil.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import type { IMenuItem, RouteObject } from '@/types';
import type { RouteLocationRaw } from 'vue-router';

/**
* Checks if the given menu item matches the current route.
*/
export function doesMenuItemMatchCurrentRoute(item: IMenuItem, currentRoute: RouteObject) {
let activateOnRouteNames: string[] = [];
if (Array.isArray(item.activateOnRouteNames)) {
activateOnRouteNames = item.activateOnRouteNames;
} else if (item.route && isNamedRouteLocation(item.route.to)) {
activateOnRouteNames = [item.route.to.name];
}

let activateOnRoutePaths: string[] = [];
if (Array.isArray(item.activateOnRoutePaths)) {
activateOnRoutePaths = item.activateOnRoutePaths;
} else if (item.route && isPathRouteLocation(item.route.to)) {
activateOnRoutePaths = [item.route.to.path];
}

return (
activateOnRouteNames.includes(currentRoute.name ?? '') ||
activateOnRoutePaths.includes(currentRoute.path)
);
}

function isPathRouteLocation(routeLocation?: RouteLocationRaw): routeLocation is { path: string } {
return (
typeof routeLocation === 'object' &&
'path' in routeLocation &&
typeof routeLocation.path === 'string'
);
}

function isNamedRouteLocation(routeLocation?: RouteLocationRaw): routeLocation is { name: string } {
return (
typeof routeLocation === 'object' &&
'name' in routeLocation &&
typeof routeLocation.name === 'string'
);
}
Loading

0 comments on commit e606e84

Please sign in to comment.