fix(editor): Fix source control push modal checkboxes (#10910)

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

@@ -0,0 +1,150 @@
+import { within } from '@testing-library/dom';
+import userEvent from '@testing-library/user-event';
+import { useRoute } from 'vue-router';
+import { createComponentRenderer } from '@/__tests__/render';
+import SourceControlPushModal from '@/components/SourceControlPushModal.ee.vue';
+import { createTestingPinia } from '@pinia/testing';
+import { createEventBus } from 'n8n-design-system';
+import type { SourceControlAggregatedFile } from '@/Interface';
+
+const eventBus = createEventBus();
+
+vi.mock('vue-router', () => ({
+	useRoute: vi.fn().mockReturnValue({
+		params: vi.fn(),
+		fullPath: vi.fn(),
+	}),
+	RouterLink: vi.fn(),
+}));
+
+let route: ReturnType<typeof useRoute>;
+
+const renderModal = createComponentRenderer(SourceControlPushModal, {
+	global: {
+		stubs: {
+			Modal: {
+				template: `
+					<div>
+						<slot name="header" />
+						<slot name="title" />
+						<slot name="content" />
+						<slot name="footer" />
+					</div>
+				`,
+			},
+		},
+	},
+});
+
+describe('SourceControlPushModal', () => {
+	beforeEach(() => {
+		route = useRoute();
+	});
+
+	it('mounts', () => {
+		vi.spyOn(route, 'fullPath', 'get').mockReturnValue('');
+
+		const { getByTitle } = renderModal({
+			pinia: createTestingPinia(),
+			props: {
+				data: {
+					eventBus,
+					status: [],
+				},
+			},
+		});
+		expect(getByTitle('Commit and push changes')).toBeInTheDocument();
+	});
+
+	it('should toggle checkboxes', async () => {
+		const status: SourceControlAggregatedFile[] = [
+			{
+				id: 'gTbbBkkYTnNyX1jD',
+				name: 'My workflow 1',
+				type: 'workflow',
+				status: 'created',
+				location: 'local',
+				conflict: false,
+				file: '/home/user/.n8n/git/workflows/gTbbBkkYTnNyX1jD.json',
+				updatedAt: '2024-09-20T10:31:40.000Z',
+			},
+			{
+				id: 'JIGKevgZagmJAnM6',
+				name: 'My workflow 2',
+				type: 'workflow',
+				status: 'created',
+				location: 'local',
+				conflict: false,
+				file: '/home/user/.n8n/git/workflows/JIGKevgZagmJAnM6.json',
+				updatedAt: '2024-09-20T14:42:51.968Z',
+			},
+		];
+
+		vi.spyOn(route, 'fullPath', 'get').mockReturnValue('/home/workflows');
+
+		const { getByTestId, getAllByTestId } = renderModal({
+			pinia: createTestingPinia(),
+			props: {
+				data: {
+					eventBus,
+					status,
+				},
+			},
+		});
+
+		const files = getAllByTestId('source-control-push-modal-file-checkbox');
+		expect(files).toHaveLength(2);
+
+		await userEvent.click(files[0]);
+		expect(within(files[0]).getByRole('checkbox')).toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).not.toBeChecked();
+
+		await userEvent.click(within(files[0]).getByRole('checkbox'));
+		expect(within(files[0]).getByRole('checkbox')).not.toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).not.toBeChecked();
+
+		await userEvent.click(within(files[1]).getByRole('checkbox'));
+		expect(within(files[0]).getByRole('checkbox')).not.toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).toBeChecked();
+
+		await userEvent.click(files[1]);
+		expect(within(files[0]).getByRole('checkbox')).not.toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).not.toBeChecked();
+
+		await userEvent.click(within(files[0]).getByText('My workflow 2'));
+		expect(within(files[0]).getByRole('checkbox')).toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).not.toBeChecked();
+
+		await userEvent.click(within(files[1]).getByText('My workflow 1'));
+		expect(within(files[0]).getByRole('checkbox')).toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).toBeChecked();
+
+		await userEvent.click(within(files[1]).getByText('My workflow 1'));
+		expect(within(files[0]).getByRole('checkbox')).toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).not.toBeChecked();
+
+		await userEvent.click(getByTestId('source-control-push-modal-toggle-all'));
+		expect(within(files[0]).getByRole('checkbox')).toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).toBeChecked();
+
+		await userEvent.click(within(files[0]).getByText('My workflow 2'));
+		await userEvent.click(within(files[1]).getByText('My workflow 1'));
+		expect(within(files[0]).getByRole('checkbox')).not.toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).not.toBeChecked();
+		expect(
+			within(getByTestId('source-control-push-modal-toggle-all')).getByRole('checkbox'),
+		).not.toBeChecked();
+
+		await userEvent.click(within(files[0]).getByText('My workflow 2'));
+		await userEvent.click(within(files[1]).getByText('My workflow 1'));
+		expect(within(files[0]).getByRole('checkbox')).toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).toBeChecked();
+		expect(
+			within(getByTestId('source-control-push-modal-toggle-all')).getByRole('checkbox'),
+		).toBeChecked();
+
+		await userEvent.click(getByTestId('source-control-push-modal-toggle-all'));
+		expect(within(files[0]).getByRole('checkbox')).not.toBeChecked();
+		expect(within(files[1]).getByRole('checkbox')).not.toBeChecked();
+	});
+});

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

@@ -262,17 +262,18 @@ function getStatusText(file: SourceControlAggregatedFile): string {
 			<div :class="$style.container">
 				<div v-if="files.length > 0">
 					<div v-if="workflowFiles.length > 0">
-						<n8n-text>
+						<n8n-text tag="div" class="mb-l">
 							{{ i18n.baseText('settings.sourceControl.modals.push.description') }}
 							<n8n-link :to="i18n.baseText('settings.sourceControl.docs.using.pushPull.url')">
 								{{ i18n.baseText('settings.sourceControl.modals.push.description.learnMore') }}
 							</n8n-link>
 						</n8n-text>
 
-						<div class="mt-l mb-2xs">
 						<n8n-checkbox
+							:class="$style.selectAll"
 							:indeterminate="selectAllIndeterminate"
 							:model-value="selectAll"
+							data-test-id="source-control-push-modal-toggle-all"
 							@update:model-value="onToggleSelectAll"
 						>
 							<n8n-text bold tag="strong">
@@ -282,46 +283,45 @@ function getStatusText(file: SourceControlAggregatedFile): string {
 								({{ stagedWorkflowFiles.length }}/{{ workflowFiles.length }})
 							</n8n-text>
 						</n8n-checkbox>
-						</div>
+
-						<n8n-card
-							v-for="file in sortedFiles"
-							v-show="!defaultStagedFileTypes.includes(file.type)"
-							:key="file.file"
-							:class="$style.listItem"
-							@click="setStagedStatus(file, !staged[file.file])"
-						>
-							<div :class="$style.listItemBody">
 						<n8n-checkbox
+							v-for="file in sortedFiles"
+							:key="file.file"
+							:class="[
+								'scopedListItem',
+								$style.listItem,
+								{ [$style.hiddenListItem]: defaultStagedFileTypes.includes(file.type) },
+							]"
+							data-test-id="source-control-push-modal-file-checkbox"
 							:model-value="staged[file.file]"
-									:class="$style.listItemCheckbox"
 							@update:model-value="setStagedStatus(file, !staged[file.file])"
-								/>
+						>
-								<div>
+							<span>
 								<n8n-text v-if="file.status === 'deleted'" color="text-light">
 									<span v-if="file.type === 'workflow'"> Deleted Workflow: </span>
 									<span v-if="file.type === 'credential'"> Deleted Credential: </span>
 									<strong>{{ file.name || file.id }}</strong>
 								</n8n-text>
 								<n8n-text v-else bold> {{ file.name }} </n8n-text>
-									<div v-if="file.updatedAt">
+								<n8n-text
-										<n8n-text color="text-light" size="small">
+									v-if="file.updatedAt"
+									tag="p"
+									class="mt-0"
+									color="text-light"
+									size="small"
+								>
 									{{ renderUpdatedAt(file) }}
 								</n8n-text>
-									</div>
+							</span>
-								</div>
+							<span>
-								<div :class="$style.listItemStatus">
+								<n8n-badge v-if="workflowId === file.id && file.type === 'workflow'" class="mr-2xs">
-									<n8n-badge
-										v-if="workflowId === file.id && file.type === 'workflow'"
-										class="mr-2xs"
-									>
 									Current workflow
 								</n8n-badge>
 								<n8n-badge :theme="statusToBadgeThemeMap[file.status] || 'default'">
 									{{ getStatusText(file) }}
 								</n8n-badge>
-								</div>
+							</span>
-							</div>
+						</n8n-checkbox>
-						</n8n-card>
 					</div>
 					<n8n-notice v-else class="mt-0">
 						<i18n-t keypath="settings.sourceControl.modals.push.noWorkflowChanges">
@@ -380,11 +380,15 @@ function getStatusText(file: SourceControlAggregatedFile): string {
 }
 
 .listItem {
-	margin-top: var(--spacing-2xs);
+	display: flex;
-	margin-bottom: var(--spacing-2xs);
+	width: 100%;
+	align-items: center;
+	margin: var(--spacing-2xs) 0 var(--spacing-2xs);
+	padding: var(--spacing-xs);
 	cursor: pointer;
 	transition: border 0.3s ease;
-	padding: var(--spacing-xs);
+	border-radius: var(--border-radius-large);
+	border: var(--border-base);
 
 	&:hover {
 		border-color: var(--color-foreground-dark);
@@ -397,22 +401,16 @@ function getStatusText(file: SourceControlAggregatedFile): string {
 	&:last-child {
 		margin-bottom: 0;
 	}
+
+	&.hiddenListItem {
+		display: none !important;
+	}
 }
 
-.listItemBody {
+.selectAll {
-	display: flex;
+	float: left;
-	flex-direction: row;
+	clear: both;
-	align-items: center;
+	margin: 0 0 var(--spacing-2xs);
-}
-
-.listItemCheckbox {
-	display: inline-flex !important;
-	margin-bottom: 0 !important;
-	margin-right: var(--spacing-2xs) !important;
-}
-
-.listItemStatus {
-	margin-left: auto;
 }
 
 .footer {
@@ -421,3 +419,12 @@ function getStatusText(file: SourceControlAggregatedFile): string {
 	justify-content: flex-end;
 }
 </style>
+
+<style scoped lang="scss">
+.scopedListItem :deep(.el-checkbox__label) {
+	display: flex;
+	width: 100%;
+	justify-content: space-between;
+	align-items: center;
+}
+</style>