Skip to content

Commit

Permalink
fix(editor): Use web native <a> element in nav menus
Browse files Browse the repository at this point in the history
Currently all navigation in the menus is done programmatically. This is not
accessible way to do navigation, as it prevents browser default behaviour,
like cmd/ctrl+click to open into new tab. Also screen readers don't give
any indication that the active element is an anchor.

This PR refactors component library's Menu and MenuItem to use either
vue-router RouterLink or <a> when the menu item is a navigation element.
  • Loading branch information
tomi committed Jan 18, 2024
1 parent 1aa35b1 commit 67fd89b
Show file tree
Hide file tree
Showing 15 changed files with 269 additions and 287 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,44 @@
<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 { h, useAttrs, useSlots } from 'vue';
import type { RouterLinkProps } from 'vue-router';
import { RouterLink } from 'vue-router';
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 slots = useSlots();
const attrs = useAttrs();
const renderContent = () => {
const { to } = props;
const { href } = attrs;
if (!to && !href) {
return slots.default?.();
}
return to
? // to is required to be passed explicitly for TS to be happy
h(RouterLink, { ...props, to }, () => slots.default?.())
: h('a', { ...attrs }, slots.default?.());
};
</script>

<template>
<div>
<!-- This will render either RouterLink or just the slot content -->
<component :is="renderContent" />
</div>
</template>
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'
);
}
26 changes: 19 additions & 7 deletions packages/design-system/src/types/menu.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import type { ElTooltipProps } from 'element-plus';
import type { AnchorHTMLAttributes } from 'vue';
import type { RouteLocationRaw, RouterLinkProps } from 'vue-router';

export type IMenuItem = {
id: string;
Expand All @@ -7,22 +9,32 @@ export type IMenuItem = {
secondaryIcon?: {
name: string;
size?: 'xsmall' | 'small' | 'medium' | 'large' | 'xlarge';
tooltip?: ElTooltipProps;
tooltip?: Partial<ElTooltipProps>;
};
customIconSize?: 'medium' | 'small';
available?: boolean;
position?: 'top' | 'bottom';
type?: 'default' | 'link';
properties?: ILinkMenuItemProperties;
// For router menus populate only one of those arrays:
// If menu item can be activated on certain route names (easy mode)

/** Use this for external links */
link?: ILinkMenuItemProperties;
/** Use this for defining a vue-router target */
route?: RouterLinkProps;
/**
* If given, item will be activated on these route names. Note that if
* route is provided, it will be highlighted automatically
*/
activateOnRouteNames?: string[];
// For more specific matching, we can use paths
activateOnRoutePaths?: string[];

children?: IMenuItem[];
};

export type IRouteMenuItemProperties = {
route: RouteLocationRaw;
};

export type ILinkMenuItemProperties = {
href: string;
newWindow?: boolean;
target?: AnchorHTMLAttributes['target'];
rel?: AnchorHTMLAttributes['rel'];
};
1 change: 0 additions & 1 deletion packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1270,7 +1270,6 @@ export interface UIState {
nodeViewOffsetPosition: XYPosition;
nodeViewMoveInProgress: boolean;
selectedNodes: INodeUi[];
sidebarMenuItems: IMenuItem[];
nodeViewInitialized: boolean;
addFirstStepOnLoad: boolean;
executionSidebarAutoRefresh: boolean;
Expand Down
Loading

0 comments on commit 67fd89b

Please sign in to comment.