fix(editor): Enhance SourceControlPullModal with improved item structure and styling (#18049)

packages/frontend/editor-ui/src/components/SourceControlPullModal.ee.test.ts

@@ -6,20 +6,62 @@ import userEvent from '@testing-library/user-event';
 import { useSourceControlStore } from '@/stores/sourceControl.store';
 import { mockedStore } from '@/__tests__/utils';
 import { waitFor } from '@testing-library/dom';
+import { reactive } from 'vue';
 
 const eventBus = createEventBus();
 
+// Mock Vue Router to eliminate injection warnings
+const mockRoute = reactive({
+	params: {},
+	query: {},
+	path: '/',
+	name: 'TestRoute',
+});
+
+vi.mock('vue-router', () => ({
+	useRoute: () => mockRoute,
+	useRouter: () => ({
+		push: vi.fn(),
+		replace: vi.fn(),
+		go: vi.fn(),
+	}),
+	RouterLink: {
+		template: '<a><slot></slot></a>',
+		props: ['to', 'target'],
+	},
+}));
+
+// Mock the toast composable to prevent Element Plus DOM errors
+vi.mock('@/composables/useToast', () => ({
+	useToast: () => ({
+		showMessage: vi.fn(),
+		showError: vi.fn(),
+		showSuccess: vi.fn(),
+		clear: vi.fn(),
+	}),
+}));
+
 const DynamicScrollerStub = {
 	props: {
 		items: Array,
+		minItemSize: Number,
+		class: String,
+		style: [String, Object],
 	},
-	template: '<div><template v-for="item in items"><slot v-bind="{ item }"></slot></template></div>',
+	template:
+		'<div><template v-for="(item, index) in items" :key="index"><slot v-bind="{ item, index, active: false }"></slot></template></div>',
 	methods: {
 		scrollToItem: vi.fn(),
 	},
 };
 
 const DynamicScrollerItemStub = {
+	props: {
+		item: Object,
+		active: Boolean,
+		sizeDependencies: Array,
+		dataIndex: Number,
+	},
 	template: '<slot></slot>',
 };
 
@@ -38,6 +80,13 @@ const renderModal = createComponentRenderer(SourceControlPullModalEe, {
 					</div>
 				`,
 			},
+			EnvFeatureFlag: {
+				template: '<div><slot></slot></div>',
+			},
+			N8nIconButton: {
+				template: '<button><slot></slot></button>',
+				props: ['icon', 'type', 'class'],
+			},
 		},
 	},
 });
@@ -66,8 +115,11 @@ const sampleFiles = [
 ];
 
 describe('SourceControlPushModal', () => {
+	let sourceControlStore: ReturnType<typeof mockedStore<typeof useSourceControlStore>>;
+
 	beforeEach(() => {
 		createTestingPinia();
+		sourceControlStore = mockedStore(useSourceControlStore);
 	});
 
 	it('mounts', () => {
@@ -97,7 +149,6 @@ describe('SourceControlPushModal', () => {
 	});
 
 	it('should force pull', async () => {
-		const sourceControlStore = mockedStore(useSourceControlStore);
 		const { getByTestId } = renderModal({
 			props: {
 				data: {
@@ -111,4 +162,111 @@ describe('SourceControlPushModal', () => {
 
 		await waitFor(() => expect(sourceControlStore.pullWorkfolder).toHaveBeenCalledWith(true));
 	});
+
+	it('should render diff button with file-diff icon for workflow items', () => {
+		const workflowFile = {
+			...sampleFiles[0], // workflow file
+			type: 'workflow',
+		};
+
+		const { container } = renderModal({
+			props: {
+				data: {
+					eventBus,
+					status: [workflowFile],
+				},
+			},
+		});
+
+		// Check if a button with file-diff icon would be rendered (via class since icon is a prop)
+		const diffButton = container.querySelector('button');
+		expect(diffButton).toBeInTheDocument();
+	});
+
+	it('should not render diff button for non-workflow items', () => {
+		const credentialFile = {
+			...sampleFiles[1], // credential file
+			type: 'credential',
+		};
+
+		const { container } = renderModal({
+			props: {
+				data: {
+					eventBus,
+					status: [credentialFile],
+				},
+			},
+		});
+
+		// For credential files, there should be no additional buttons in the item actions
+		const itemActions = container.querySelector('[class*="itemActions"]');
+		const buttons = itemActions?.querySelectorAll('button');
+		expect(buttons).toHaveLength(0);
+	});
+
+	it('should render item names with ellipsis for long text', () => {
+		const longNameFile = {
+			...sampleFiles[0],
+			name: 'This is a very long workflow name that should be truncated with ellipsis to prevent wrapping to multiple lines',
+		};
+
+		const { container } = renderModal({
+			props: {
+				data: {
+					eventBus,
+					status: [longNameFile],
+				},
+			},
+		});
+
+		// Check if the itemName container exists and has the proper structure
+		const nameContainer = container.querySelector('[class*="itemName"]');
+		expect(nameContainer).toBeInTheDocument();
+
+		// Check if the RouterLink stub is rendered (since the name is rendered inside it)
+		const routerLink = nameContainer?.querySelector('a');
+		expect(routerLink).toBeInTheDocument();
+	});
+
+	it('should render badges and actions in separate container', () => {
+		const { getAllByTestId } = renderModal({
+			props: {
+				data: {
+					eventBus,
+					status: sampleFiles,
+				},
+			},
+		});
+
+		const listItems = getAllByTestId('pull-modal-item');
+
+		// Each list item should have the new structure with itemActions container
+		listItems.forEach((item) => {
+			const actionsContainer = item.querySelector('[class*="itemActions"]');
+			expect(actionsContainer).toBeInTheDocument();
+
+			// Badge should be inside actions container
+			const badge = actionsContainer?.querySelector('[class*="listBadge"]');
+			expect(badge).toBeInTheDocument();
+		});
+	});
+
+	it('should apply proper spacing and alignment styles', () => {
+		const { container, getAllByTestId } = renderModal({
+			props: {
+				data: {
+					eventBus,
+					status: sampleFiles,
+				},
+			},
+		});
+
+		// Check if the scroller container exists (using generic div since stub doesn't preserve CSS modules)
+		const scrollerContainer = container.querySelector('div');
+		expect(scrollerContainer).toBeInTheDocument();
+
+		// Check if list items exist and have proper structure
+		const listItems = getAllByTestId('pull-modal-item');
+		expect(listItems.length).toBeGreaterThan(0);
+	});
 });

packages/frontend/editor-ui/src/components/SourceControlPullModal.ee.vue

@@ -141,7 +141,7 @@ function openDiffModal(id: string) {
 					ref="scroller"
 					:items="files"
 					:min-item-size="47"
-					class="full-height scroller"
+					:class="$style.scroller"
 					style="max-height: 440px"
 				>
 					<template #default="{ item, index, active }">
@@ -160,6 +160,7 @@ function openDiffModal(id: string) {
 							:data-index="index"
 						>
 							<div :class="$style.listItem" data-test-id="pull-modal-item">
+								<div :class="$style.itemName">
 									<RouterLink
 										v-if="item.type === 'credential'"
 										target="_blank"
@@ -175,18 +176,22 @@ function openDiffModal(id: string) {
 										<N8nText>{{ item.name }}</N8nText>
 									</RouterLink>
 									<N8nText v-else>{{ item.name }}</N8nText>
+								</div>
+								<div :class="$style.itemActions">
 									<N8nBadge :theme="getStatusTheme(item.status)" :class="$style.listBadge">
 										{{ getStatusText(item.status) }}
 									</N8nBadge>
 									<EnvFeatureFlag name="SOURCE_CONTROL_WORKFLOW_DIFF">
 										<N8nIconButton
 											v-if="item.type === SOURCE_CONTROL_FILE_TYPE.workflow"
-										icon="git-branch"
+											icon="file-diff"
 											type="secondary"
+											:class="$style.diffButton"
 											@click="openDiffModal(item.id)"
 										/>
 									</EnvFeatureFlag>
 								</div>
+							</div>
 						</DynamicScrollerItem>
 					</template>
 				</DynamicScroller>
@@ -207,8 +212,13 @@ function openDiffModal(id: string) {
 </template>
 
 <style module lang="scss">
-.container > * {
+.container {
 	overflow-wrap: break-word;
+	padding-right: 8px;
+}
+
+.scroller {
+	margin-right: -8px;
 }
 
 .filesList {
@@ -225,17 +235,15 @@ function openDiffModal(id: string) {
 	padding-top: 16px;
 	padding-bottom: 12px;
 	height: 47px;
-}
+	margin-right: 8px;
-
-.listBadge {
-	margin-left: auto;
-	align-self: flex-start;
-	margin-top: 2px;
 }
 
 .listItem {
 	display: flex;
-	padding-bottom: 10px;
+	align-items: center;
+	padding-bottom: 8px;
+	margin-right: 8px;
+
 	&::before {
 		display: block;
 		content: '';
@@ -248,6 +256,37 @@ function openDiffModal(id: string) {
 	}
 }
 
+.itemName {
+	flex: 1;
+	min-width: 0;
+	margin-right: 8px;
+
+	a,
+	span {
+		white-space: nowrap;
+		overflow: hidden;
+		text-overflow: ellipsis;
+		display: block;
+	}
+}
+
+.itemActions {
+	display: flex;
+	align-items: center;
+	gap: 8px;
+	flex-shrink: 0;
+}
+
+.listBadge {
+	align-self: center;
+	white-space: nowrap;
+	height: 30px;
+}
+
+.diffButton {
+	flex-shrink: 0;
+}
+
 .footer {
 	display: flex;
 	flex-direction: row;