Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(LogViewer): Prevent long lines from overlapping the line after them #62

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
],
"scripts": {
"build": "yarn workspace @patternfly/react-log-viewer build",
"build:watch": "npm run build:watch -w @patternfly/react-log-viewer",
"build:docs": "yarn workspace @patternfly/react-log-viewer docs:build",
"start": "yarn build && concurrently --kill-others \"yarn workspace @patternfly/react-log-viewer docs:develop\"",
"start": "concurrently --kill-others \"npm run build:watch\" \"npm run docs:develop -w @patternfly/react-log-viewer\"",
"serve:docs": "yarn workspace @patternfly/react-log-viewer docs:serve",
"clean": "yarn workspace @patternfly/react-log-viewer clean",
"lint:js": "node --max-old-space-size=4096 node_modules/.bin/eslint packages --ext js,jsx,ts,tsx --cache",
Expand Down
1 change: 1 addition & 0 deletions packages/module/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
},
"scripts": {
"build": "yarn build:esm && yarn build:cjs",
"build:watch": "npm run build:esm -- --watch",
"build:esm": "tsc --build --verbose ./tsconfig.json",
"build:cjs": "tsc --build --verbose ./tsconfig.cjs.json",
"clean": "rimraf dist",
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ const pageData = {
"type": "string | string[]",
"description": "String or String Array data being sent by the consumer"
},
{
"name": "fastRowHeightEstimationLimit",
"type": "number",
"description": "The maximum char length for fast row height estimation.\nFor wrapped lines in Chrome based browsers, lines over this length will actually be rendered to the dom and\nmeasured to prevent a bug where one line can overlap another."
},
{
"name": "footer",
"type": "React.ReactNode",
Expand Down Expand Up @@ -101,6 +106,11 @@ const pageData = {
"type": "React.ReactNode",
"description": "Toolbar rendered in the log viewer header"
},
{
"name": "useAnsiClasses",
"type": "boolean",
"description": "Flag to enable or disable the use of classes (instead of inline styles) for ANSI coloring/formatting."
},
{
"name": "width",
"type": "number | string",
Expand Down
39 changes: 38 additions & 1 deletion packages/module/src/LogViewer/LogViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ interface LogViewerProps {
innerRef?: React.RefObject<any>;
/** Flag to enable or disable the use of classes (instead of inline styles) for ANSI coloring/formatting. */
useAnsiClasses?: boolean;
/** The maximum char length for fast row height estimation.
* For wrapped lines in Chrome based browsers, lines over this length will actually be rendered to the dom and
* measured to prevent a bug where one line can overlap another.
*/
fastRowHeightEstimationLimit?: number;
}

let canvas: HTMLCanvasElement | undefined;
Expand Down Expand Up @@ -92,6 +97,7 @@ const LogViewerBase: React.FunctionComponent<LogViewerProps> = memo(
isTextWrapped = true,
initialIndexWidth,
useAnsiClasses,
fastRowHeightEstimationLimit = 5000,
...props
}: LogViewerProps) => {
const [searchedInput, setSearchedInput] = useState<string | null>('');
Expand All @@ -108,6 +114,8 @@ const LogViewerBase: React.FunctionComponent<LogViewerProps> = memo(
/* Parse data every time it changes */
const parsedData = React.useMemo(() => parseConsoleOutput(data), [data]);

const isChrome = React.useMemo(() => navigator.userAgent.indexOf('Chrome') !== -1, []);

const ansiUp = new AnsiUp();
// eslint-disable-next-line camelcase
ansiUp.escape_html = false;
Expand Down Expand Up @@ -228,6 +236,26 @@ const LogViewerBase: React.FunctionComponent<LogViewerProps> = memo(
setListKey(listKey => listKey + 1);
}, [isTextWrapped]);

const computeRowHeight = (rowText: string, estimatedHeight: number) => {
const logViewerList = containerRef.current.firstChild.firstChild;

// early return with the estimated height if the log viewer list hasn't been rendered yet,
// this will be called again once it has been rendered and the correct height will be set
if (!logViewerList) {
return estimatedHeight;
}

const dummyText = document.createElement('span');
dummyText.className = css(styles.logViewerText);
dummyText.innerHTML = rowText;

logViewerList.appendChild(dummyText);
const computedHeight = dummyText.clientHeight
logViewerList.removeChild(dummyText);

return computedHeight
}

const guessRowHeight = (rowIndex: number) => {
if (!isTextWrapped) {
return lineHeight;
Expand All @@ -237,7 +265,16 @@ const LogViewerBase: React.FunctionComponent<LogViewerProps> = memo(
// get the row numbers of the current text
const numRows = Math.ceil(rowText.length / charNumsPerLine);
// multiply by line height to get the total height
return lineHeight * (numRows || 1);
const heightGuess = lineHeight * (numRows || 1);

// because of a bug in react-window (which seems to be limited to chrome) we need to
// actually compute row height in long lines to prevent them from overflowing.
// related issue https://github.com/bvaughn/react-window/issues/593
if (rowText.length > fastRowHeightEstimationLimit && isChrome && isTextWrapped) {
return computeRowHeight(rowText, heightGuess);
}

return heightGuess
};

const createList = (parsedData: string[]) => (
Expand Down
Loading