mirror of
https://github.com/usebruno/bruno.git
synced 2026-06-11 09:51:30 +00:00
fix(ui): prevent empty header row from persisting state and crashing CLI (#8167)
* fix: 3228 Empty header row persists in state, file, and crashes CLI * fix: refactor test steps for auto-append empty header row functionality * fix: update key column identification to use isKeyField property * fix: prevent duplicate empty rows in EditableTable and improve empty row detection * fix: update addMultipartFileToLastRow to target the last row correctly * addressed review comment
This commit is contained in:
@@ -168,6 +168,17 @@ const EditableTable = ({
|
||||
};
|
||||
}, [defaultRow, checkboxKey]);
|
||||
|
||||
const hasAnyValue = useCallback((row) => {
|
||||
for (const col of columns) {
|
||||
const val = col.getValue ? col.getValue(row) : row[col.key];
|
||||
const defaultVal = defaultRow[col.key];
|
||||
if (val && val !== defaultVal && (typeof val !== 'string' || val.trim() !== '')) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}, [columns, defaultRow]);
|
||||
|
||||
const rowsWithEmpty = useMemo(() => {
|
||||
if (!showAddRow) {
|
||||
return rows;
|
||||
@@ -177,16 +188,11 @@ const EditableTable = ({
|
||||
return [createEmptyRow()];
|
||||
}
|
||||
|
||||
const lastRow = rows[rows.length - 1];
|
||||
const keyColumn = columns.find((col) => col.isKeyField);
|
||||
|
||||
if (keyColumn) {
|
||||
const lastRowKeyValue = keyColumn.getValue ? keyColumn.getValue(lastRow) : lastRow[keyColumn.key];
|
||||
const isLastRowEmpty = !lastRowKeyValue || (typeof lastRowKeyValue === 'string' && lastRowKeyValue.trim() === '');
|
||||
|
||||
if (isLastRowEmpty) {
|
||||
return rows;
|
||||
}
|
||||
// If the last row is already empty (e.g. a stray empty row loaded from a
|
||||
// pre-existing file), don't append another one — otherwise the table would
|
||||
// render two empty rows at the bottom on the initial render.
|
||||
if (!hasAnyValue(rows[rows.length - 1])) {
|
||||
return rows;
|
||||
}
|
||||
|
||||
if (!emptyRowUidRef.current || rows.some((r) => r.uid === emptyRowUidRef.current)) {
|
||||
@@ -198,15 +204,11 @@ const EditableTable = ({
|
||||
[checkboxKey]: true,
|
||||
...defaultRow
|
||||
}];
|
||||
}, [rows, columns, defaultRow, checkboxKey, createEmptyRow, showAddRow]);
|
||||
}, [rows, columns, defaultRow, checkboxKey, createEmptyRow, hasAnyValue, showAddRow]);
|
||||
|
||||
const isEmptyRow = useCallback((row) => {
|
||||
const keyColumn = columns.find((col) => col.isKeyField);
|
||||
if (!keyColumn) return false;
|
||||
|
||||
const value = keyColumn.getValue ? keyColumn.getValue(row) : row[keyColumn.key];
|
||||
return !value || (typeof value === 'string' && value.trim() === '');
|
||||
}, [columns]);
|
||||
// A row is empty when none of its columns hold a value — the single source of
|
||||
// truth used everywhere (memo guard, persistence filter, last-row rendering).
|
||||
const isEmptyRow = useCallback((row) => !hasAnyValue(row), [hasAnyValue]);
|
||||
|
||||
const isLastEmptyRow = useCallback((row, index) => {
|
||||
if (!showAddRow) return false;
|
||||
@@ -227,50 +229,20 @@ const EditableTable = ({
|
||||
const rowIndex = rowsWithEmpty.findIndex((r) => r.uid === rowUid);
|
||||
if (rowIndex === -1) return;
|
||||
|
||||
const currentRow = rowsWithEmpty[rowIndex];
|
||||
const isLast = rowIndex === rowsWithEmpty.length - 1;
|
||||
const wasEmpty = isEmptyRow(currentRow);
|
||||
|
||||
const keyColumn = columns.find((col) => col.isKeyField);
|
||||
const isKeyFieldChange = keyColumn && keyColumn.key === key;
|
||||
|
||||
let updatedRows = rowsWithEmpty.map((row) => {
|
||||
const updatedRows = rowsWithEmpty.map((row) => {
|
||||
if (row.uid === rowUid) {
|
||||
return { ...row, [key]: value };
|
||||
}
|
||||
return row;
|
||||
});
|
||||
|
||||
// Only add a new empty row when the key field is filled
|
||||
if (showAddRow && isLast && wasEmpty && isKeyFieldChange && value && value.trim() !== '') {
|
||||
emptyRowUidRef.current = uuid();
|
||||
updatedRows.push({
|
||||
uid: emptyRowUidRef.current,
|
||||
[checkboxKey]: true,
|
||||
...defaultRow
|
||||
});
|
||||
}
|
||||
|
||||
const hasAnyValue = (row) => {
|
||||
for (const col of columns) {
|
||||
const val = col.getValue ? col.getValue(row) : row[col.key];
|
||||
const defaultVal = defaultRow[col.key];
|
||||
if (val && val !== defaultVal && (typeof val !== 'string' || val.trim() !== '')) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
};
|
||||
|
||||
const result = updatedRows.filter((row, i) => {
|
||||
if (showAddRow && i === updatedRows.length - 1) {
|
||||
return hasAnyValue(row);
|
||||
}
|
||||
return true;
|
||||
});
|
||||
// Remove any fully-empty rows from the persisted data. The trailing empty
|
||||
// "add row" is re-added by the rowsWithEmpty memo, so there's always
|
||||
// exactly one empty row at the bottom and never a stray empty row above it.
|
||||
const result = showAddRow ? updatedRows.filter(hasAnyValue) : updatedRows;
|
||||
|
||||
onChange(result);
|
||||
}, [rowsWithEmpty, columns, onChange, checkboxKey, defaultRow, isEmptyRow, showAddRow]);
|
||||
}, [rowsWithEmpty, hasAnyValue, onChange, showAddRow]);
|
||||
|
||||
const handleCheckboxChange = useCallback((rowUid, checked) => {
|
||||
handleValueChange(rowUid, checkboxKey, checked);
|
||||
|
||||
79
tests/request/headers/auto-append-empty-header-row.spec.ts
Normal file
79
tests/request/headers/auto-append-empty-header-row.spec.ts
Normal file
@@ -0,0 +1,79 @@
|
||||
import { test, expect } from '../../../playwright';
|
||||
import {
|
||||
buildCommonLocators,
|
||||
closeAllCollections,
|
||||
createCollection,
|
||||
createFolder,
|
||||
createRequest,
|
||||
selectRequestPaneTab
|
||||
} from '../../utils/page';
|
||||
|
||||
test.afterEach(async ({ page }) => {
|
||||
await closeAllCollections(page);
|
||||
});
|
||||
|
||||
test('Auto-appends an empty header row when either Name or Value is filled', async ({ page, createTmpDir }) => {
|
||||
const collectionName = 'auto-append-empty-header-row';
|
||||
const locators = buildCommonLocators(page);
|
||||
|
||||
await test.step('Create a collection', async () => {
|
||||
await createCollection(page, collectionName, await createTmpDir());
|
||||
});
|
||||
|
||||
await test.step('Create folder-1 inside the collection', async () => {
|
||||
await createFolder(page, 'folder-1', collectionName, true);
|
||||
await locators.sidebar.folder('folder-1').dblclick();
|
||||
});
|
||||
|
||||
await test.step('Create an HTTP request inside folder-1', async () => {
|
||||
await createRequest(page, 'request-1', 'folder-1', { url: 'https://example.com/api', inFolder: true });
|
||||
await selectRequestPaneTab(page, 'Headers');
|
||||
});
|
||||
|
||||
const headersTable = page.locator('table').first();
|
||||
const rows = headersTable.locator('tbody tr');
|
||||
|
||||
await test.step('Starts with a single empty header row', async () => {
|
||||
await expect(rows).toHaveCount(1);
|
||||
});
|
||||
|
||||
await test.step('Typing into the Name field appends a new empty row', async () => {
|
||||
const nameEditor = rows.first().getByTestId('column-name').locator('.CodeMirror');
|
||||
await nameEditor.click();
|
||||
await page.keyboard.type('Content-Type');
|
||||
|
||||
await expect(rows).toHaveCount(2);
|
||||
});
|
||||
|
||||
await test.step('Typing only into the Value field (Name left empty) also appends a new empty row', async () => {
|
||||
const valueEditor = rows.nth(1).getByTestId('column-value').locator('.CodeMirror');
|
||||
await valueEditor.click();
|
||||
await page.keyboard.type('application/json');
|
||||
|
||||
await expect(rows).toHaveCount(3);
|
||||
});
|
||||
|
||||
const selectAll = process.platform === 'darwin' ? 'Meta+a' : 'Control+a';
|
||||
|
||||
await test.step('Clearing the Name field empties that row and removes it', async () => {
|
||||
// Row 0 has only a Name ("Content-Type"); clearing it makes the row fully
|
||||
// empty, so it is dropped and the trailing empty row remains.
|
||||
const nameEditor = rows.first().getByTestId('column-name').locator('.CodeMirror');
|
||||
await nameEditor.click();
|
||||
await page.keyboard.press(selectAll);
|
||||
await page.keyboard.press('Backspace');
|
||||
|
||||
await expect(rows).toHaveCount(2);
|
||||
});
|
||||
|
||||
await test.step('Clearing the Value field empties that row, leaving only the empty add row', async () => {
|
||||
// The Value-only row is now first; clearing its Value makes it fully empty,
|
||||
// so it is dropped and only the single trailing empty row is left.
|
||||
const valueEditor = rows.first().getByTestId('column-value').locator('.CodeMirror');
|
||||
await valueEditor.click();
|
||||
await page.keyboard.press(selectAll);
|
||||
await page.keyboard.press('Backspace');
|
||||
|
||||
await expect(rows).toHaveCount(1);
|
||||
});
|
||||
});
|
||||
@@ -1120,13 +1120,17 @@ const addMultipartFileToLastRow = async (page: Page, electronApp: ElectronApplic
|
||||
await mockBrowseFiles(electronApp, [filePath]);
|
||||
|
||||
const table = buildCommonLocators(page).table('editable-table');
|
||||
const lastRow = table.allRows().last();
|
||||
// The last row is the empty "add" row. Capture its index now, because once
|
||||
// we set a file the table appends a new empty row — so `.last()` would jump
|
||||
// to that new row instead of staying on the one we just filled.
|
||||
const rowIndex = (await table.allRows().count()) - 1;
|
||||
const targetRow = table.allRows().nth(rowIndex);
|
||||
|
||||
await expect(lastRow.locator('.upload-btn')).toBeVisible();
|
||||
await lastRow.locator('.upload-btn').click();
|
||||
await expect(lastRow.locator('.file-value-cell')).toBeVisible();
|
||||
const inlineChip = lastRow.getByTestId('multipart-file-chip').filter({ hasText: path.basename(filePath) });
|
||||
const summary = lastRow.getByTestId('multipart-file-summary');
|
||||
await expect(targetRow.locator('.upload-btn')).toBeVisible();
|
||||
await targetRow.locator('.upload-btn').click();
|
||||
await expect(targetRow.locator('.file-value-cell')).toBeVisible();
|
||||
const inlineChip = targetRow.getByTestId('multipart-file-chip').filter({ hasText: path.basename(filePath) });
|
||||
const summary = targetRow.getByTestId('multipart-file-summary');
|
||||
await expect(inlineChip.or(summary)).toBeVisible();
|
||||
});
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user