Skip to content

Commit

Permalink
feat(editor): Separate node output execution tooltip from status icon (
Browse files Browse the repository at this point in the history
  • Loading branch information
CharlieKolb authored Oct 21, 2024
1 parent 321d6de commit cd15e95
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 65 deletions.
1 change: 1 addition & 0 deletions cypress/e2e/40-manual-partial-execution.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('Manual partial execution', () => {
canvas.actions.openNode('Webhook1');

ndv.getters.nodeRunSuccessIndicator().should('exist');
ndv.getters.nodeRunTooltipIndicator().should('exist');
ndv.getters.outputRunSelector().should('not.exist'); // single run
});
});
8 changes: 6 additions & 2 deletions cypress/e2e/5-ndv.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,10 @@ describe('NDV', () => {
"An expression here won't work because it uses .item and n8n can't figure out the matching item.",
);
ndv.getters.nodeRunErrorIndicator().should('be.visible');
ndv.getters.nodeRunTooltipIndicator().should('be.visible');
// The error details should be hidden behind a tooltip
ndv.getters.nodeRunErrorIndicator().should('not.contain', 'Start Time');
ndv.getters.nodeRunErrorIndicator().should('not.contain', 'Execution Time');
ndv.getters.nodeRunTooltipIndicator().should('not.contain', 'Start Time');
ndv.getters.nodeRunTooltipIndicator().should('not.contain', 'Execution Time');
});

it('should save workflow using keyboard shortcut from NDV', () => {
Expand Down Expand Up @@ -617,8 +618,10 @@ describe('NDV', () => {
// Should not show run info before execution
ndv.getters.nodeRunSuccessIndicator().should('not.exist');
ndv.getters.nodeRunErrorIndicator().should('not.exist');
ndv.getters.nodeRunTooltipIndicator().should('not.exist');
ndv.getters.nodeExecuteButton().click();
ndv.getters.nodeRunSuccessIndicator().should('exist');
ndv.getters.nodeRunTooltipIndicator().should('exist');
});

it('should properly show node execution indicator for multiple nodes', () => {
Expand All @@ -630,6 +633,7 @@ describe('NDV', () => {
// Manual tigger node should show success indicator
workflowPage.actions.openNode('When clicking ‘Test workflow’');
ndv.getters.nodeRunSuccessIndicator().should('exist');
ndv.getters.nodeRunTooltipIndicator().should('exist');
// Code node should show error
ndv.getters.backToCanvas().click();
workflowPage.actions.openNode('Code');
Expand Down
5 changes: 3 additions & 2 deletions cypress/pages/ndv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,9 @@ export class NDV extends BasePage {
codeEditorFullscreenButton: () => cy.getByTestId('code-editor-fullscreen-button'),
codeEditorDialog: () => cy.getByTestId('code-editor-fullscreen'),
codeEditorFullscreen: () => this.getters.codeEditorDialog().find('.cm-content'),
nodeRunSuccessIndicator: () => cy.getByTestId('node-run-info-success'),
nodeRunErrorIndicator: () => cy.getByTestId('node-run-info-danger'),
nodeRunTooltipIndicator: () => cy.getByTestId('node-run-info'),
nodeRunSuccessIndicator: () => cy.getByTestId('node-run-status-success'),
nodeRunErrorIndicator: () => cy.getByTestId('node-run-status-danger'),
nodeRunErrorMessage: () => cy.getByTestId('node-error-message'),
nodeRunErrorDescription: () => cy.getByTestId('node-error-description'),
fixedCollectionParameter: (paramName: string) =>
Expand Down
58 changes: 22 additions & 36 deletions packages/design-system/src/components/N8nInfoTip/InfoTip.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,24 @@
import type { Placement } from 'element-plus';
import { computed } from 'vue';
import type { IconColor } from 'n8n-design-system/types/icon';
import N8nIcon from '../N8nIcon';
import N8nTooltip from '../N8nTooltip';
const THEME = ['info', 'info-light', 'warning', 'danger', 'success'] as const;
const TYPE = ['note', 'tooltip'] as const;
const ICON_MAP = {
info: 'info-circle',
// eslint-disable-next-line @typescript-eslint/naming-convention
'info-light': 'info-circle',
warning: 'exclamation-triangle',
danger: 'exclamation-triangle',
success: 'check-circle',
} as const;
type IconMap = typeof ICON_MAP;
interface InfoTipProps {
theme?: (typeof THEME)[number];
type?: (typeof TYPE)[number];
Expand All @@ -23,39 +35,11 @@ const props = withDefaults(defineProps<InfoTipProps>(), {
tooltipPlacement: 'top',
});
const iconData = computed((): { icon: string; color: string } => {
switch (props.theme) {
case 'info':
return {
icon: 'info-circle',
color: '--color-text-light)',
};
case 'info-light':
return {
icon: 'info-circle',
color: 'var(--color-foreground-dark)',
};
case 'warning':
return {
icon: 'exclamation-triangle',
color: 'var(--color-warning)',
};
case 'danger':
return {
icon: 'exclamation-triangle',
color: 'var(--color-danger)',
};
case 'success':
return {
icon: 'check-circle',
color: 'var(--color-success)',
};
default:
return {
icon: 'info-circle',
color: '--color-text-light)',
};
}
const iconData = computed<{ icon: IconMap[keyof IconMap]; color: IconColor }>(() => {
return {
icon: ICON_MAP[props.theme],
color: props.theme === 'info' || props.theme === 'info-light' ? 'text-base' : props.theme,
} as const;
});
</script>

Expand All @@ -69,14 +53,16 @@ const iconData = computed((): { icon: string; color: string } => {
[$style.bold]: bold,
}"
>
<!-- Note that the branching is required to support displaying
the slot either in the tooltip of the icon or following it -->
<N8nTooltip
v-if="type === 'tooltip'"
:placement="tooltipPlacement"
:popper-class="$style.tooltipPopper"
:disabled="type !== 'tooltip'"
>
<span :class="$style.iconText" :style="{ color: iconData.color }">
<N8nIcon :icon="iconData.icon" />
<span :class="$style.iconText">
<N8nIcon :icon="iconData.icon" :color="iconData.color" />
</span>
<template #content>
<span>
Expand All @@ -85,7 +71,7 @@ const iconData = computed((): { icon: string; color: string } => {
</template>
</N8nTooltip>
<span v-else :class="$style.iconText">
<N8nIcon :icon="iconData.icon" />
<N8nIcon :icon="iconData.icon" :color="iconData.color" />
<span>
<slot />
</span>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`N8nInfoTip > should render correctly as note 1`] = `"<div class="n8n-info-tip infoTip info note bold"><span class="iconText"><span class="n8n-text compact size-medium regular n8n-icon n8n-icon"><!----></span><span>Need help doing something?<a href="/docs" target="_blank">Open docs</a></span></span></div>"`;
exports[`N8nInfoTip > should render correctly as note 1`] = `
"<div class="n8n-info-tip infoTip info note bold">
<!-- Note that the branching is required to support displaying
the slot either in the tooltip of the icon or following it --><span class="iconText"><span class="n8n-text text-base compact size-medium regular n8n-icon n8n-icon"><!----></span><span>Need help doing something?<a href="/docs" target="_blank">Open docs</a></span></span>
</div>"
`;

exports[`N8nInfoTip > should render correctly as tooltip 1`] = `
"<div class="n8n-info-tip infoTip info tooltip bold"><span class="iconText el-tooltip__trigger el-tooltip__trigger"><span class="n8n-text compact size-medium regular n8n-icon n8n-icon"><!----></span></span>
"<div class="n8n-info-tip infoTip info tooltip bold">
<!-- Note that the branching is required to support displaying
the slot either in the tooltip of the icon or following it --><span class="iconText el-tooltip__trigger el-tooltip__trigger"><span class="n8n-text text-base compact size-medium regular n8n-icon n8n-icon"><!----></span></span>
<!--teleport start-->
<!--teleport end-->
</div>"
Expand Down
57 changes: 34 additions & 23 deletions packages/editor-ui/src/components/RunInfo.vue
Original file line number Diff line number Diff line change
Expand Up @@ -50,27 +50,38 @@ const runMetadata = computed(() => {
"
></span>
</n8n-info-tip>
<n8n-info-tip
v-else-if="runMetadata"
type="tooltip"
:theme="theme"
:data-test-id="`node-run-info-${theme}`"
tooltip-placement="right"
>
<div>
<n8n-text :bold="true" size="small"
>{{
runTaskData?.error
? i18n.baseText('runData.executionStatus.failed')
: i18n.baseText('runData.executionStatus.success')
}} </n8n-text
><br />
<n8n-text :bold="true" size="small">{{ i18n.baseText('runData.startTime') + ':' }}</n8n-text>
{{ runMetadata.startTime }}<br />
<n8n-text :bold="true" size="small">{{
i18n.baseText('runData.executionTime') + ':'
}}</n8n-text>
{{ runMetadata.executionTime }} {{ i18n.baseText('runData.ms') }}
</div>
</n8n-info-tip>
<div v-else-if="runMetadata" :class="$style.tooltipRow">
<n8n-info-tip type="note" :theme="theme" :data-test-id="`node-run-status-${theme}`" />
<n8n-info-tip
type="tooltip"
theme="info"
:data-test-id="`node-run-info`"
tooltip-placement="right"
>
<div>
<n8n-text :bold="true" size="small"
>{{
runTaskData?.error
? i18n.baseText('runData.executionStatus.failed')
: i18n.baseText('runData.executionStatus.success')
}} </n8n-text
><br />
<n8n-text :bold="true" size="small">{{
i18n.baseText('runData.startTime') + ':'
}}</n8n-text>
{{ runMetadata.startTime }}<br />
<n8n-text :bold="true" size="small">{{
i18n.baseText('runData.executionTime') + ':'
}}</n8n-text>
{{ runMetadata.executionTime }} {{ i18n.baseText('runData.ms') }}
</div>
</n8n-info-tip>
</div>
</template>

<style lang="scss" module>
.tooltipRow {
display: flex;
column-gap: var(--spacing-4xs);
}
</style>

0 comments on commit cd15e95

Please sign in to comment.