From 36b8156b7fb6159db8e9c67a983842650ff8eed6 Mon Sep 17 00:00:00 2001 From: mikebender Date: Fri, 27 Dec 2024 10:37:21 -0500 Subject: [PATCH 1/3] 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. --- packages/console/src/Console.tsx | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/console/src/Console.tsx b/packages/console/src/Console.tsx index f7baf377d6..647308d02f 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 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, From cfed54e7ca788c148e3c23268a15127eda394622 Mon Sep 17 00:00:00 2001 From: mikebender Date: Fri, 27 Dec 2024 11:14:31 -0500 Subject: [PATCH 2/3] Add e2e tests --- tests/console.spec.ts | 69 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 3 deletions(-) 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); + }); +}); From e121c904dc9accd155c943ba344660289c7f499c Mon Sep 17 00:00:00 2001 From: mikebender Date: Fri, 27 Dec 2024 12:31:42 -0500 Subject: [PATCH 3/3] Fix unit tests - Needed to define `IntersectionObserver` on the `window` --- jest.setup.ts | 6 ++++++ packages/console/src/Console.tsx | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) 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 647308d02f..b94bc78ff8 100644 --- a/packages/console/src/Console.tsx +++ b/packages/console/src/Console.tsx @@ -215,7 +215,7 @@ export class Console extends PureComponent { this.consoleHistoryScrollPane = React.createRef(); this.pending = new Pending(); this.queuedLogMessages = []; - this.visibilityObserver = new IntersectionObserver( + this.visibilityObserver = new window.IntersectionObserver( this.handleVisibilityChange );