Bug 1416714 - Remove request list scroll to bottom r?honza
MozReview-Commit-ID: 6yD63GIVwjQ
--- a/devtools/client/netmonitor/src/components/RequestListContent.js
+++ b/devtools/client/netmonitor/src/components/RequestListContent.js
@@ -23,18 +23,16 @@ const {
const RequestListHeader = createFactory(require("./RequestListHeader"));
const RequestListItem = createFactory(require("./RequestListItem"));
const RequestListContextMenu = require("../request-list-context-menu");
const { div } = DOM;
// Tooltip show / hide delay in ms
const REQUESTS_TOOLTIP_TOGGLE_DELAY = 500;
-// Gecko's scrollTop is int32_t, so the maximum value is 2^31 - 1 = 2147483647
-const MAX_SCROLL_HEIGHT = 2147483647;
/**
* Renders the actual contents of the request list.
*/
class RequestListContent extends Component {
static get propTypes() {
return {
connector: PropTypes.object.isRequired,
@@ -48,22 +46,20 @@ class RequestListContent extends Compone
onWaterfallMouseDown: PropTypes.func.isRequired,
scale: PropTypes.number,
selectedRequestId: PropTypes.string,
};
}
constructor(props) {
super(props);
- this.isScrolledToBottom = this.isScrolledToBottom.bind(this);
this.onHover = this.onHover.bind(this);
this.onScroll = this.onScroll.bind(this);
this.onKeyDown = this.onKeyDown.bind(this);
this.onContextMenu = this.onContextMenu.bind(this);
- this.onFocusedNodeChange = this.onFocusedNodeChange.bind(this);
}
componentWillMount() {
const { dispatch, connector } = this.props;
this.contextMenu = new RequestListContextMenu({
cloneSelectedRequest: () => dispatch(Actions.cloneSelectedRequest()),
getTabTarget: connector.getTabTarget,
getLongString: connector.getLongString,
@@ -79,53 +75,24 @@ class RequestListContent extends Compone
toggleDelay: REQUESTS_TOOLTIP_TOGGLE_DELAY,
interactive: true
});
// Install event handler to hide the tooltip on scroll
this.refs.contentEl.addEventListener("scroll", this.onScroll, true);
}
- componentWillUpdate(nextProps) {
- // Check if the list is scrolled to bottom before the UI update.
- // The scroll is ever needed only if new rows are added to the list.
- const delta = nextProps.displayedRequests.size - this.props.displayedRequests.size;
- this.shouldScrollBottom = delta > 0 && this.isScrolledToBottom();
- }
-
- componentDidUpdate(prevProps) {
- let node = this.refs.contentEl;
- // Keep the list scrolled to bottom if a new row was added
- if (this.shouldScrollBottom && node.scrollTop !== MAX_SCROLL_HEIGHT) {
- // Using maximum scroll height rather than node.scrollHeight to avoid sync reflow.
- node.scrollTop = MAX_SCROLL_HEIGHT;
- }
- }
-
componentWillUnmount() {
+ // Uninstall scroll event handler
this.refs.contentEl.removeEventListener("scroll", this.onScroll, true);
// Uninstall the tooltip event handler
this.tooltip.stopTogglingOnHover();
}
- isScrolledToBottom() {
- const { contentEl } = this.refs;
- const lastChildEl = contentEl.lastElementChild;
-
- if (!lastChildEl) {
- return false;
- }
-
- let lastChildRect = lastChildEl.getBoundingClientRect();
- let contentRect = contentEl.getBoundingClientRect();
-
- return (lastChildRect.height + lastChildRect.top) <= contentRect.bottom;
- }
-
/**
* The predicate used when deciding whether a popup should be shown
* over a request item or not.
*
* @param nsIDOMNode target
* The element node currently being hovered.
* @param object tooltip
* The current tooltip instance.
@@ -198,56 +165,47 @@ class RequestListContent extends Compone
}
}
onContextMenu(evt) {
evt.preventDefault();
this.contextMenu.open(evt);
}
- /**
- * If selection has just changed (by keyboard navigation), don't keep the list
- * scrolled to bottom, but allow scrolling up with the selection.
- */
- onFocusedNodeChange() {
- this.shouldScrollBottom = false;
- }
-
render() {
const {
columns,
displayedRequests,
firstRequestStartedMillis,
onItemMouseDown,
onWaterfallMouseDown,
scale,
selectedRequestId,
} = this.props;
return (
- div({ className: "requests-list-wrapper"},
- div({ className: "requests-list-table"},
+ div({ className: "requests-list-wrapper" },
+ div({ className: "requests-list-table" },
div({
ref: "contentEl",
className: "requests-list-contents",
tabIndex: 0,
onKeyDown: this.onKeyDown,
- style: {"--timings-scale": scale, "--timings-rev-scale": 1 / scale}
+ style: { "--timings-scale": scale, "--timings-rev-scale": 1 / scale }
},
RequestListHeader(),
displayedRequests.map((item, index) => RequestListItem({
firstRequestStartedMillis,
fromCache: item.status === "304" || item.fromCache,
columns,
item,
index,
isSelected: item.id === selectedRequestId,
key: item.id,
onContextMenu: this.onContextMenu,
- onFocusedNodeChange: this.onFocusedNodeChange,
onMouseDown: () => onItemMouseDown(item.id),
onWaterfallMouseDown: () => onWaterfallMouseDown(),
}))
)
)
)
);
}
--- a/devtools/client/netmonitor/src/components/RequestListItem.js
+++ b/devtools/client/netmonitor/src/components/RequestListItem.js
@@ -77,17 +77,16 @@ class RequestListItem extends Component
return {
columns: PropTypes.object.isRequired,
item: PropTypes.object.isRequired,
index: PropTypes.number.isRequired,
isSelected: PropTypes.bool.isRequired,
firstRequestStartedMillis: PropTypes.number.isRequired,
fromCache: PropTypes.bool,
onContextMenu: PropTypes.func.isRequired,
- onFocusedNodeChange: PropTypes.func,
onMouseDown: PropTypes.func.isRequired,
onWaterfallMouseDown: PropTypes.func.isRequired,
waterfallWidth: PropTypes.number,
};
}
componentDidMount() {
if (this.props.isSelected) {
@@ -99,19 +98,16 @@ class RequestListItem extends Component
return !propertiesEqual(UPDATED_REQ_ITEM_PROPS, this.props.item, nextProps.item) ||
!propertiesEqual(UPDATED_REQ_PROPS, this.props, nextProps) ||
!I.is(this.props.columns, nextProps.columns);
}
componentDidUpdate(prevProps) {
if (!prevProps.isSelected && this.props.isSelected) {
this.refs.listItem.focus();
- if (this.props.onFocusedNodeChange) {
- this.props.onFocusedNodeChange();
- }
}
}
render() {
let {
columns,
item,
index,
--- a/devtools/client/netmonitor/test/browser.ini
+++ b/devtools/client/netmonitor/test/browser.ini
@@ -56,17 +56,16 @@ support-files =
!/devtools/client/framework/test/shared-head.js
xhr_bundle.js
xhr_bundle.js.map
xhr_original.js
[browser_net_accessibility-01.js]
[browser_net_accessibility-02.js]
[browser_net_api-calls.js]
-[browser_net_autoscroll.js]
[browser_net_cached-status.js]
[browser_net_cause.js]
[browser_net_cause_redirect.js]
[browser_net_cause_source_map.js]
[browser_net_service-worker-status.js]
[browser_net_charts-01.js]
[browser_net_charts-02.js]
[browser_net_charts-03.js]
deleted file mode 100644
--- a/devtools/client/netmonitor/test/browser_net_autoscroll.js
+++ /dev/null
@@ -1,100 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
- http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-/**
- * Bug 863102 - Automatically scroll down upon new network requests.
- * edited to account for changes made to fix Bug 1360457
- */
-add_task(function* () {
- requestLongerTimeout(4);
-
- let { tab, monitor } = yield initNetMonitor(INFINITE_GET_URL, true);
- let { document, windowRequire, store } = monitor.panelWin;
- let Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
-
- store.dispatch(Actions.batchEnable(false));
-
- // Wait until the first request makes the empty notice disappear
- yield waitForRequestListToAppear();
-
- let requestsContainer = document.querySelector(".requests-list-contents");
- ok(requestsContainer, "Container element exists as expected.");
-
- // (1) Check that the scroll position is maintained at the bottom
- // when the requests overflow the vertical size of the container.
- yield waitForRequestsToOverflowContainer();
- yield waitForScroll();
- ok(true, "Scrolled to bottom on overflow.");
-
- // (2) Now scroll to the top and check that additional requests
- // do not change the scroll position.
- requestsContainer.scrollTop = 0;
- yield waitSomeTime();
- ok(!scrolledToBottom(requestsContainer), "Not scrolled to bottom.");
- // save for comparison later
- let scrollTop = requestsContainer.scrollTop;
- yield waitForNetworkEvents(monitor, 8);
- yield waitSomeTime();
- is(requestsContainer.scrollTop, scrollTop, "Did not scroll.");
-
- // (3) Now set the scroll position back at the bottom and check that
- // additional requests *do* cause the container to scroll down.
- requestsContainer.scrollTop = requestsContainer.scrollHeight;
- ok(scrolledToBottom(requestsContainer), "Set scroll position to bottom.");
- yield waitForNetworkEvents(monitor, 8);
- yield waitForScroll();
- ok(true, "Still scrolled to bottom.");
-
- // (4) Now select the first item in the list
- // and check that additional requests do not change the scroll position
- // from just below the headers.
- store.dispatch(Actions.selectRequestByIndex(0));
- yield waitForNetworkEvents(monitor, 8);
- yield waitSomeTime();
- let requestsContainerHeaders = requestsContainer.firstChild;
- let headersHeight = requestsContainerHeaders.offsetHeight;
- is(requestsContainer.scrollTop, headersHeight, "Did not scroll.");
-
- // Stop doing requests.
- yield ContentTask.spawn(tab.linkedBrowser, {}, function () {
- content.wrappedJSObject.stopRequests();
- });
-
- // Done: clean up.
- return teardown(monitor);
-
- function waitForRequestListToAppear() {
- info("Waiting until the empty notice disappears and is replaced with the list");
- return waitUntil(() => !!document.querySelector(".requests-list-contents"));
- }
-
- function* waitForRequestsToOverflowContainer() {
- info("Waiting for enough requests to overflow the container");
- while (true) {
- info("Waiting for one network request");
- yield waitForNetworkEvents(monitor, 1);
- console.log(requestsContainer.scrollHeight);
- console.log(requestsContainer.clientHeight);
- if (requestsContainer.scrollHeight > requestsContainer.clientHeight) {
- info("The list is long enough, returning");
- return;
- }
- }
- }
-
- function scrolledToBottom(element) {
- return element.scrollTop + element.clientHeight >= element.scrollHeight;
- }
-
- function waitSomeTime() {
- // Wait to make sure no scrolls happen
- return wait(50);
- }
-
- function waitForScroll() {
- info("Waiting for the list to scroll to bottom");
- return waitUntil(() => scrolledToBottom(requestsContainer));
- }
-});