fix(editor): Refine push modal layout (#12886)

Co-authored-by: Csaba Tuncsik <csaba.tuncsik@gmail.com>

packages/editor-ui/package.json

@@ -41,8 +41,8 @@
     "@n8n/codemirror-lang-sql": "^1.0.2",
     "@n8n/permissions": "workspace:*",
     "@replit/codemirror-indentation-markers": "^6.5.3",
-    "@typescript/vfs": "^1.6.0",
     "@sentry/vue": "catalog:frontend",
+    "@typescript/vfs": "^1.6.0",
     "@vue-flow/background": "^1.3.2",
     "@vue-flow/controls": "^1.1.2",
     "@vue-flow/core": "^1.41.6",
@@ -56,6 +56,7 @@
     "chart.js": "^4.4.0",
     "codemirror-lang-html-n8n": "^1.0.0",
     "comlink": "^4.4.1",
+    "core-js": "^3.40.0",
     "dateformat": "^3.0.3",
     "email-providers": "^2.0.1",
     "esprima-next": "5.8.4",

packages/editor-ui/src/__tests__/setup.ts

@@ -1,5 +1,6 @@
 import '@testing-library/jest-dom';
 import { configure } from '@testing-library/vue';
+import 'core-js/proposals/set-methods-v2';
 
 configure({ testIdAttribute: 'data-test-id' });
 

packages/editor-ui/src/components/SourceControlPushModal.ee.test.ts

@@ -24,17 +24,25 @@ vi.mock('vue-router', () => ({
 
 let route: ReturnType<typeof useRoute>;
 
-const RecycleScroller = {
+const DynamicScrollerStub = {
 	props: {
 		items: Array,
 	},
 	template: '<div><template v-for="item in items"><slot v-bind="{ item }"></slot></template></div>',
+	methods: {
+		scrollToItem: vi.fn(),
+	},
+};
+
+const DynamicScrollerItemStub = {
+	template: '<slot></slot>',
 };
 
 const renderModal = createComponentRenderer(SourceControlPushModal, {
 	global: {
 		stubs: {
-			RecycleScroller,
+			DynamicScroller: DynamicScrollerStub,
+			DynamicScrollerItem: DynamicScrollerItemStub,
 			Modal: {
 				template: `
 					<div>
@@ -195,7 +203,7 @@ describe('SourceControlPushModal', () => {
 
 		const sourceControlStore = mockedStore(useSourceControlStore);
 
-		const { getByTestId, getByText } = renderModal({
+		const { getByTestId, getByRole } = renderModal({
 			props: {
 				data: {
 					eventBus,
@@ -207,9 +215,9 @@ describe('SourceControlPushModal', () => {
 		const submitButton = getByTestId('source-control-push-modal-submit');
 		const commitMessage = 'commit message';
 		expect(submitButton).toBeDisabled();
-		expect(getByText('1 new credentials added, 0 deleted and 0 changed')).toBeInTheDocument();
-		expect(getByText('At least one new variable has been added or modified')).toBeInTheDocument();
-		expect(getByText('At least one new tag has been added or modified')).toBeInTheDocument();
+		expect(getByRole('alert').textContent).toContain('Credentials: 1 added.');
+		expect(getByRole('alert').textContent).toContain('Variables: at least one new or modified.');
+		expect(getByRole('alert').textContent).toContain('Tags: at least one new or modified.');
 
 		await userEvent.type(getByTestId('source-control-push-modal-commit'), commitMessage);
 

packages/editor-ui/src/components/SourceControlPushModal.ee.vue

@@ -1,7 +1,7 @@
 <script lang="ts" setup>
 import Modal from './Modal.vue';
 import { SOURCE_CONTROL_PUSH_MODAL_KEY, VIEWS } from '@/constants';
-import { computed, onMounted, ref } from 'vue';
+import { computed, onMounted, ref, toRaw } from 'vue';
 import type { EventBus } from 'n8n-design-system/utils';
 import { useI18n } from '@/composables/useI18n';
 import { useLoadingService } from '@/composables/useLoadingService';
@@ -10,7 +10,7 @@ import { useSourceControlStore } from '@/stores/sourceControl.store';
 import { useUIStore } from '@/stores/ui.store';
 import { useRoute } from 'vue-router';
 import dateformat from 'dateformat';
-import { RecycleScroller } from 'vue-virtual-scroller';
+import { DynamicScroller, DynamicScrollerItem } from 'vue-virtual-scroller';
 import 'vue-virtual-scroller/dist/vue-virtual-scroller.css';
 import { refDebounced } from '@vueuse/core';
 import {
@@ -49,6 +49,9 @@ const i18n = useI18n();
 const sourceControlStore = useSourceControlStore();
 const route = useRoute();
 
+const concatenateWithAnd = (messages: string[]) =>
+	new Intl.ListFormat(i18n.locale, { style: 'long', type: 'conjunction' }).format(messages);
+
 type SourceControlledFileStatus = SourceControlledFile['status'];
 
 type Changes = {
@@ -101,22 +104,33 @@ const classifyFilesByType = (files: SourceControlledFile[], currentWorkflowId?:
 	);
 
 const userNotices = computed(() => {
-	const messages: string[] = [];
+	const messages: Array<{ title: string; content: string }> = [];
 
 	if (changes.value.credentials.length) {
 		const { created, deleted, modified } = groupBy(changes.value.credentials, 'status');
 
-		messages.push(
-			`${created?.length ?? 0} new credentials added, ${deleted?.length ?? 0} deleted and ${modified?.length ?? 0} changed`,
-		);
+		messages.push({
+			title: 'Credentials',
+			content: concatenateWithAnd([
+				...(created?.length ? [`${created.length} added`] : []),
+				...(deleted?.length ? [`${deleted.length} deleted`] : []),
+				...(modified?.length ? [`${modified.length} changed`] : []),
+			]),
+		});
 	}
 
 	if (changes.value.variables.length) {
-		messages.push('At least one new variable has been added or modified');
+		messages.push({
+			title: 'Variables',
+			content: 'at least one new or modified',
+		});
 	}
 
 	if (changes.value.tags.length) {
-		messages.push('At least one new tag has been added or modified');
+		messages.push({
+			title: 'Tags',
+			content: 'at least one new or modified',
+		});
 	}
 
 	return messages;
@@ -218,20 +232,38 @@ const isSubmitDisabled = computed(() => {
 	return false;
 });
 
-const selectAll = computed(
-	() =>
-		selectedChanges.value.size > 0 && selectedChanges.value.size === sortedWorkflows.value.length,
-);
+const sortedWorkflowsSet = computed(() => new Set(sortedWorkflows.value.map(({ id }) => id)));
 
-const selectAllIndeterminate = computed(
-	() => selectedChanges.value.size > 0 && selectedChanges.value.size < sortedWorkflows.value.length,
-);
+const selectAll = computed(() => {
+	if (!selectedChanges.value.size) {
+		return false;
+	}
+
+	const notSelectedVisibleItems = toRaw(sortedWorkflowsSet.value).difference(selectedChanges.value);
+
+	return !Boolean(notSelectedVisibleItems.size);
+});
+
+const selectAllIndeterminate = computed(() => {
+	if (!selectedChanges.value.size) {
+		return false;
+	}
+
+	const selectedVisibleItems = toRaw(selectedChanges.value).intersection(sortedWorkflowsSet.value);
+
+	if (selectedVisibleItems.size === 0) {
+		return false;
+	}
+
+	return !selectAll.value;
+});
 
 function onToggleSelectAll() {
+	const selected = toRaw(selectedChanges.value);
 	if (selectAll.value) {
-		selectedChanges.value.clear();
+		selectedChanges.value = selected.difference(sortedWorkflowsSet.value);
 	} else {
-		selectedChanges.value = new Set(changes.value.workflows.map((file) => file.id));
+		selectedChanges.value = selected.union(sortedWorkflowsSet.value);
 	}
 }
 
@@ -289,7 +321,7 @@ const successNotificationMessage = () => {
 	}
 
 	return [
-		new Intl.ListFormat(i18n.locale, { style: 'long', type: 'conjunction' }).format(messages),
+		concatenateWithAnd(messages),
 		i18n.baseText('settings.sourceControl.modals.push.success.description'),
 	].join(' ');
 };
@@ -320,6 +352,8 @@ async function commitAndPush() {
 		loadingService.stopLoading();
 	}
 }
+
+const modalHeight = computed(() => (changes.value.workflows.length ? 'min(80vh, 850px)' : 'auto'));
 </script>
 
 <template>
@@ -327,161 +361,190 @@ async function commitAndPush() {
 		width="812px"
 		:event-bus="data.eventBus"
 		:name="SOURCE_CONTROL_PUSH_MODAL_KEY"
-		max-height="80%"
+		:height="modalHeight"
 		:custom-class="$style.sourceControlPush"
 	>
 		<template #header>
 			<N8nHeading tag="h1" size="xlarge">
 				{{ i18n.baseText('settings.sourceControl.modals.push.title') }}
 			</N8nHeading>
-			<div class="mt-l">
-				<N8nText tag="div">
-					{{ i18n.baseText('settings.sourceControl.modals.push.description') }}
-					<N8nLink :to="i18n.baseText('settings.sourceControl.docs.using.pushPull.url')">
-						{{ i18n.baseText('settings.sourceControl.modals.push.description.learnMore') }}
-					</N8nLink>
-				</N8nText>
 
-				<N8nNotice v-if="userNotices.length" class="mt-xs" :compact="false">
-					<ul class="ml-m">
-						<li v-for="notice in userNotices" :key="notice">{{ notice }}</li>
-					</ul>
-				</N8nNotice>
-			</div>
+			<div v-if="changes.workflows.length" :class="[$style.filtersRow]" class="mt-l">
+				<div :class="[$style.filters]">
+					<N8nInput
+						v-model="search"
+						data-test-id="source-control-push-search"
+						placeholder="Filter by title"
+						clearable
+						style="width: 234px"
+					>
+						<template #prefix>
+							<N8nIcon icon="search" />
+						</template>
+					</N8nInput>
+					<N8nPopover trigger="click" width="304" style="align-self: normal">
+						<template #reference>
+							<N8nButton
+								icon="filter"
+								type="tertiary"
+								style="height: 100%"
+								:active="Boolean(filterCount)"
+								data-test-id="source-control-filter-dropdown"
+							>
+								<N8nBadge v-if="filterCount" theme="primary" class="mr-4xs">
+									{{ filterCount }}
+								</N8nBadge>
+							</N8nButton>
+						</template>
+						<N8nInputLabel
+							:label="i18n.baseText('workflows.filters.status')"
+							:bold="false"
+							size="small"
+							color="text-base"
+							class="mb-3xs"
+						/>
+						<N8nSelect
+							v-model="filters.status"
+							data-test-id="source-control-status-filter"
+							clearable
+						>
+							<N8nOption
+								v-for="option in statusFilterOptions"
+								:key="option.label"
+								data-test-id="source-control-status-filter-option"
+								v-bind="option"
+							>
+							</N8nOption>
+						</N8nSelect>
+					</N8nPopover>
+				</div>
 
-			<div v-if="changes.workflows.length" :class="[$style.filers]" class="mt-l">
-				<N8nCheckbox
-					:class="$style.selectAll"
-					:indeterminate="selectAllIndeterminate"
-					:model-value="selectAll"
-					data-test-id="source-control-push-modal-toggle-all"
-					@update:model-value="onToggleSelectAll"
-				>
-					<N8nText bold tag="strong">
-						{{ i18n.baseText('settings.sourceControl.modals.push.workflowsToCommit') }}
+				<div>
+					<N8nText bold color="text-base" size="small">
+						{{ selectedChanges.size }} of {{ changes.workflows.length }}
 					</N8nText>
-					<N8nText tag="strong">
-						({{ selectedChanges.size }}/{{ sortedWorkflows.length }})
-					</N8nText>
-				</N8nCheckbox>
-				<N8nPopover trigger="click" width="304" style="align-self: normal">
-					<template #reference>
-						<N8nButton
-							icon="filter"
-							type="tertiary"
-							style="height: 100%"
-							:active="Boolean(filterCount)"
-							data-test-id="source-control-filter-dropdown"
-						>
-							<N8nBadge v-show="filterCount" theme="primary" class="mr-4xs">
-								{{ filterCount }}
-							</N8nBadge>
-							{{ i18n.baseText('forms.resourceFiltersDropdown.filters') }}
-						</N8nButton>
-					</template>
-					<N8nInputLabel
-						:label="i18n.baseText('workflows.filters.status')"
-						:bold="false"
-						size="small"
-						color="text-base"
-						class="mb-3xs"
-					/>
-					<N8nSelect v-model="filters.status" data-test-id="source-control-status-filter" clearable>
-						<N8nOption
-							v-for="option in statusFilterOptions"
-							:key="option.label"
-							data-test-id="source-control-status-filter-option"
-							v-bind="option"
-						>
-						</N8nOption>
-					</N8nSelect>
-				</N8nPopover>
-				<N8nInput
-					v-model="search"
-					data-test-id="source-control-push-search"
-					:placeholder="i18n.baseText('workflows.search.placeholder')"
-					clearable
-				>
-					<template #prefix>
-						<N8nIcon icon="search" />
-					</template>
-				</N8nInput>
+					<N8nText color="text-base" size="small"> workflows selected</N8nText>
+				</div>
 			</div>
 		</template>
 		<template #content>
-			<N8nInfoTip v-if="filtersApplied && !sortedWorkflows.length" :bold="false">
-				{{ i18n.baseText('workflows.filters.active') }}
-				<N8nLink size="small" data-test-id="source-control-filters-reset" @click="resetFilters">
-					{{ i18n.baseText('workflows.filters.active.reset') }}
-				</N8nLink>
-			</N8nInfoTip>
-			<RecycleScroller
-				v-if="sortedWorkflows.length"
-				:class="[$style.scroller]"
-				:items="sortedWorkflows"
-				:item-size="69"
-				key-field="id"
-			>
-				<template #default="{ item: file }">
+			<div :class="[$style.table]" v-if="changes.workflows.length">
+				<div :class="[$style.tableHeader]">
 					<N8nCheckbox
-						:class="['scopedListItem', $style.listItem]"
-						data-test-id="source-control-push-modal-file-checkbox"
-						:model-value="selectedChanges.has(file.id)"
-						@update:model-value="toggleSelected(file.id)"
+						:class="$style.selectAll"
+						:indeterminate="selectAllIndeterminate"
+						:model-value="selectAll"
+						data-test-id="source-control-push-modal-toggle-all"
+						@update:model-value="onToggleSelectAll"
 					>
-						<span>
-							<N8nText v-if="file.status === SOURCE_CONTROL_FILE_STATUS.deleted" color="text-light">
-								<span v-if="file.type === SOURCE_CONTROL_FILE_TYPE.workflow">
-									Deleted Workflow:
-								</span>
-								<span v-if="file.type === SOURCE_CONTROL_FILE_TYPE.credential">
-									Deleted Credential:
-								</span>
-								<strong>{{ file.name || file.id }}</strong>
-							</N8nText>
-							<N8nText v-else bold> {{ file.name }} </N8nText>
-							<N8nText v-if="file.updatedAt" tag="p" class="mt-0" color="text-light" size="small">
-								{{ renderUpdatedAt(file) }}
-							</N8nText>
-						</span>
-						<span :class="[$style.badges]">
-							<N8nBadge
-								v-if="changes.currentWorkflow && file.id === changes.currentWorkflow.id"
-								class="mr-2xs"
-							>
-								Current workflow
-							</N8nBadge>
-							<N8nBadge :theme="getStatusTheme(file.status)">
-								{{ getStatusText(file.status) }}
-							</N8nBadge>
-						</span>
+						<N8nText> Title </N8nText>
 					</N8nCheckbox>
-				</template>
-			</RecycleScroller>
+				</div>
+				<div style="flex: 1; overflow: hidden">
+					<N8nInfoTip v-if="filtersApplied && !sortedWorkflows.length" :bold="false">
+						{{ i18n.baseText('workflows.filters.active') }}
+						<N8nLink size="small" data-test-id="source-control-filters-reset" @click="resetFilters">
+							{{ i18n.baseText('workflows.filters.active.reset') }}
+						</N8nLink>
+					</N8nInfoTip>
+					<DynamicScroller
+						v-if="sortedWorkflows.length"
+						:class="[$style.scroller]"
+						:items="sortedWorkflows"
+						:min-item-size="58"
+						item-class="scrollerItem"
+					>
+						<template #default="{ item: file, active, index }">
+							<DynamicScrollerItem
+								:item="file"
+								:active="active"
+								:size-dependencies="[file.name, file.id]"
+								:data-index="index"
+							>
+								<N8nCheckbox
+									:class="[$style.listItem]"
+									data-test-id="source-control-push-modal-file-checkbox"
+									:model-value="selectedChanges.has(file.id)"
+									@update:model-value="toggleSelected(file.id)"
+								>
+									<span>
+										<N8nText
+											v-if="file.status === SOURCE_CONTROL_FILE_STATUS.deleted"
+											color="text-light"
+										>
+											<span v-if="file.type === SOURCE_CONTROL_FILE_TYPE.workflow">
+												Deleted Workflow:
+											</span>
+											<span v-if="file.type === SOURCE_CONTROL_FILE_TYPE.credential">
+												Deleted Credential:
+											</span>
+											<strong>{{ file.name || file.id }}</strong>
+										</N8nText>
+										<N8nText v-else tag="div" bold color="text-dark" :class="[$style.listItemName]">
+											{{ file.name }}
+										</N8nText>
+										<N8nText
+											v-if="file.updatedAt"
+											tag="p"
+											class="mt-0"
+											color="text-light"
+											size="small"
+										>
+											{{ renderUpdatedAt(file) }}
+										</N8nText>
+									</span>
+									<span :class="[$style.badges]">
+										<N8nBadge
+											v-if="changes.currentWorkflow && file.id === changes.currentWorkflow.id"
+											class="mr-2xs"
+										>
+											Current workflow
+										</N8nBadge>
+										<N8nBadge :theme="getStatusTheme(file.status)">
+											{{ getStatusText(file.status) }}
+										</N8nBadge>
+									</span>
+								</N8nCheckbox>
+							</DynamicScrollerItem>
+						</template>
+					</DynamicScroller>
+				</div>
+			</div>
 		</template>
 
 		<template #footer>
-			<N8nText bold tag="p" class="mb-2xs">
+			<N8nNotice v-if="userNotices.length" :compact="false" class="mt-0">
+				<N8nText bold size="medium">Changes to credentials, variables and tags </N8nText>
+				<br />
+				<template v-for="{ title, content } in userNotices" :key="title">
+					<N8nText bold size="small">{{ title }}</N8nText>
+					<N8nText size="small">: {{ content }}. </N8nText>
+				</template>
+			</N8nNotice>
+
+			<N8nText bold tag="p">
 				{{ i18n.baseText('settings.sourceControl.modals.push.commitMessage') }}
 			</N8nText>
-			<N8nInput
-				v-model="commitMessage"
-				data-test-id="source-control-push-modal-commit"
-				:placeholder="i18n.baseText('settings.sourceControl.modals.push.commitMessage.placeholder')"
-				@keydown.enter="onCommitKeyDownEnter"
-			/>
 
 			<div :class="$style.footer">
-				<N8nButton type="tertiary" class="mr-2xs" @click="close">
-					{{ i18n.baseText('settings.sourceControl.modals.push.buttons.cancel') }}
-				</N8nButton>
+				<N8nInput
+					v-model="commitMessage"
+					class="mr-2xs"
+					data-test-id="source-control-push-modal-commit"
+					:placeholder="
+						i18n.baseText('settings.sourceControl.modals.push.commitMessage.placeholder')
+					"
+					@keydown.enter="onCommitKeyDownEnter"
+				/>
 				<N8nButton
 					data-test-id="source-control-push-modal-submit"
 					type="primary"
 					:disabled="isSubmitDisabled"
+					size="large"
 					@click="commitAndPush"
 				>
 					{{ i18n.baseText('settings.sourceControl.modals.push.buttons.save') }}
+					{{ selectedChanges.size ? `(${selectedChanges.size})` : undefined }}
 				</N8nButton>
 			</div>
 		</template>
@@ -489,7 +552,14 @@ async function commitAndPush() {
 </template>
 
 <style module lang="scss">
-.filers {
+.filtersRow {
+	display: flex;
+	align-items: center;
+	gap: 8px;
+	justify-content: space-between;
+}
+
+.filters {
 	display: flex;
 	align-items: center;
 	gap: 8px;
@@ -501,18 +571,33 @@ async function commitAndPush() {
 }
 
 .scroller {
-	max-height: 380px;
+	max-height: 100%;
+	scrollbar-color: var(--color-foreground-base) transparent;
+	outline: var(--border-base);
+
+	:global(.scrollerItem) {
+		&:last-child {
+			.listItem {
+				border-bottom: 0;
+			}
+		}
+	}
 }
 
 .listItem {
 	align-items: center;
-	padding: var(--spacing-xs);
-	transition: border 0.3s ease;
-	border-radius: var(--border-radius-large);
-	border: var(--border-base);
+	padding: 10px 16px;
+	margin: 0;
+	border-bottom: var(--border-base);
 
-	&:hover {
-		border-color: var(--color-foreground-dark);
+	.listItemName {
+		line-clamp: 2;
+		-webkit-line-clamp: 2;
+		text-overflow: ellipsis;
+		overflow: hidden;
+		display: -webkit-box;
+		-webkit-box-orient: vertical;
+		word-wrap: break-word; /* Important for long words! */
 	}
 
 	:global(.el-checkbox__label) {
@@ -520,6 +605,7 @@ async function commitAndPush() {
 		width: 100%;
 		justify-content: space-between;
 		align-items: center;
+		gap: 30px;
 	}
 
 	:global(.el-checkbox__inner) {
@@ -535,12 +621,30 @@ async function commitAndPush() {
 	display: flex;
 	flex-direction: row;
 	justify-content: flex-end;
-	margin-top: 20px;
+	margin-top: 8px;
 }
 
 .sourceControlPush {
+	&:global(.el-dialog) {
+		margin: 0;
+	}
+
 	:global(.el-dialog__header) {
 		padding-bottom: var(--spacing-xs);
 	}
 }
+
+.table {
+	height: 100%;
+	overflow: hidden;
+	display: flex;
+	flex-direction: column;
+	border: var(--border-base);
+	border-radius: 8px;
+}
+
+.tableHeader {
+	border-bottom: var(--border-base);
+	padding: 10px 16px;
+}
 </style>