mirror of
https://github.com/hyperdxio/hyperdx
synced 2026-04-21 13:37:15 +00:00
fix: Properly enable line wrap behavior in JSON viewer by default (#1998)
Resolves HDX-3845 | New | Old | |--|--| | <img width="474" height="732" alt="image" src="https://github.com/user-attachments/assets/a3909da5-d03e-4f19-9916-d5002acc4083" /> | <img width="576" height="226" alt="image" src="https://github.com/user-attachments/assets/22e67a65-cac8-4b44-a1da-9a1b40bee809" /> |
This commit is contained in:
parent
4e54d85061
commit
e6a0455aa5
6 changed files with 415 additions and 10 deletions
5
.changeset/cold-eyes-marry.md
Normal file
5
.changeset/cold-eyes-marry.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@hyperdx/app": patch
|
||||
---
|
||||
|
||||
fix: Properly enable line wrap behavior in JSON viewer by default
|
||||
|
|
@ -110,23 +110,124 @@ function filterBlankValuesRecursively(value: any): any {
|
|||
return value;
|
||||
}
|
||||
|
||||
const viewerOptionsAtom = atomWithStorage('hdx_json_viewer_options', {
|
||||
type ViewerOptions = {
|
||||
normallyExpanded: boolean;
|
||||
whiteSpace?: 'pre' | 'pre-wrap';
|
||||
tabulate: boolean;
|
||||
filterBlanks: boolean;
|
||||
};
|
||||
|
||||
const VIEWER_OPTIONS_KEY = 'hdx_json_viewer_options';
|
||||
|
||||
const DEFAULT_VIEWER_OPTIONS: ViewerOptions = {
|
||||
normallyExpanded: true,
|
||||
lineWrap: true,
|
||||
whiteSpace: 'pre-wrap',
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
});
|
||||
};
|
||||
|
||||
/**
|
||||
* Migrates old `lineWrap` boolean to `whiteSpace` enum.
|
||||
*
|
||||
* Old behavior was inverted:
|
||||
* lineWrap: true → white-space: pre (no wrapping) — was the default
|
||||
* lineWrap: false → word-break: break-all (wrapping, but collapsed whitespace)
|
||||
*
|
||||
* New behavior:
|
||||
* whiteSpace: 'pre' → preserve formatting, no wrapping
|
||||
* whiteSpace: 'pre-wrap' → preserve formatting + wrap long lines
|
||||
* whiteSpace: undefined → use default ('pre-wrap'), or future team default
|
||||
*/
|
||||
/** @internal Exported for testing only */
|
||||
export function migrateViewerOptions(
|
||||
stored: string | null,
|
||||
): ViewerOptions | null {
|
||||
if (!stored) return null;
|
||||
try {
|
||||
const parsed = JSON.parse(stored);
|
||||
if (typeof parsed !== 'object' || parsed === null) return null;
|
||||
|
||||
if ('lineWrap' in parsed) {
|
||||
const { lineWrap, ...rest } = parsed;
|
||||
const migrated: ViewerOptions = {
|
||||
...DEFAULT_VIEWER_OPTIONS,
|
||||
...rest,
|
||||
// Old lineWrap: true meant no-wrap (was default) → undefined (inherit default)
|
||||
// Old lineWrap: false meant user wanted wrapping → 'pre-wrap'
|
||||
whiteSpace: lineWrap === false ? 'pre-wrap' : undefined,
|
||||
};
|
||||
try {
|
||||
if (typeof window !== 'undefined') {
|
||||
localStorage.setItem(VIEWER_OPTIONS_KEY, JSON.stringify(migrated));
|
||||
}
|
||||
} catch {
|
||||
// Ignore localStorage errors
|
||||
}
|
||||
return migrated;
|
||||
}
|
||||
|
||||
return parsed as ViewerOptions;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// Custom storage adapter to migrate old `lineWrap` boolean to `whiteSpace` enum
|
||||
// on first read, before React renders (avoids flash of wrong state).
|
||||
const viewerOptionsStorage = {
|
||||
getItem: (key: string, initialValue: ViewerOptions): ViewerOptions => {
|
||||
if (typeof window === 'undefined') return initialValue;
|
||||
try {
|
||||
const stored = localStorage.getItem(key);
|
||||
return migrateViewerOptions(stored) ?? initialValue;
|
||||
} catch {
|
||||
return initialValue;
|
||||
}
|
||||
},
|
||||
setItem: (key: string, value: ViewerOptions): void => {
|
||||
try {
|
||||
if (typeof window !== 'undefined') {
|
||||
localStorage.setItem(key, JSON.stringify(value));
|
||||
}
|
||||
} catch {
|
||||
// Ignore localStorage errors
|
||||
}
|
||||
},
|
||||
removeItem: (key: string): void => {
|
||||
try {
|
||||
if (typeof window !== 'undefined') {
|
||||
localStorage.removeItem(key);
|
||||
}
|
||||
} catch {
|
||||
// Ignore localStorage errors
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
const viewerOptionsAtom = atomWithStorage<ViewerOptions>(
|
||||
VIEWER_OPTIONS_KEY,
|
||||
DEFAULT_VIEWER_OPTIONS,
|
||||
viewerOptionsStorage,
|
||||
);
|
||||
|
||||
function HyperJsonMenu() {
|
||||
const [jsonOptions, setJsonOptions] = useAtom(viewerOptionsAtom);
|
||||
const effectiveWhiteSpace = jsonOptions.whiteSpace ?? 'pre-wrap';
|
||||
|
||||
return (
|
||||
<Group>
|
||||
<UnstyledButton
|
||||
color="gray"
|
||||
data-testid="json-viewer-wrap-toggle"
|
||||
onClick={() =>
|
||||
setJsonOptions({ ...jsonOptions, lineWrap: !jsonOptions.lineWrap })
|
||||
setJsonOptions({
|
||||
...jsonOptions,
|
||||
whiteSpace: effectiveWhiteSpace === 'pre-wrap' ? 'pre' : 'pre-wrap',
|
||||
})
|
||||
}
|
||||
style={{
|
||||
opacity: effectiveWhiteSpace === 'pre-wrap' ? 1 : 0.5,
|
||||
}}
|
||||
>
|
||||
<IconTextWrap size={14} />
|
||||
</UnstyledButton>
|
||||
|
|
|
|||
|
|
@ -14,10 +14,10 @@
|
|||
}
|
||||
}
|
||||
|
||||
.withLineWrap {
|
||||
.withPreWrap {
|
||||
.valueContainer {
|
||||
.string {
|
||||
white-space: pre;
|
||||
white-space: pre-wrap;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -198,7 +198,8 @@
|
|||
|
||||
.string {
|
||||
color: var(--color-json-string);
|
||||
word-break: break-all;
|
||||
white-space: pre;
|
||||
overflow-wrap: break-word;
|
||||
|
||||
&::before,
|
||||
&::after {
|
||||
|
|
|
|||
|
|
@ -391,7 +391,7 @@ type HyperJsonProps = {
|
|||
data: object;
|
||||
normallyExpanded?: boolean;
|
||||
tabulate?: boolean;
|
||||
lineWrap?: boolean;
|
||||
whiteSpace?: 'pre' | 'pre-wrap';
|
||||
getLineActions?: GetLineActions;
|
||||
};
|
||||
|
||||
|
|
@ -399,7 +399,7 @@ const HyperJson = ({
|
|||
data,
|
||||
normallyExpanded = false,
|
||||
tabulate = false,
|
||||
lineWrap,
|
||||
whiteSpace = 'pre-wrap',
|
||||
getLineActions,
|
||||
}: HyperJsonProps) => {
|
||||
const isEmpty = React.useMemo(() => Object.keys(data).length === 0, [data]);
|
||||
|
|
@ -410,7 +410,7 @@ const HyperJson = ({
|
|||
<div
|
||||
className={cx(styles.container, {
|
||||
[styles.withTabulate]: tabulate,
|
||||
[styles.withLineWrap]: lineWrap,
|
||||
[styles.withPreWrap]: whiteSpace === 'pre-wrap',
|
||||
})}
|
||||
>
|
||||
{isEmpty ? <div>Empty</div> : <TreeNode data={data} />}
|
||||
|
|
|
|||
113
packages/app/src/components/__tests__/DBRowJsonViewer.test.ts
Normal file
113
packages/app/src/components/__tests__/DBRowJsonViewer.test.ts
Normal file
|
|
@ -0,0 +1,113 @@
|
|||
import { migrateViewerOptions } from '../DBRowJsonViewer';
|
||||
|
||||
describe('migrateViewerOptions', () => {
|
||||
beforeEach(() => {
|
||||
localStorage.clear();
|
||||
});
|
||||
|
||||
it('returns null for null input', () => {
|
||||
expect(migrateViewerOptions(null)).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for invalid JSON', () => {
|
||||
expect(migrateViewerOptions('not json')).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for non-object JSON', () => {
|
||||
expect(migrateViewerOptions('"string"')).toBeNull();
|
||||
expect(migrateViewerOptions('42')).toBeNull();
|
||||
expect(migrateViewerOptions('null')).toBeNull();
|
||||
});
|
||||
|
||||
it('migrates lineWrap: true (old default) to whiteSpace: undefined', () => {
|
||||
const old = JSON.stringify({
|
||||
normallyExpanded: true,
|
||||
lineWrap: true,
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
});
|
||||
const result = migrateViewerOptions(old);
|
||||
expect(result).toEqual({
|
||||
normallyExpanded: true,
|
||||
whiteSpace: undefined,
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('migrates lineWrap: false (user wanted wrapping) to whiteSpace: pre-wrap', () => {
|
||||
const old = JSON.stringify({
|
||||
normallyExpanded: true,
|
||||
lineWrap: false,
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
});
|
||||
const result = migrateViewerOptions(old);
|
||||
expect(result).toEqual({
|
||||
normallyExpanded: true,
|
||||
whiteSpace: 'pre-wrap',
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('writes migrated value back to localStorage', () => {
|
||||
const old = JSON.stringify({
|
||||
normallyExpanded: true,
|
||||
lineWrap: false,
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
});
|
||||
localStorage.setItem('hdx_json_viewer_options', old);
|
||||
migrateViewerOptions(old);
|
||||
|
||||
const stored = JSON.parse(localStorage.getItem('hdx_json_viewer_options')!);
|
||||
expect(stored.whiteSpace).toBe('pre-wrap');
|
||||
expect(stored).not.toHaveProperty('lineWrap');
|
||||
});
|
||||
|
||||
it('passes through already-migrated options unchanged', () => {
|
||||
const current = JSON.stringify({
|
||||
normallyExpanded: true,
|
||||
whiteSpace: 'pre',
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
});
|
||||
const result = migrateViewerOptions(current);
|
||||
expect(result).toEqual({
|
||||
normallyExpanded: true,
|
||||
whiteSpace: 'pre',
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('passes through options with whiteSpace: pre-wrap unchanged', () => {
|
||||
const current = JSON.stringify({
|
||||
normallyExpanded: false,
|
||||
whiteSpace: 'pre-wrap',
|
||||
tabulate: false,
|
||||
filterBlanks: true,
|
||||
});
|
||||
const result = migrateViewerOptions(current);
|
||||
expect(result).toEqual({
|
||||
normallyExpanded: false,
|
||||
whiteSpace: 'pre-wrap',
|
||||
tabulate: false,
|
||||
filterBlanks: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('preserves other options during migration', () => {
|
||||
const old = JSON.stringify({
|
||||
normallyExpanded: false,
|
||||
lineWrap: true,
|
||||
tabulate: false,
|
||||
filterBlanks: true,
|
||||
});
|
||||
const result = migrateViewerOptions(old);
|
||||
expect(result?.normallyExpanded).toBe(false);
|
||||
expect(result?.tabulate).toBe(false);
|
||||
expect(result?.filterBlanks).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,185 @@
|
|||
import { SearchPage } from '../../page-objects/SearchPage';
|
||||
import { expect, test } from '../../utils/base-test';
|
||||
|
||||
const VIEWER_OPTIONS_KEY = 'hdx_json_viewer_options';
|
||||
|
||||
test.describe('JSON Viewer WhiteSpace Toggle', { tag: ['@search'] }, () => {
|
||||
let searchPage: SearchPage;
|
||||
|
||||
async function openParsedTab(searchPage: SearchPage) {
|
||||
await searchPage.goto();
|
||||
await searchPage.submitEmptySearch();
|
||||
await expect(searchPage.table.firstRow).toBeVisible({ timeout: 10000 });
|
||||
await searchPage.table.clickFirstRow();
|
||||
await searchPage.sidePanel.clickTab('parsed');
|
||||
}
|
||||
|
||||
test('should default to pre-wrap (wrapping enabled)', async ({ page }) => {
|
||||
searchPage = new SearchPage(page);
|
||||
await openParsedTab(searchPage);
|
||||
|
||||
// Find a string value in the JSON viewer and check its white-space
|
||||
const stringEl = page.locator('[class*="string"]').first();
|
||||
await expect(stringEl).toBeVisible();
|
||||
await expect(stringEl).toHaveCSS('white-space', 'pre-wrap');
|
||||
});
|
||||
|
||||
test('should toggle between pre-wrap and pre', async ({ page }) => {
|
||||
searchPage = new SearchPage(page);
|
||||
await openParsedTab(searchPage);
|
||||
|
||||
const wrapToggle = page.getByTestId('json-viewer-wrap-toggle');
|
||||
const stringEl = page.locator('[class*="string"]').first();
|
||||
await expect(stringEl).toBeVisible();
|
||||
|
||||
// Default is pre-wrap
|
||||
await expect(stringEl).toHaveCSS('white-space', 'pre-wrap');
|
||||
|
||||
// Click toggle → should switch to pre
|
||||
await wrapToggle.click();
|
||||
await expect(stringEl).toHaveCSS('white-space', 'pre');
|
||||
|
||||
// Click again → should switch back to pre-wrap
|
||||
await wrapToggle.click();
|
||||
await expect(stringEl).toHaveCSS('white-space', 'pre-wrap');
|
||||
});
|
||||
|
||||
test('should persist toggle state in localStorage', async ({ page }) => {
|
||||
searchPage = new SearchPage(page);
|
||||
await openParsedTab(searchPage);
|
||||
|
||||
const wrapToggle = page.getByTestId('json-viewer-wrap-toggle');
|
||||
await wrapToggle.click();
|
||||
|
||||
// Verify localStorage was updated
|
||||
const stored = await page.evaluate(
|
||||
key => localStorage.getItem(key),
|
||||
VIEWER_OPTIONS_KEY,
|
||||
);
|
||||
const parsed = JSON.parse(stored!);
|
||||
expect(parsed.whiteSpace).toBe('pre');
|
||||
expect(parsed).not.toHaveProperty('lineWrap');
|
||||
});
|
||||
|
||||
test('should migrate old lineWrap: true (no-wrap default) to use new default', async ({
|
||||
page,
|
||||
}) => {
|
||||
// Seed old format before navigating
|
||||
await page.addInitScript(key => {
|
||||
localStorage.setItem(
|
||||
key,
|
||||
JSON.stringify({
|
||||
normallyExpanded: true,
|
||||
lineWrap: true,
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
}),
|
||||
);
|
||||
}, VIEWER_OPTIONS_KEY);
|
||||
|
||||
searchPage = new SearchPage(page);
|
||||
await openParsedTab(searchPage);
|
||||
|
||||
// Old lineWrap: true → whiteSpace: undefined → falls back to pre-wrap default
|
||||
const stringEl = page.locator('[class*="string"]').first();
|
||||
await expect(stringEl).toBeVisible();
|
||||
await expect(stringEl).toHaveCSS('white-space', 'pre-wrap');
|
||||
|
||||
// Verify migration happened in localStorage
|
||||
const stored = await page.evaluate(
|
||||
key => localStorage.getItem(key),
|
||||
VIEWER_OPTIONS_KEY,
|
||||
);
|
||||
const parsed = JSON.parse(stored!);
|
||||
expect(parsed).not.toHaveProperty('lineWrap');
|
||||
});
|
||||
|
||||
test('should visually wrap long text in pre-wrap mode and overflow in pre mode', async ({
|
||||
page,
|
||||
}) => {
|
||||
// Use a narrow viewport to force wrapping
|
||||
await page.setViewportSize({ width: 800, height: 600 });
|
||||
|
||||
searchPage = new SearchPage(page);
|
||||
await openParsedTab(searchPage);
|
||||
|
||||
const wrapToggle = page.getByTestId('json-viewer-wrap-toggle');
|
||||
|
||||
// Find a string element with content
|
||||
const stringEl = page.locator('[class*="string"]').first();
|
||||
await expect(stringEl).toBeVisible();
|
||||
|
||||
// Inject a very long string into the first string element to guarantee overflow
|
||||
await page.evaluate(() => {
|
||||
const el = document.querySelector('[class*="string"]');
|
||||
if (el) {
|
||||
el.textContent = 'A'.repeat(500) + ' BREAK_HERE ' + 'B'.repeat(500);
|
||||
}
|
||||
});
|
||||
|
||||
// In pre-wrap mode (default): element should wrap, so scrollWidth <= clientWidth of container
|
||||
const preWrapDimensions = await stringEl.evaluate(el => ({
|
||||
scrollWidth: el.scrollWidth,
|
||||
clientWidth: el.clientWidth,
|
||||
offsetHeight: (el as HTMLElement).offsetHeight,
|
||||
}));
|
||||
|
||||
// Toggle to pre mode (no wrap)
|
||||
await wrapToggle.click();
|
||||
|
||||
// Re-inject the long string (toggle may have re-rendered)
|
||||
await page.evaluate(() => {
|
||||
const el = document.querySelector('[class*="string"]');
|
||||
if (el) {
|
||||
el.textContent = 'A'.repeat(500) + ' BREAK_HERE ' + 'B'.repeat(500);
|
||||
}
|
||||
});
|
||||
|
||||
const preDimensions = await stringEl.evaluate(el => ({
|
||||
scrollWidth: el.scrollWidth,
|
||||
clientWidth: el.clientWidth,
|
||||
offsetHeight: (el as HTMLElement).offsetHeight,
|
||||
}));
|
||||
|
||||
// In pre-wrap mode: text should wrap, making the element taller
|
||||
// In pre mode: text should not wrap, making it wider (scrollWidth > clientWidth)
|
||||
// or shorter height
|
||||
expect(preWrapDimensions.offsetHeight).toBeGreaterThan(
|
||||
preDimensions.offsetHeight,
|
||||
);
|
||||
});
|
||||
|
||||
test('should migrate old lineWrap: false (user wanted wrapping) to pre-wrap', async ({
|
||||
page,
|
||||
}) => {
|
||||
// Seed old format — user had explicitly toggled wrapping on
|
||||
await page.addInitScript(key => {
|
||||
localStorage.setItem(
|
||||
key,
|
||||
JSON.stringify({
|
||||
normallyExpanded: true,
|
||||
lineWrap: false,
|
||||
tabulate: true,
|
||||
filterBlanks: false,
|
||||
}),
|
||||
);
|
||||
}, VIEWER_OPTIONS_KEY);
|
||||
|
||||
searchPage = new SearchPage(page);
|
||||
await openParsedTab(searchPage);
|
||||
|
||||
// Old lineWrap: false → whiteSpace: 'pre-wrap'
|
||||
const stringEl = page.locator('[class*="string"]').first();
|
||||
await expect(stringEl).toBeVisible();
|
||||
await expect(stringEl).toHaveCSS('white-space', 'pre-wrap');
|
||||
|
||||
// Verify migration wrote pre-wrap explicitly
|
||||
const stored = await page.evaluate(
|
||||
key => localStorage.getItem(key),
|
||||
VIEWER_OPTIONS_KEY,
|
||||
);
|
||||
const parsed = JSON.parse(stored!);
|
||||
expect(parsed.whiteSpace).toBe('pre-wrap');
|
||||
expect(parsed).not.toHaveProperty('lineWrap');
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue