Skip to content

Commit c874463

Browse files
ktranDevtools-frontend LUCI CQ
authored andcommitted
Reland "Fix scrolling into view in Treeoutline"
This is a reland of commit 1dcc489 Difference to original: I've added an await function to properly wait for the expected scroll difference before asserting it. Original change's description: > Fix scrolling into view in Treeoutline > > The scrollable container for the Treeoutline has changed, and as a > result, the wrong container was used for scrolling when we attempted > to scoll an item in the tree into view. > > This CL changes the way we retrieve the correct scroll container. > Instead of using the parent element, we either get the parent > element or the shadow host. > > Fixed: 374592181 > Change-Id: Ib3d033b0a4d1c7f7b88e2af63f9bd1f67de452d9 > Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5961819 > Commit-Queue: Kim-Anh Tran <[email protected]> > Reviewed-by: Benedikt Meurer <[email protected]> Bug: 374592181 Change-Id: I56fa39a1473b5d40f4f1b0089302a079bf7fad53 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5969660 Auto-Submit: Kim-Anh Tran <[email protected]> Commit-Queue: Benedikt Meurer <[email protected]> Reviewed-by: Benedikt Meurer <[email protected]>
1 parent 397bdd9 commit c874463

File tree

2 files changed

+61
-6
lines changed

2 files changed

+61
-6
lines changed

front_end/ui/legacy/Treeoutline.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,12 @@ export class TreeOutline extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
349349

350350
// Usually, this.element is the tree container that scrolls. But sometimes
351351
// (i.e. in the Elements panel), its parent is.
352-
let scrollParentElement: HTMLElement = this.element;
353-
while (getComputedStyle(scrollParentElement).overflow === 'visible' && scrollParentElement.parentElement) {
354-
scrollParentElement = scrollParentElement.parentElement;
352+
let scrollParentElement: Element = this.element;
353+
while (getComputedStyle(scrollParentElement).overflow === 'visible' &&
354+
scrollParentElement.parentElementOrShadowHost()) {
355+
const parent = scrollParentElement.parentElementOrShadowHost();
356+
Platform.assertNotNullOrUndefined(parent);
357+
scrollParentElement = parent;
355358
}
356359

357360
const viewRect = scrollParentElement.getBoundingClientRect();

test/e2e/sources/navigator-view_test.ts

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,23 @@
44

55
import {assert} from 'chai';
66

7-
import {getBrowserAndPages, waitFor, waitForNone} from '../../shared/helper.js';
8-
7+
import {
8+
getBrowserAndPages,
9+
waitFor,
10+
waitForElementWithTextContent,
11+
waitForFunction,
12+
waitForNone,
13+
} from '../../shared/helper.js';
914
import {openSoftContextMenuAndClickOnItem} from '../helpers/context-menu-helpers.js';
10-
import {openFileWithQuickOpen, runCommandWithQuickOpen} from '../helpers/quick_open-helpers.js';
1115
import {
16+
getMenuItemAtPosition,
17+
openFileQuickOpen,
18+
openFileWithQuickOpen,
19+
runCommandWithQuickOpen,
20+
typeIntoQuickOpen,
21+
} from '../helpers/quick_open-helpers.js';
22+
import {
23+
createNewSnippet,
1224
openFileInSourcesPanel,
1325
openSnippetsSubPane,
1426
openSourceCodeEditorForFile,
@@ -140,5 +152,45 @@ describe('The Sources panel', () => {
140152
await toggleNavigatorSidebar(frontend);
141153
await waitFor('.navigator-tabbed-pane');
142154
});
155+
156+
it('which can scroll the navigator element into view on source file change', async () => {
157+
async function openSnippet(snippet: string) {
158+
await openFileQuickOpen();
159+
await typeIntoQuickOpen(snippet);
160+
const firstItem = await getMenuItemAtPosition(0);
161+
await firstItem.click();
162+
}
163+
164+
async function assertSnippetIsSelected(snippet: string) {
165+
const selectedItem = await waitFor('.navigator-file-tree-item.selected');
166+
const selectedItemName = await selectedItem.evaluate(node => node.textContent);
167+
assert.strictEqual(selectedItemName, snippet);
168+
}
169+
170+
await openSourcesPanel();
171+
await openSnippetsSubPane();
172+
173+
const numSnippets = 50;
174+
for (let i = 0; i < numSnippets; ++i) {
175+
await createNewSnippet(`Snippet${i}`);
176+
}
177+
const firstSnippetName = 'Snippet0';
178+
const lastSnippetName = `Snippet${numSnippets - 1}`;
179+
180+
await waitForElementWithTextContent(lastSnippetName);
181+
await assertSnippetIsSelected(lastSnippetName);
182+
183+
const snippetsPanel = await waitFor('[aria-label="Snippets panel"]');
184+
const scrollTopBeforeFileChange = await snippetsPanel.evaluate(panel => panel.scrollTop);
185+
186+
await openSnippet(firstSnippetName);
187+
await assertSnippetIsSelected(firstSnippetName);
188+
await waitForFunction(async () => {
189+
return await snippetsPanel.evaluate(panel => panel.scrollTop) === 0;
190+
});
191+
192+
const scrollTopAfterFileChange = await snippetsPanel.evaluate(panel => panel.scrollTop);
193+
assert.notStrictEqual(scrollTopBeforeFileChange, scrollTopAfterFileChange);
194+
});
143195
});
144196
});

0 commit comments

Comments
 (0)