Skip to content

Commit

Permalink
fix: Console history did not stick to bottom on visibility changes (#…
Browse files Browse the repository at this point in the history
…2324)

- When debugging, I found that the `Code` block was rendering while the
Console tab was in the background (invisible, size 0) so when it did
finally render, there was a lot of content added but it wasn't scrolled.
By checking sticky bottom when visibility changes, we handle this
situtation where content causes a resize while it is invisible
- Use an IntersectionObserver to detect visibility changes:
https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API
- Tested by running the following:
1. Run a code snippet to print a bunch of lines, causing the console
history to scroll:
```python
for i in range(0, 200):
    print("i is", i)
```
2. Run a snippet to create a dashboard, and have a lot of text in the
command:
```python
from deephaven import ui

... have lots of blank lines so that it takes up more than a page of scroll space

d = ui.dashboard(ui.panel("Hello"))
```
3. Go back to the Console tab, see that it is still scroll to the
bottom.
  • Loading branch information
mofojed authored Jan 2, 2025
1 parent 9899c1c commit ca5f6cd
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 9 deletions.
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);
});
});

0 comments on commit ca5f6cd

Please sign in to comment.