From 648e8c030bb3f03ca00fc0503874b4947f3f8d54 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Mon, 30 Dec 2024 09:20:17 -0500 Subject: [PATCH] fix: Console history did not stick to bottom on visibility changes (#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. --- jest.setup.ts | 6 +++ packages/console/src/Console.tsx | 27 ++++++++++++- tests/console.spec.ts | 69 ++++++++++++++++++++++++++++++-- 3 files changed, 98 insertions(+), 4 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..b94bc78ff8 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.handleVisibilityChange = this.handleVisibilityChange.bind(this); this.handleToggleAutoLaunchPanels = this.handleToggleAutoLaunchPanels.bind(this); this.handleToggleClosePanelsOnDisconnect = @@ -213,6 +215,9 @@ export class Console extends PureComponent { this.consoleHistoryScrollPane = React.createRef(); this.pending = new Pending(); this.queuedLogMessages = []; + this.visibilityObserver = new window.IntersectionObserver( + this.handleVisibilityChange + ); const { objectMap, settings } = this.props; @@ -253,6 +258,10 @@ export class Console extends PureComponent { ); this.updateDimensions(); + + if (this.consolePane.current) { + this.visibilityObserver.observe(this.consolePane.current); + } } componentDidUpdate(prevProps: ConsoleProps, prevState: ConsoleState): void { @@ -280,6 +289,7 @@ export class Console extends PureComponent { this.processLogMessageQueue.cancel(); this.deinitConsoleLogging(); + this.visibilityObserver.disconnect(); } cancelListener?: () => void; @@ -294,6 +304,8 @@ export class Console extends PureComponent { queuedLogMessages: ConsoleHistoryActionItem[]; + visibilityObserver: IntersectionObserver; + initConsoleLogging(): void { const { session } = this.props; this.cancelListener = session.onLogMessage(this.handleLogMessage); @@ -670,7 +682,8 @@ export class Console extends PureComponent { }); } - handleScrollPaneScroll(): void { + handleScrollPaneScroll(event: UIEvent): void { + log.debug('handleScrollPaneScroll', event); const scrollPane = this.consoleHistoryScrollPane.current; assertNotNull(scrollPane); this.setState({ @@ -681,6 +694,18 @@ export class Console extends PureComponent { }); } + 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, diff --git a/tests/console.spec.ts b/tests/console.spec.ts index 8681712784..5afbc9aa79 100644 --- a/tests/console.spec.ts +++ b/tests/console.spec.ts @@ -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; @@ -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 ({ @@ -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); + }); +});