From cd15e959c7af82a7d8c682e94add2b2640624a70 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Mon, 21 Oct 2024 09:35:23 +0200 Subject: [PATCH] feat(editor): Separate node output execution tooltip from status icon (#11196) --- cypress/e2e/40-manual-partial-execution.cy.ts | 1 + cypress/e2e/5-ndv.cy.ts | 8 ++- cypress/pages/ndv.ts | 5 +- .../src/components/N8nInfoTip/InfoTip.vue | 58 +++++++------------ .../__snapshots__/InfoTip.spec.ts.snap | 11 +++- packages/editor-ui/src/components/RunInfo.vue | 57 ++++++++++-------- 6 files changed, 75 insertions(+), 65 deletions(-) diff --git a/cypress/e2e/40-manual-partial-execution.cy.ts b/cypress/e2e/40-manual-partial-execution.cy.ts index 5fe31b56ad..2eb129475f 100644 --- a/cypress/e2e/40-manual-partial-execution.cy.ts +++ b/cypress/e2e/40-manual-partial-execution.cy.ts @@ -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 }); }); diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index a591d62895..f2ccccb6ab 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -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', () => { @@ -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', () => { @@ -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'); diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts index cae1fb47b0..f32b1ff1a3 100644 --- a/cypress/pages/ndv.ts +++ b/cypress/pages/ndv.ts @@ -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) => diff --git a/packages/design-system/src/components/N8nInfoTip/InfoTip.vue b/packages/design-system/src/components/N8nInfoTip/InfoTip.vue index cde570d716..04ba27ae5f 100644 --- a/packages/design-system/src/components/N8nInfoTip/InfoTip.vue +++ b/packages/design-system/src/components/N8nInfoTip/InfoTip.vue @@ -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]; @@ -23,39 +35,11 @@ const props = withDefaults(defineProps(), { 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; }); @@ -69,14 +53,16 @@ const iconData = computed((): { icon: string; color: string } => { [$style.bold]: bold, }" > + - - + + - + diff --git a/packages/design-system/src/components/N8nInfoTip/__tests__/__snapshots__/InfoTip.spec.ts.snap b/packages/design-system/src/components/N8nInfoTip/__tests__/__snapshots__/InfoTip.spec.ts.snap index 4bbaaa8b5d..ef7db8dfe8 100644 --- a/packages/design-system/src/components/N8nInfoTip/__tests__/__snapshots__/InfoTip.spec.ts.snap +++ b/packages/design-system/src/components/N8nInfoTip/__tests__/__snapshots__/InfoTip.spec.ts.snap @@ -1,9 +1,16 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`N8nInfoTip > should render correctly as note 1`] = `"
Need help doing something?Open docs
"`; +exports[`N8nInfoTip > should render correctly as note 1`] = ` +"
+ Need help doing something?Open docs +
" +`; exports[`N8nInfoTip > should render correctly as tooltip 1`] = ` -"
+"
+
" diff --git a/packages/editor-ui/src/components/RunInfo.vue b/packages/editor-ui/src/components/RunInfo.vue index 31689dbc1e..f4e68657e5 100644 --- a/packages/editor-ui/src/components/RunInfo.vue +++ b/packages/editor-ui/src/components/RunInfo.vue @@ -50,27 +50,38 @@ const runMetadata = computed(() => { " > - -
- {{ - runTaskData?.error - ? i18n.baseText('runData.executionStatus.failed') - : i18n.baseText('runData.executionStatus.success') - }}
- {{ i18n.baseText('runData.startTime') + ':' }} - {{ runMetadata.startTime }}
- {{ - i18n.baseText('runData.executionTime') + ':' - }} - {{ runMetadata.executionTime }} {{ i18n.baseText('runData.ms') }} -
-
+
+ + +
+ {{ + runTaskData?.error + ? i18n.baseText('runData.executionStatus.failed') + : i18n.baseText('runData.executionStatus.success') + }}
+ {{ + i18n.baseText('runData.startTime') + ':' + }} + {{ runMetadata.startTime }}
+ {{ + i18n.baseText('runData.executionTime') + ':' + }} + {{ runMetadata.executionTime }} {{ i18n.baseText('runData.ms') }} +
+
+
+ +