From ca5f6cd7b7f6af11d34be8ba532723d834a7be12 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Thu, 2 Jan 2025 11:45:52 -0500 Subject: [PATCH] fix: Console history did not stick to bottom on visibility changes (#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. --- jest.setup.ts | 6 ++ packages/console/src/Console.tsx | 34 ++++++- .../src/console-history/ConsoleHistory.tsx | 16 +++- tests/console.spec.ts | 89 ++++++++++++++++++- 4 files changed, 136 insertions(+), 9 deletions(-) diff --git a/jest.setup.ts b/jest.setup.ts index 3aeec7e365..8d0dea0b90 100644 --- a/jest.setup.ts +++ b/jest.setup.ts @@ -50,6 +50,12 @@ Object.defineProperty(window, 'ResizeObserver', { }, }); +Object.defineProperty(window, 'IntersectionObserver', { + value: function () { + return TestUtils.createMockProxy(); + }, +}); + Object.defineProperty(window, 'DOMRect', { value: function (x: number = 0, y: number = 0, width = 0, height = 0) { return TestUtils.createMockProxy({ diff --git a/packages/console/src/Console.tsx b/packages/console/src/Console.tsx index f7baf377d6..ffc6936020 100644 --- a/packages/console/src/Console.tsx +++ b/packages/console/src/Console.tsx @@ -7,6 +7,7 @@ import React, { type ReactElement, type ReactNode, type RefObject, + type UIEvent, } from 'react'; import { ContextActions, @@ -190,6 +191,7 @@ export class Console extends PureComponent { 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 = @@ -211,8 +213,10 @@ export class Console extends PureComponent { 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; @@ -253,6 +257,14 @@ export class Console extends PureComponent { ); 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 { @@ -280,6 +292,7 @@ export class Console extends PureComponent { this.processLogMessageQueue.cancel(); this.deinitConsoleLogging(); + this.resizeObserver.disconnect(); } cancelListener?: () => void; @@ -290,10 +303,14 @@ export class Console extends PureComponent { consoleHistoryScrollPane: RefObject; + consoleHistoryContent: RefObject; + pending: Pending; queuedLogMessages: ConsoleHistoryActionItem[]; + resizeObserver: ResizeObserver; + initConsoleLogging(): void { const { session } = this.props; this.cancelListener = session.onLogMessage(this.handleLogMessage); @@ -666,11 +683,12 @@ export class Console extends PureComponent { } window.requestAnimationFrame(() => { - pane.scrollTo({ top: pane.scrollHeight }); + pane.scrollTo({ top: pane.scrollHeight, behavior: 'instant' }); }); } - handleScrollPaneScroll(): void { + handleScrollPaneScroll(event: UIEvent): void { + log.debug('handleScrollPaneScroll', event); const scrollPane = this.consoleHistoryScrollPane.current; assertNotNull(scrollPane); this.setState({ @@ -681,6 +699,17 @@ export class Console extends PureComponent { }); } + 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, @@ -1067,6 +1096,7 @@ export class Console extends PureComponent { disabled={disabled} supportsType={supportsType} iconForType={iconForType} + ref={this.consoleHistoryContent} /> {historyChildren} diff --git a/packages/console/src/console-history/ConsoleHistory.tsx b/packages/console/src/console-history/ConsoleHistory.tsx index e239fec978..1179764a8f 100644 --- a/packages/console/src/console-history/ConsoleHistory.tsx +++ b/packages/console/src/console-history/ConsoleHistory.tsx @@ -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'; @@ -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 +): ReactElement { const { disabled = false, items, @@ -50,8 +56,10 @@ function ConsoleHistory(props: ConsoleHistoryProps): ReactElement { } return ( -
{historyElements}
+
+ {historyElements} +
); -} +}); export default ConsoleHistory; diff --git a/tests/console.spec.ts b/tests/console.spec.ts index 8681712784..8f6fd432a7 100644 --- a/tests/console.spec.ts +++ b/tests/console.spec.ts @@ -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; @@ -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 ({ @@ -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); + }); +});