mirror of
https://github.com/usebruno/bruno.git
synced 2026-06-11 09:51:30 +00:00
feat: Stabilize variable info editor transition and popup lifecycle (#7912)
* feat: Stabilize variable info editor transition and popup lifecycle (#7458) (#7818) * fix: prevent var-info split second UI flicker. Co-authored-by: Mhammed Reda El Jirari <ridaeljirari@gmail.com> Co-authored-by: Fred Rukundo <dukefred9@gmail.com> Co-authored-by: El Mehdi Bennamrouche <bennamrouchex@gmail.com> Co-authored-by: Ahmed Bouregba <medex0606@gmail.com> * fix: variable-tooltip hover issues (#7907) --------- Co-authored-by: Avictos <125824783+avictos@users.noreply.github.com> Co-authored-by: Mhammed Reda El Jirari <ridaeljirari@gmail.com> Co-authored-by: Fred Rukundo <dukefred9@gmail.com> Co-authored-by: El Mehdi Bennamrouche <bennamrouchex@gmail.com> Co-authored-by: Ahmed Bouregba <medex0606@gmail.com> Co-authored-by: shubh-bruno <shubham@usebruno.com>
This commit is contained in:
Sid
GitHub
@@ -132,7 +132,7 @@ const containsSecretVariableReferences = (rawValue, collection, item) => {
|
||||
return false;
|
||||
};
|
||||
|
||||
const getCopyButton = (variableValue, onCopyCallback) => {
|
||||
const getCopyButton = (getVariableValue, onCopyCallback) => {
|
||||
const copyButton = document.createElement('button');
|
||||
|
||||
copyButton.className = 'copy-button';
|
||||
@@ -150,8 +150,11 @@ const getCopyButton = (variableValue, onCopyCallback) => {
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolve the latest value at click time so edits/saves are reflected.
|
||||
const valueToCopy = typeof getVariableValue === 'function' ? getVariableValue() : getVariableValue;
|
||||
|
||||
navigator.clipboard
|
||||
.writeText(variableValue)
|
||||
.writeText(valueToCopy ?? '')
|
||||
.then(() => {
|
||||
isCopied = true;
|
||||
copyButton.innerHTML = CHECKMARK_ICON_SVG_TEXT;
|
||||
@@ -415,6 +418,11 @@ export const renderVarInfo = (token, options) => {
|
||||
// Store original value for comparison and track editing state
|
||||
let originalValue = rawValue;
|
||||
let isEditing = false;
|
||||
// Latest resolved value and mask state used by the copy button, eye toggle, and
|
||||
// error-revert path. Updated after each successful save so subsequent redraws
|
||||
// reflect the saved state. `??` preserves falsy-but-valid values like 0 / false.
|
||||
let currentInterpolatedValue = variableValue ?? '';
|
||||
let currentShouldMaskValue = shouldMaskValue;
|
||||
|
||||
cmEditor.setOption('extraKeys', {
|
||||
'Enter': (cm) => {
|
||||
@@ -461,8 +469,8 @@ export const renderVarInfo = (token, options) => {
|
||||
// Update icon
|
||||
toggleButton.innerHTML = isRevealed ? EYE_OFF_ICON_SVG : EYE_ICON_SVG;
|
||||
|
||||
// Update display mode
|
||||
updateValueDisplay(valueDisplay, variableValue, shouldMaskValue, isMasked, isRevealed);
|
||||
// Update display mode using live state so post-save values/masking are reflected.
|
||||
updateValueDisplay(valueDisplay, currentInterpolatedValue, currentShouldMaskValue, isMasked, isRevealed);
|
||||
|
||||
// Update editor mode
|
||||
if (maskedEditor) {
|
||||
@@ -480,8 +488,9 @@ export const renderVarInfo = (token, options) => {
|
||||
iconsContainer.appendChild(toggleButton);
|
||||
}
|
||||
|
||||
// Copy button (copy actual value, not masked)
|
||||
const copyButton = getCopyButton(variableValue || '', () => {
|
||||
// Copy button (copy actual value, not masked). Uses a getter so it always
|
||||
// reflects the latest saved value, not the value captured at popup creation.
|
||||
const copyButton = getCopyButton(() => currentInterpolatedValue, () => {
|
||||
// Refocus the editor if it's currently in edit mode
|
||||
if (isEditing) {
|
||||
setTimeout(() => {
|
||||
@@ -500,23 +509,30 @@ export const renderVarInfo = (token, options) => {
|
||||
if (isEditing) return;
|
||||
|
||||
isEditing = true;
|
||||
valueDisplay.style.display = 'none';
|
||||
|
||||
// Stage editor off-visual first to avoid a visible resize/text flash.
|
||||
editorContainer.style.display = 'block';
|
||||
editorContainer.style.visibility = 'hidden';
|
||||
|
||||
// Focus the editor and ensure proper sizing
|
||||
setTimeout(() => {
|
||||
requestAnimationFrame(() => {
|
||||
cmEditor.refresh();
|
||||
|
||||
// Adjust height based on content before revealing editor
|
||||
const sizer = cmEditor.getWrapperElement().querySelector('.CodeMirror-sizer');
|
||||
const contentHeight = sizer ? sizer.clientHeight : cmEditor.getScrollInfo().height;
|
||||
editorContainer.style.height = `${calculateEditorHeight(contentHeight)}rem`;
|
||||
|
||||
// Swap display only after editor layout is ready
|
||||
valueDisplay.style.display = 'none';
|
||||
editorContainer.style.visibility = 'visible';
|
||||
cmEditor.focus();
|
||||
|
||||
// Set cursor to end of content
|
||||
const lineCount = cmEditor.lineCount();
|
||||
const lastLine = cmEditor.getLine(lineCount - 1);
|
||||
cmEditor.setCursor(lineCount - 1, lastLine ? lastLine.length : 0);
|
||||
|
||||
// Adjust height based on content
|
||||
const contentHeight = cmEditor.getScrollInfo().height;
|
||||
editorContainer.style.height = `${calculateEditorHeight(contentHeight)}rem`;
|
||||
}, 0);
|
||||
});
|
||||
});
|
||||
|
||||
// Save on blur and return to display mode
|
||||
@@ -525,6 +541,7 @@ export const renderVarInfo = (token, options) => {
|
||||
|
||||
// Switch back to display mode
|
||||
editorContainer.style.display = 'none';
|
||||
editorContainer.style.visibility = 'visible';
|
||||
editorContainer.style.height = `${EDITOR_MIN_HEIGHT}rem`; // Reset to minimum height
|
||||
valueDisplay.style.display = 'block';
|
||||
isEditing = false;
|
||||
@@ -547,18 +564,22 @@ export const renderVarInfo = (token, options) => {
|
||||
}
|
||||
}
|
||||
|
||||
// Re-interpolate the new value to show the resolved value in display
|
||||
// Re-interpolate the new value to show the resolved value in display.
|
||||
// Use `??` so falsy-but-valid values (0 / false / '') survive the assignment.
|
||||
const interpolatedValue = interpolate(newValue, allVariables);
|
||||
// Check if the NEW value contains secret references
|
||||
currentInterpolatedValue = interpolatedValue ?? '';
|
||||
// Check if the NEW value contains secret references and update live mask state
|
||||
const newHasSecretRefs = containsSecretVariableReferences(newValue, collection, item);
|
||||
const newShouldMask = isSecret || newHasSecretRefs;
|
||||
updateValueDisplay(valueDisplay, interpolatedValue, newShouldMask, isMasked, isRevealed);
|
||||
currentShouldMaskValue = isSecret || newHasSecretRefs;
|
||||
updateValueDisplay(valueDisplay, currentInterpolatedValue, currentShouldMaskValue, isMasked, isRevealed);
|
||||
})
|
||||
.catch((err) => {
|
||||
console.error('Failed to update variable:', err);
|
||||
// Revert on error
|
||||
// Revert on error to the last good state — currentInterpolatedValue and
|
||||
// currentShouldMaskValue still hold pre-attempt values since the success
|
||||
// block above never ran.
|
||||
cmEditor.setValue(originalValue);
|
||||
updateValueDisplay(valueDisplay, variableValue, shouldMaskValue, isMasked, isRevealed);
|
||||
updateValueDisplay(valueDisplay, currentInterpolatedValue, currentShouldMaskValue, isMasked, isRevealed);
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -810,8 +831,10 @@ if (!SERVER_RENDERED) {
|
||||
}
|
||||
|
||||
function showPopup(cm, box, brunoVarInfo) {
|
||||
// If there's already an active popup, remove it first
|
||||
if (activePopup && activePopup.parentNode) {
|
||||
// If there's already an active popup, hide it first to ensure listeners are cleaned up
|
||||
if (activePopup && typeof activePopup._hidePopup === 'function') {
|
||||
activePopup._hidePopup({ immediate: true });
|
||||
} else if (activePopup && activePopup.parentNode) {
|
||||
activePopup.parentNode.removeChild(activePopup);
|
||||
activePopup = null;
|
||||
}
|
||||
@@ -865,20 +888,49 @@ if (!SERVER_RENDERED) {
|
||||
popup.style.left = `${leftPos / 16}rem`;
|
||||
|
||||
let popupTimeout;
|
||||
let isPinned = false;
|
||||
let isHidden = false;
|
||||
|
||||
const onMouseOverPopup = function () {
|
||||
clearTimeout(popupTimeout);
|
||||
};
|
||||
|
||||
const onMouseOut = function () {
|
||||
if (isPinned) {
|
||||
return;
|
||||
}
|
||||
clearTimeout(popupTimeout);
|
||||
popupTimeout = setTimeout(hidePopup, 500);
|
||||
};
|
||||
|
||||
const hidePopup = function () {
|
||||
const onPopupClick = function (e) {
|
||||
if (!popup.contains(e.target)) {
|
||||
return;
|
||||
}
|
||||
isPinned = true;
|
||||
clearTimeout(popupTimeout);
|
||||
};
|
||||
|
||||
const onDocumentClick = function (e) {
|
||||
if (!popup.contains(e.target)) {
|
||||
isPinned = false;
|
||||
hidePopup();
|
||||
}
|
||||
};
|
||||
|
||||
const hidePopup = function (options = {}) {
|
||||
if (isHidden) {
|
||||
return;
|
||||
}
|
||||
isHidden = true;
|
||||
|
||||
const { immediate = false } = options;
|
||||
clearTimeout(popupTimeout);
|
||||
CodeMirror.off(popup, 'mouseover', onMouseOverPopup);
|
||||
CodeMirror.off(popup, 'mouseout', onMouseOut);
|
||||
CodeMirror.off(popup, 'click', onPopupClick);
|
||||
CodeMirror.off(cm.getWrapperElement(), 'mouseout', onMouseOut);
|
||||
CodeMirror.off(document, 'click', onDocumentClick);
|
||||
CodeMirror.off(cm, 'change', onEditorChange);
|
||||
|
||||
// Cleanup CodeMirror and MaskedEditor instances
|
||||
@@ -908,6 +960,13 @@ if (!SERVER_RENDERED) {
|
||||
activePopup = null;
|
||||
}
|
||||
|
||||
if (immediate) {
|
||||
if (popup.parentNode) {
|
||||
popup.parentNode.removeChild(popup);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (popup.style.opacity) {
|
||||
popup.style.opacity = 0;
|
||||
setTimeout(function () {
|
||||
@@ -922,12 +981,19 @@ if (!SERVER_RENDERED) {
|
||||
|
||||
// Hide popup when user types in the main editor
|
||||
const onEditorChange = function () {
|
||||
if (!isPinned) {
|
||||
hidePopup();
|
||||
}
|
||||
};
|
||||
|
||||
// Allow replacing existing popup with full cleanup
|
||||
popup._hidePopup = hidePopup;
|
||||
|
||||
CodeMirror.on(popup, 'mouseover', onMouseOverPopup);
|
||||
CodeMirror.on(popup, 'mouseout', onMouseOut);
|
||||
CodeMirror.on(popup, 'click', onPopupClick);
|
||||
CodeMirror.on(cm.getWrapperElement(), 'mouseout', onMouseOut);
|
||||
CodeMirror.on(document, 'click', onDocumentClick);
|
||||
CodeMirror.on(cm, 'change', onEditorChange);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -218,11 +218,7 @@ test.describe('Variable Tooltip', () => {
|
||||
// Click copy button
|
||||
await copyButton.click();
|
||||
|
||||
// Should show success state (checkmark)
|
||||
await expect(copyButton.locator('svg polyline')).toBeVisible({ timeout: 1000 });
|
||||
|
||||
// Wait for it to revert back to copy icon
|
||||
await expect(copyButton.locator('svg rect')).toBeVisible();
|
||||
await expect.poll(() => page.evaluate(() => navigator.clipboard.readText()), { timeout: 1000 }).toBe('https://api.example.com/users/posts');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -431,4 +427,211 @@ test.describe('Variable Tooltip', () => {
|
||||
await expect(tooltip.locator('.var-value-editable-display')).not.toBeVisible();
|
||||
});
|
||||
});
|
||||
|
||||
test('should keep tooltip open while editing when mouse leaves popup area', async ({ page, createTmpDir }) => {
|
||||
const collectionName = 'tooltip-pin-test';
|
||||
|
||||
await test.step('Setup collection, environment variable, and request', async () => {
|
||||
await createCollection(page, collectionName, await createTmpDir('tooltip-pin-collection'));
|
||||
|
||||
await createEnvironment(page, 'Pin Env', 'collection');
|
||||
await addEnvironmentVariables(page, [{ name: 'pinVar', value: 'pin-value' }]);
|
||||
await saveEnvironment(page);
|
||||
await closeEnvironmentPanel(page);
|
||||
|
||||
await createRequest(page, 'Pin Test Request', collectionName);
|
||||
await page.locator('.collection-item-name').filter({ hasText: 'Pin Test Request' }).click();
|
||||
const urlEditor = page.locator('#request-url .CodeMirror');
|
||||
await urlEditor.click();
|
||||
await page.keyboard.type('https://api.example.com?key={{pinVar}}');
|
||||
await page.keyboard.press(saveShortcut);
|
||||
});
|
||||
|
||||
await test.step('Tooltip stays open and accepts input while mouse is outside popup', async () => {
|
||||
await page.mouse.move(0, 0);
|
||||
|
||||
const urlEditor = page.locator('#request-url .CodeMirror');
|
||||
const pinVar = urlEditor.locator('.cm-variable-valid').filter({ hasText: 'pinVar' }).first();
|
||||
await pinVar.hover();
|
||||
|
||||
const tooltip = page.locator('.CodeMirror-brunoVarInfo').first();
|
||||
await expect(tooltip).toBeVisible();
|
||||
|
||||
// Click value display to enter edit mode (this also pins the popup)
|
||||
const valueDisplay = tooltip.locator('.var-value-editable-display');
|
||||
await valueDisplay.click();
|
||||
|
||||
const editor = tooltip.locator('.var-value-editor .CodeMirror');
|
||||
await expect(editor).toBeVisible();
|
||||
|
||||
// Move mouse far outside the popup
|
||||
await page.mouse.move(0, 0);
|
||||
|
||||
// Type with a per-keystroke delay so the typing window spans past the internal
|
||||
// 500ms hide timer. If the popup were not pinned, it would hide mid-typing and
|
||||
// the keystrokes would never reach the editor — the assertion below would fail.
|
||||
// This validates pinning via real editor activity instead of a fixed sleep.
|
||||
await page.keyboard.press('End');
|
||||
await page.keyboard.type('-still-editable-after-mouse-left', { delay: 25 });
|
||||
|
||||
await expect(editor.locator('.CodeMirror-line')).toContainText(
|
||||
'pin-value-still-editable-after-mouse-left'
|
||||
);
|
||||
await expect(tooltip).toBeVisible();
|
||||
});
|
||||
});
|
||||
|
||||
test('should persist subsequent edits while popup stays open', async ({ page, createTmpDir }) => {
|
||||
const collectionName = 'tooltip-subsequent-edit-test';
|
||||
|
||||
await test.step('Setup collection, environment variable, and request', async () => {
|
||||
await createCollection(page, collectionName, await createTmpDir('tooltip-subsequent-collection'));
|
||||
|
||||
await createEnvironment(page, 'Edit Env', 'collection');
|
||||
await addEnvironmentVariables(page, [{ name: 'editVar', value: 'initial' }]);
|
||||
await saveEnvironment(page);
|
||||
await closeEnvironmentPanel(page);
|
||||
|
||||
await createRequest(page, 'Edit Test Request', collectionName);
|
||||
await page.locator('.collection-item-name').filter({ hasText: 'Edit Test Request' }).click();
|
||||
const urlEditor = page.locator('#request-url .CodeMirror');
|
||||
await urlEditor.click();
|
||||
await page.keyboard.type('https://api.example.com?key={{editVar}}');
|
||||
await page.keyboard.press(saveShortcut);
|
||||
});
|
||||
|
||||
await test.step('First edit saves via Enter and keeps popup open', async () => {
|
||||
await page.mouse.move(0, 0);
|
||||
|
||||
const urlEditor = page.locator('#request-url .CodeMirror');
|
||||
const editVar = urlEditor.locator('.cm-variable-valid').filter({ hasText: 'editVar' }).first();
|
||||
await editVar.hover();
|
||||
|
||||
const tooltip = page.locator('.CodeMirror-brunoVarInfo').first();
|
||||
await expect(tooltip).toBeVisible();
|
||||
|
||||
const valueDisplay = tooltip.locator('.var-value-editable-display');
|
||||
await expect(valueDisplay).toContainText('initial');
|
||||
|
||||
await valueDisplay.click();
|
||||
|
||||
const editor = tooltip.locator('.var-value-editor .CodeMirror');
|
||||
await expect(editor).toBeVisible();
|
||||
|
||||
await page.keyboard.press('End');
|
||||
await page.keyboard.type('-one');
|
||||
|
||||
// Pressing Enter saves and keeps the popup open (does not click outside)
|
||||
await page.keyboard.press('Enter');
|
||||
|
||||
// Display reflects the saved value, and tooltip is still visible
|
||||
await expect(valueDisplay).toContainText('initial-one');
|
||||
await expect(tooltip).toBeVisible();
|
||||
});
|
||||
|
||||
await test.step('Second edit on the same popup also saves', async () => {
|
||||
const tooltip = page.locator('.CodeMirror-brunoVarInfo').first();
|
||||
await expect(tooltip).toBeVisible();
|
||||
|
||||
const valueDisplay = tooltip.locator('.var-value-editable-display');
|
||||
await valueDisplay.click();
|
||||
|
||||
const editor = tooltip.locator('.var-value-editor .CodeMirror');
|
||||
await expect(editor).toBeVisible();
|
||||
|
||||
await page.keyboard.press('End');
|
||||
await page.keyboard.type('-two');
|
||||
|
||||
await page.keyboard.press('Enter');
|
||||
|
||||
await expect(valueDisplay).toContainText('initial-one-two');
|
||||
});
|
||||
|
||||
await test.step('Reopen tooltip and verify the second edit persisted', async () => {
|
||||
// Close the existing tooltip with an outside click, then re-hover to get a fresh one
|
||||
await page.locator('body').click();
|
||||
const tooltip = page.locator('.CodeMirror-brunoVarInfo').first();
|
||||
await expect(tooltip).not.toBeVisible();
|
||||
|
||||
await page.mouse.move(0, 0);
|
||||
|
||||
const urlEditor = page.locator('#request-url .CodeMirror');
|
||||
const editVar = urlEditor.locator('.cm-variable-valid').filter({ hasText: 'editVar' }).first();
|
||||
await editVar.hover();
|
||||
|
||||
const newTooltip = page.locator('.CodeMirror-brunoVarInfo').first();
|
||||
await expect(newTooltip).toBeVisible();
|
||||
await expect(newTooltip.locator('.var-value-editable-display')).toContainText('initial-one-two');
|
||||
});
|
||||
});
|
||||
|
||||
test('should copy latest value after editing within the same tooltip', async ({ page, createTmpDir }) => {
|
||||
const collectionName = 'tooltip-copy-latest-test';
|
||||
|
||||
await test.step('Setup collection, environment variable, and request', async () => {
|
||||
await createCollection(page, collectionName, await createTmpDir('tooltip-copy-latest-collection'));
|
||||
|
||||
await createEnvironment(page, 'Copy Env', 'collection');
|
||||
await addEnvironmentVariables(page, [{ name: 'copyVar', value: 'original-copy' }]);
|
||||
await saveEnvironment(page);
|
||||
await closeEnvironmentPanel(page);
|
||||
|
||||
await createRequest(page, 'Copy Test Request', collectionName);
|
||||
await page.locator('.collection-item-name').filter({ hasText: 'Copy Test Request' }).click();
|
||||
const urlEditor = page.locator('#request-url .CodeMirror');
|
||||
await urlEditor.click();
|
||||
await page.keyboard.type('https://api.example.com?key={{copyVar}}');
|
||||
await page.keyboard.press(saveShortcut);
|
||||
});
|
||||
|
||||
await test.step('Copy button copies the initial value', async () => {
|
||||
await page.mouse.move(0, 0);
|
||||
|
||||
const urlEditor = page.locator('#request-url .CodeMirror');
|
||||
const copyVar = urlEditor.locator('.cm-variable-valid').filter({ hasText: 'copyVar' }).first();
|
||||
await copyVar.hover();
|
||||
|
||||
const tooltip = page.locator('.CodeMirror-brunoVarInfo').first();
|
||||
await expect(tooltip).toBeVisible();
|
||||
|
||||
const copyButton = tooltip.locator('.copy-button');
|
||||
await copyButton.click();
|
||||
|
||||
// Success state confirms writeText resolved before we read the clipboard
|
||||
await expect(copyButton.locator('svg polyline')).toBeVisible({ timeout: 1000 });
|
||||
|
||||
const initialClipboard = await page.evaluate(() => navigator.clipboard.readText());
|
||||
expect(initialClipboard).toBe('original-copy');
|
||||
|
||||
// Wait for the icon to revert so the next click is allowed
|
||||
await expect(copyButton.locator('svg rect')).toBeVisible();
|
||||
});
|
||||
|
||||
await test.step('Edit value, save with Enter, then copy without re-hovering', async () => {
|
||||
const tooltip = page.locator('.CodeMirror-brunoVarInfo').first();
|
||||
await expect(tooltip).toBeVisible();
|
||||
|
||||
const valueDisplay = tooltip.locator('.var-value-editable-display');
|
||||
await valueDisplay.click();
|
||||
|
||||
const editor = tooltip.locator('.var-value-editor .CodeMirror');
|
||||
await expect(editor).toBeVisible();
|
||||
|
||||
await page.keyboard.press('End');
|
||||
await page.keyboard.type('-edited');
|
||||
|
||||
await page.keyboard.press('Enter');
|
||||
|
||||
// Wait for the display to reflect the saved value before clicking copy
|
||||
await expect(valueDisplay).toContainText('original-copy-edited');
|
||||
|
||||
const copyButton = tooltip.locator('.copy-button');
|
||||
await copyButton.click();
|
||||
|
||||
await expect(copyButton.locator('svg polyline')).toBeVisible({ timeout: 1000 });
|
||||
|
||||
const updatedClipboard = await page.evaluate(() => navigator.clipboard.readText());
|
||||
expect(updatedClipboard).toBe('original-copy-edited');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user