zzos/react-native-devtools-frontend
1
0
Fork 0
mirror of https://github.com/react/react-native-devtools-frontend.git synced 2026-09-30 17:27:22 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix conflicting scroll instructions in Breadcrumbs

Browse Source 
The flakey tests on Windows exposed a bug where:

* We render and click the right button to scroll
* The code that ensures the active node is in view runs, and tries to
  scroll the active element into view.

These two actions would conflict with each other. On Linux/Mac and our Windows
bots, it seems the right scroll "wins" and the tests pass. However, on Windows
10 the right scroll is overriden by scrolling the active node into view, and
therefore the test fails.

The fix we landed on is to flag if the user has manually scrolled the
crumbs, and gate the scrolling active node into view logic on that. If
the user manually scrolls, we effectively cede control and let them
control the scrolling. When we get an update (e.g. a new active selected
node), we take back control.

Additionally, the resize observer was being far too aggressive in its behaviour
(just trigger a re-render), so that's been updated to check explicitly just if
the scroll buttons need to be hidden/shown after a render. This caused a bug
when running tests in Windows where it would also cause scroll conflicts. It's
not a bug I think users would ever have hit, but one that the tests hit because
they do things so much quicker than a user actually would (e.g. render + click
button in same frame). There are tests added for the resize observer behaviour.

Change-Id: Iedf4cbe5bef652d16fc80a0e7c0b3b45a95ebbfa
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2449984
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This commit is contained in:
Jack Franklin
2020-10-06 15:17:10 +00:00
committed by Commit Bot
parent e017318956
commit 10071897de
5 changed files with 258 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
front_end/component_docs/elements_breadcrumbs/BUILD.gn
+1
View File
@@ -7,6 +7,7 @@ import("../../../scripts/build/ninja/copy.gni")
copy_to_gen("elements_breadcrumbs") {
sources = [
"basic.html",
"scroll-to-active-element.html",
"scroll.html",
]
front_end/component_docs/elements_breadcrumbs/scroll-to-active-element.html
+142
View File
@@ -0,0 +1,142 @@
<!--
Copyright 2020 The Chromium Authors. All rights reserved.
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
-->
<html>
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width" />
<title>Scrolling breadcrumbs example</title>
<style>
#container {
width: 500px;
border: 1px solid black;
}
button {
margin-top: 20px;
width: 400px;
font-size: 18px;
padding: 10px;
}
:root {
--tab-selected-bg-color: lightblue;
}
</style>
</head>
<body>
<div id="container">
</div>
<button>Click me to make the <code>div</code> crumb the selected node (as if the user had clicked it in the elements pane)</button>
<script type="module">
import * as ComponentHelpers from '../../component_helpers/component_helpers.js';
import {ElementsBreadcrumbs} from '../../elements/ElementsBreadcrumbs.js';
ComponentHelpers.ComponentServerSetup.setup().then(() => renderComponent())
const renderComponent = () => {
let id = 0;
const makeCrumb = (overrides = {}) => {
const attributes = overrides.attributes || {};
const newCrumb = {
nodeType: Node.ELEMENT_NODE,
id: id++,
pseudoType: '',
shadowRootType: '',
nodeName: 'body',
nodeNameNicelyCased: 'body',
legacyDomNode: {},
highlightNode: () => {},
clearHighlight: () => {},
getAttribute: x => attributes[x] || '',
...overrides,
};
return newCrumb;
};
const component = new ElementsBreadcrumbs();
const bodyCrumb = makeCrumb({
nodeType: Node.ELEMENT_NODE,
id: 1,
nodeName: 'body',
nodeNameNicelyCased: 'body',
attributes: {
class: 'body-class1 body-class2'
}
});
const divCrumb = makeCrumb({
nodeType: Node.ELEMENT_NODE,
id: 2,
nodeName: 'div',
nodeNameNicelyCased: 'div',
attributes: {
id: 'test-id',
class: 'wrapper-div',
},
});
const spanCrumb = makeCrumb({
nodeType: Node.ELEMENT_NODE,
id: 3,
nodeName: 'span',
nodeNameNicelyCased: 'span',
attributes: {
id: 'my-span-has-a-long-id',
},
});
const strongCrumb = makeCrumb({
nodeType: Node.ELEMENT_NODE,
id: 4,
nodeName: 'strong',
nodeNameNicelyCased: 'strong',
attributes: {
id: 'gotta-be-bold',
},
});
const emCrumb = makeCrumb({
nodeType: Node.ELEMENT_NODE,
id: 5,
nodeName: 'em',
nodeNameNicelyCased: 'em',
attributes: {
id: 'my-em-has-a-long-id',
class: 'and-a-very-long-class'
},
});
document.getElementById('container').appendChild(component);
component.data = {
crumbs: [emCrumb, strongCrumb, spanCrumb, divCrumb, bodyCrumb],
selectedNode: bodyCrumb,
};
const button = component.shadowRoot.querySelector('button.overflow.right');
button.dispatchEvent(new MouseEvent('click'))
// Each subsequent click is timed out to allow the smooth scroll to finish.
window.setTimeout(() => {
button.dispatchEvent(new MouseEvent('click'))
window.setTimeout(() => {
button.dispatchEvent(new MouseEvent('click'))
}, 200)
}, 200)
const btn = document.querySelector('button');
btn.addEventListener('click', () => {
component.data = {
crumbs: [emCrumb, strongCrumb, spanCrumb, divCrumb, bodyCrumb],
selectedNode: divCrumb,
};
})
};
</script>
</body>
</html>
front_end/elements/ElementsBreadcrumbs.ts
+46 -3
View File
@@ -17,17 +17,19 @@ export interface ElementsBreadcrumbsData {
}
export class ElementsBreadcrumbs extends HTMLElement {
private readonly shadow = this.attachShadow({mode: 'open'});
private readonly resizeObserver = new ResizeObserver(() => this.update());
private readonly resizeObserver = new ResizeObserver(() => this.checkForOverflowOnResize());
private crumbsData: ReadonlyArray<DOMNode> = [];
private selectedDOMNode: Readonly<DOMNode>|null = null;
private overflowing = false;
private userScrollPosition: UserScrollPosition = 'start';
private isObservingResize = false;
private userHasManuallyScrolled = false;
set data(data: ElementsBreadcrumbsData) {
this.selectedDOMNode = data.selectedNode;
this.crumbsData = data.crumbs;
this.userHasManuallyScrolled = false;
this.update();
}
@@ -43,6 +45,34 @@ export class ElementsBreadcrumbs extends HTMLElement {
};
}
/*
* When the window is resized, we need to check if we either:
* 1) overflowing, and now the window is big enough that we don't need to
* 2) not overflowing, and now the window is small and we do need to
*
* If either of these are true, we toggle the overflowing state accordingly and trigger a re-render.
*/
private checkForOverflowOnResize() {
const wrappingElement = this.shadow.querySelector('.crumbs');
const crumbs = this.shadow.querySelector('.crumbs-scroll-container');
if (!wrappingElement || !crumbs) {
return;
}
const totalContainingWidth = wrappingElement.clientWidth;
const totalCrumbsWidth = crumbs.clientWidth;
if (totalCrumbsWidth >= totalContainingWidth && this.overflowing === false) {
this.overflowing = true;
this.userScrollPosition = 'start';
this.render();
} else if (totalCrumbsWidth < totalContainingWidth && this.overflowing === true) {
this.overflowing = false;
this.userScrollPosition = 'start';
this.render();
}
}
private update() {
this.overflowing = false;
this.userScrollPosition = 'start';
@@ -157,6 +187,7 @@ export class ElementsBreadcrumbs extends HTMLElement {
private onOverflowClick(direction: 'left'|'right') {
return () => {
this.userHasManuallyScrolled = true;
const scrollWindow = this.shadow.querySelector('.crumbs-window');
if (!scrollWindow) {
@@ -309,14 +340,26 @@ export class ElementsBreadcrumbs extends HTMLElement {
}
private ensureSelectedNodeIsVisible() {
if (!this.selectedDOMNode || !this.shadow || !this.overflowing) {
/*
* If the user has manually scrolled the crumbs in either direction, we
* effectively hand control over the scrolling down to them. This is to
* prevent the user manually scrolling to the end, and then us scrolling
* them back to the selected node. The moment they click either scroll
* button we set userHasManuallyScrolled, and we reset it when we get new
* data in. This means if the user clicks on a different element in the
* tree, we will auto-scroll that element into view, because we'll get new
* data and hence the flag will be reset.
*/
if (!this.selectedDOMNode || !this.shadow || !this.overflowing || this.userHasManuallyScrolled) {
return;
}
const activeCrumbId = this.selectedDOMNode.id;
const activeCrumb = this.shadow.querySelector(`.crumb[data-node-id="${activeCrumbId}"]`);
if (activeCrumb) {
activeCrumb.scrollIntoView();
activeCrumb.scrollIntoView({
behavior: 'smooth',
});
}
}
}
test/unittests/front_end/elements/ElementsBreadcrumbs_test.ts
+65 -1
View File
@@ -4,7 +4,7 @@
import {ElementsBreadcrumbs} from '../../../../front_end/elements/ElementsBreadcrumbs.js';
import {crumbsToRender, determineElementTitle, DOMNode} from '../../../../front_end/elements/ElementsBreadcrumbsUtils.js';
import {assertElement, assertElements, assertShadowRoot, dispatchClickEvent, renderElementIntoDOM, waitForScrollLeft} from '../helpers/DOMHelpers.js';
import {assertElement, assertElements, assertShadowRoot, dispatchClickEvent, doubleRaf, renderElementIntoDOM, waitForScrollLeft} from '../helpers/DOMHelpers.js';
import {withNoMutations} from '../helpers/MutationHelpers.js';
const {assert} = chai;
@@ -349,6 +349,70 @@ describe('ElementsBreadcrumbs', () => {
assert.isTrue(rightButton.disabled);
});
});
it('hides the overflow buttons should the user resize the window to be large enough', async () => {
const thinWrapper = document.createElement('div');
thinWrapper.style.width = '400px';
const component = new ElementsBreadcrumbs();
thinWrapper.appendChild(component);
renderElementIntoDOM(thinWrapper);
component.data = {
crumbs: [divCrumb, bodyCrumb],
selectedNode: bodyCrumb,
};
assertShadowRoot(component.shadowRoot);
const leftButton = component.shadowRoot.querySelector('button.overflow.left');
assertElement(leftButton, HTMLButtonElement);
const rightButton = component.shadowRoot.querySelector('button.overflow.right');
assertElement(rightButton, HTMLButtonElement);
assert.isFalse(leftButton.classList.contains('hidden'));
assert.isFalse(rightButton.classList.contains('hidden'));
thinWrapper.style.width = '800px';
// So the browser has time to paint
await doubleRaf();
assert.isTrue(leftButton.classList.contains('hidden'));
assert.isTrue(rightButton.classList.contains('hidden'));
});
it('shows the overflow buttons should the user resize the window down to be small', async () => {
const thinWrapper = document.createElement('div');
thinWrapper.style.width = '800px';
const component = new ElementsBreadcrumbs();
thinWrapper.appendChild(component);
renderElementIntoDOM(thinWrapper);
component.data = {
crumbs: [divCrumb, bodyCrumb],
selectedNode: bodyCrumb,
};
assertShadowRoot(component.shadowRoot);
const leftButton = component.shadowRoot.querySelector('button.overflow.left');
assertElement(leftButton, HTMLButtonElement);
const rightButton = component.shadowRoot.querySelector('button.overflow.right');
assertElement(rightButton, HTMLButtonElement);
assert.isTrue(leftButton.classList.contains('hidden'));
assert.isTrue(rightButton.classList.contains('hidden'));
thinWrapper.style.width = '400px';
// So the browser has time to paint
await doubleRaf();
assert.isFalse(leftButton.classList.contains('hidden'));
assert.isFalse(rightButton.classList.contains('hidden'));
});
});
});
test/unittests/front_end/helpers/DOMHelpers.ts
+4
View File
@@ -130,3 +130,7 @@ export function dispatchClickEvent<T extends Element>(element: T, options: Mouse
assert.fail('Failed to trigger click event successfully.');
}
}
export async function doubleRaf() {
await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve)));
}