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
…2320)

fix: Console history did not stick to bottom on visibility changes
    
- 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 Dec 30, 2024
1 parent 57b6e21 commit 648e8c0
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 4 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
27 changes: 26 additions & 1 deletion 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.handleVisibilityChange = this.handleVisibilityChange.bind(this);
this.handleToggleAutoLaunchPanels =
this.handleToggleAutoLaunchPanels.bind(this);
this.handleToggleClosePanelsOnDisconnect =
Expand All @@ -213,6 +215,9 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
this.consoleHistoryScrollPane = React.createRef();
this.pending = new Pending();
this.queuedLogMessages = [];
this.visibilityObserver = new window.IntersectionObserver(
this.handleVisibilityChange
);

const { objectMap, settings } = this.props;

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

this.updateDimensions();

if (this.consolePane.current) {
this.visibilityObserver.observe(this.consolePane.current);
}
}

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

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

cancelListener?: () => void;
Expand All @@ -294,6 +304,8 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {

queuedLogMessages: ConsoleHistoryActionItem[];

visibilityObserver: IntersectionObserver;

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

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

handleVisibilityChange(entries: IntersectionObserverEntry[]): void {
log.debug('handleVisibilityChange', entries);

const entry = entries[0];
if (entry.isIntersecting) {
const { isStuckToBottom } = this.state;
if (isStuckToBottom && !this.isAtBottom()) {
this.scrollConsoleHistoryToBottom();
}
}
}

handleToggleAutoLaunchPanels(): void {
this.setState(state => ({
isAutoLaunchPanelsEnabled: !state.isAutoLaunchPanelsEnabled,
Expand Down
69 changes: 66 additions & 3 deletions tests/console.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@ import {
generateId,
} from './utils';

function logMessageLocator(page: Page, text: string): Locator {
return page
.locator('.console-history .log-message')
.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 +46,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 +74,53 @@ test.describe('console input tests', () => {
await expect(btnLocator.nth(1)).not.toBeDisabled();
});
});

test.describe('console scroll tests', () => {
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(300).keys()).map(() => generateId());
const command = ids.map(i => `# Really long command ${i}`).join('\n');

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

// Allow time for the text to colorize/render
await page.waitForTimeout(100);

// Expect the console to be scrolled to the bottom
const scrollPane = await scrollPanelLocator(page);
expect(
await scrollPane.evaluate(el => {
return 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(300).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);
await 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
// Since it's in the background, we can't use the waitForSelector method. It should render in less than 1s.
await page.waitForTimeout(1000);

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

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

0 comments on commit 648e8c0

Please sign in to comment.