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: Console history did not stick to bottom on visibility changes #2324

Merged
merged 6 commits into from
Jan 2, 2025
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
6 changes: 6 additions & 0 deletions jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ Object.defineProperty(window, 'ResizeObserver', {
},
});

Object.defineProperty(window, 'IntersectionObserver', {
value: function () {
return TestUtils.createMockProxy<IntersectionObserver>();
},
});

Object.defineProperty(window, 'DOMRect', {
value: function (x: number = 0, y: number = 0, width = 0, height = 0) {
return TestUtils.createMockProxy<DOMRect>({
Expand Down
34 changes: 32 additions & 2 deletions packages/console/src/Console.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import React, {
type ReactElement,
type ReactNode,
type RefObject,
type UIEvent,
} from 'react';
import {
ContextActions,
Expand Down Expand Up @@ -190,6 +191,7 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
this.handleLogMessage = this.handleLogMessage.bind(this);
this.handleOverflowActions = this.handleOverflowActions.bind(this);
this.handleScrollPaneScroll = this.handleScrollPaneScroll.bind(this);
this.handleHistoryResize = this.handleHistoryResize.bind(this);
this.handleToggleAutoLaunchPanels =
this.handleToggleAutoLaunchPanels.bind(this);
this.handleToggleClosePanelsOnDisconnect =
Expand All @@ -211,8 +213,10 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
this.consolePane = React.createRef();
this.consoleInput = React.createRef();
this.consoleHistoryScrollPane = React.createRef();
this.consoleHistoryContent = React.createRef();
this.pending = new Pending();
this.queuedLogMessages = [];
this.resizeObserver = new window.ResizeObserver(this.handleHistoryResize);

const { objectMap, settings } = this.props;

Expand Down Expand Up @@ -253,6 +257,14 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
);

this.updateDimensions();

if (
this.consoleHistoryScrollPane.current &&
this.consoleHistoryContent.current
) {
this.resizeObserver.observe(this.consoleHistoryScrollPane.current);
this.resizeObserver.observe(this.consoleHistoryContent.current);
}
}

componentDidUpdate(prevProps: ConsoleProps, prevState: ConsoleState): void {
Expand Down Expand Up @@ -280,6 +292,7 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
this.processLogMessageQueue.cancel();

this.deinitConsoleLogging();
this.resizeObserver.disconnect();
}

cancelListener?: () => void;
Expand All @@ -290,10 +303,14 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {

consoleHistoryScrollPane: RefObject<HTMLDivElement>;

consoleHistoryContent: RefObject<HTMLDivElement>;

pending: Pending;

queuedLogMessages: ConsoleHistoryActionItem[];

resizeObserver: ResizeObserver;

initConsoleLogging(): void {
const { session } = this.props;
this.cancelListener = session.onLogMessage(this.handleLogMessage);
Expand Down Expand Up @@ -666,11 +683,12 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
}

window.requestAnimationFrame(() => {
pane.scrollTo({ top: pane.scrollHeight });
pane.scrollTo({ top: pane.scrollHeight, behavior: 'instant' });
});
}

handleScrollPaneScroll(): void {
handleScrollPaneScroll(event: UIEvent<HTMLDivElement>): void {
log.debug('handleScrollPaneScroll', event);
const scrollPane = this.consoleHistoryScrollPane.current;
assertNotNull(scrollPane);
this.setState({
Expand All @@ -681,6 +699,17 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
});
}

handleHistoryResize(entries: ResizeObserverEntry[]): void {
log.debug('handleHistoryResize', entries);
const entry = entries[0];
if (entry.contentRect.height > 0 && entry.contentRect.width > 0) {
const { isStuckToBottom } = this.state;
if (isStuckToBottom && !this.isAtBottom()) {
this.scrollConsoleHistoryToBottom();
}
}
}

handleToggleAutoLaunchPanels(): void {
this.setState(state => ({
isAutoLaunchPanelsEnabled: !state.isAutoLaunchPanelsEnabled,
Expand Down Expand Up @@ -1067,6 +1096,7 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
disabled={disabled}
supportsType={supportsType}
iconForType={iconForType}
ref={this.consoleHistoryContent}
/>
{historyChildren}
</div>
Expand Down
16 changes: 12 additions & 4 deletions packages/console/src/console-history/ConsoleHistory.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Console display for use in the Iris environment.
*/
import { type ReactElement } from 'react';
import React, { type ReactElement } from 'react';
import type { dh } from '@deephaven/jsapi-types';
import ConsoleHistoryItem from './ConsoleHistoryItem';

Expand All @@ -23,7 +23,13 @@ function itemKey(i: number, item: ConsoleHistoryActionItem): string {
}`;
}

function ConsoleHistory(props: ConsoleHistoryProps): ReactElement {
/**
* Display the console history.
*/
const ConsoleHistory = React.forwardRef(function ConsoleHistory(
props: ConsoleHistoryProps,
ref: React.Ref<HTMLDivElement>
): ReactElement {
const {
disabled = false,
items,
Expand All @@ -50,8 +56,10 @@ function ConsoleHistory(props: ConsoleHistoryProps): ReactElement {
}

return (
<div className="container-fluid console-history">{historyElements}</div>
<div ref={ref} className="container-fluid console-history">
{historyElements}
</div>
);
}
});

export default ConsoleHistory;
89 changes: 86 additions & 3 deletions tests/console.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,26 @@ import {
generateId,
} from './utils';

function logMessageLocator(page: Page, text?: string): Locator {
return page
.locator('.console-history .log-message')
.filter({ hasText: text });
}

function historyContentLocator(page: Page, text?: string): Locator {
return page
.locator('.console-history .console-history-content')
.filter({ hasText: text });
}

function panelTabLocator(page: Page, text: string): Locator {
return page.locator('.lm_tab .lm_title').filter({ hasText: text });
}

function scrollPanelLocator(page: Page): Locator {
return page.locator('.console-pane .scroll-pane');
}

let page: Page;
let consoleInput: Locator;

Expand All @@ -32,9 +52,8 @@ test.describe('console input tests', () => {
await page.keyboard.press('Enter');

// Expect the output to show up in the log
await expect(
page.locator('.console-history .log-message').filter({ hasText: message })
).toHaveCount(1);
await expect(logMessageLocator(page, message)).toHaveCount(1);
await expect(logMessageLocator(page, message)).toBeVisible();
});

test('object button is created when creating a table', async ({
Expand All @@ -61,3 +80,67 @@ test.describe('console input tests', () => {
await expect(btnLocator.nth(1)).not.toBeDisabled();
});
});

test.describe('console scroll tests', () => {
test.beforeEach(async () => {
// Whenever we start a session, the server sends it logs over
// We should wait for those to appear before running commands
await logMessageLocator(page).first().waitFor();
});

test('scrolls to the bottom when command is executed', async () => {
// The command needs to be long, but it doesn't need to actually print anything
const ids = Array.from(Array(50).keys()).map(() => generateId());
const command = ids.map(i => `# Really long command ${i}`).join('\n');

await pasteInMonaco(consoleInput, command);
await page.keyboard.press('Enter');

await historyContentLocator(page, ids[ids.length - 1]).waitFor({
state: 'attached',
});

// Wait for the scroll to complete, since it starts on the next available animation frame
await page.waitForTimeout(500);

// Expect the console to be scrolled to the bottom
const scrollPane = await scrollPanelLocator(page);
expect(
await scrollPane.evaluate(el =>
Math.floor(el.scrollHeight - el.scrollTop - el.clientHeight)
)
).toBeLessThanOrEqual(0);
});

test('scrolls to the bottom when focus changed when command executed', async () => {
// The command needs to be long, but it doesn't need to actually print anything
const ids = Array.from(Array(50).keys()).map(() => generateId());
const command = `import time\ntime.sleep(0.5)\n${ids
.map(i => `# Really long command ${i}`)
.join('\n')}`;

await pasteInMonaco(consoleInput, command);
page.keyboard.press('Enter');

// Immediately open the log before the command code block has a chance to finish colorizing/rendering
await panelTabLocator(page, 'Log').click();

// wait for a bit for the code block to render
await historyContentLocator(page, ids[ids.length - 1]).waitFor({
state: 'attached',
});

// Switch back to the console, and expect it to be scrolled to the bottom
await panelTabLocator(page, 'Console').click();

// Wait for the scroll to complete, since it starts on the next available animation frame
await page.waitForTimeout(500);

const scrollPane = await scrollPanelLocator(page);
expect(
await scrollPane.evaluate(el =>
Math.floor(el.scrollHeight - el.scrollTop - el.clientHeight)
)
).toBeLessThanOrEqual(0);
});
});
Loading