From 487c19c1d873743af6e5076e54b9a0bd4a0c79f2 Mon Sep 17 00:00:00 2001 From: "tera_1225@hotmail.com" Date: Thu, 26 Oct 2017 19:41:10 +0200 Subject: [PATCH] Bug 1360457 - moved header into content, created wrapper around header, adapted css, fixed an old test, created new regression test. r=ntim,rickychien Post-review fixes applied x2 MozReview-Commit-ID: AXNQFZC3fih --HG-- extra : rebase_source : 42411ab50219f7cdfa2e5da0a031c7f448149d0b --- .../src/assets/styles/netmonitor.css | 19 ++++- .../netmonitor/src/components/RequestList.js | 9 +-- .../src/components/RequestListContent.js | 2 + .../src/components/RequestListEmptyNotice.js | 6 +- .../src/components/RequestListHeader.js | 76 ++++++++++--------- devtools/client/netmonitor/test/browser.ini | 1 + .../netmonitor/test/browser_net_autoscroll.js | 17 +++-- .../test/browser_net_headers-alignment.js | 64 ++++++++++++++++ 8 files changed, 139 insertions(+), 55 deletions(-) create mode 100644 devtools/client/netmonitor/test/browser_net_headers-alignment.js diff --git a/devtools/client/netmonitor/src/assets/styles/netmonitor.css b/devtools/client/netmonitor/src/assets/styles/netmonitor.css index be905948d8ad..f9be3435ac51 100644 --- a/devtools/client/netmonitor/src/assets/styles/netmonitor.css +++ b/devtools/client/netmonitor/src/assets/styles/netmonitor.css @@ -163,12 +163,17 @@ body, .request-list-empty-notice { margin: 0; - padding: 12px; - font-size: 120%; flex: 1; overflow: auto; } +.empty-notice-element { + padding-top: 12px; + padding-left: 12px; + padding-right: 12px; + font-size: 1.2rem; +} + .notice-perf-message { margin-top: 2px; display: flex; @@ -249,10 +254,19 @@ body, /* Requests list headers */ +.requests-list-headers-wrapper { + position: sticky; + top: 0; + z-index: 1000; + width: 100%; + padding: 0; +} + .requests-list-headers { display: table-header-group; height: 24px; padding: 0; + width: 100%; } .requests-list-headers .requests-list-column:first-child .requests-list-header-button { @@ -742,6 +756,7 @@ body, /* Request list item */ .request-list-item { + position: relative; display: table-row; height: 24px; } diff --git a/devtools/client/netmonitor/src/components/RequestList.js b/devtools/client/netmonitor/src/components/RequestList.js index 21057bc64449..75d110376451 100644 --- a/devtools/client/netmonitor/src/components/RequestList.js +++ b/devtools/client/netmonitor/src/components/RequestList.js @@ -21,15 +21,11 @@ const { div } = DOM; /** * Request panel component */ -function RequestList({ - connector, - isEmpty, -}) { +function RequestList({ isEmpty }) { return ( div({ className: "request-list-container" }, - RequestListHeader(), isEmpty ? RequestListEmptyNotice({connector}) : RequestListContent({connector}), - StatusBar({connector}), + StatusBar(), ) ); } @@ -37,7 +33,6 @@ function RequestList({ RequestList.displayName = "RequestList"; RequestList.propTypes = { - connector: PropTypes.object.isRequired, isEmpty: PropTypes.bool.isRequired, }; diff --git a/devtools/client/netmonitor/src/components/RequestListContent.js b/devtools/client/netmonitor/src/components/RequestListContent.js index 5320ddee59a7..2dacc72bc719 100644 --- a/devtools/client/netmonitor/src/components/RequestListContent.js +++ b/devtools/client/netmonitor/src/components/RequestListContent.js @@ -20,6 +20,7 @@ const { } = require("../selectors/index"); // Components +const RequestListHeader = createFactory(require("./RequestListHeader")); const RequestListItem = createFactory(require("./RequestListItem")); const RequestListContextMenu = require("../request-list-context-menu"); @@ -258,6 +259,7 @@ class RequestListContent extends Component { tabIndex: 0, onKeyDown: this.onKeyDown, }, + RequestListHeader(), displayedRequests.map((item, index) => RequestListItem({ firstRequestStartedMillis, fromCache: item.status === "304" || item.fromCache, diff --git a/devtools/client/netmonitor/src/components/RequestListEmptyNotice.js b/devtools/client/netmonitor/src/components/RequestListEmptyNotice.js index 3a87cddcfc6c..6311f1ba732c 100644 --- a/devtools/client/netmonitor/src/components/RequestListEmptyNotice.js +++ b/devtools/client/netmonitor/src/components/RequestListEmptyNotice.js @@ -18,6 +18,7 @@ const { getPerformanceAnalysisURL } = require("../utils/mdn-utils"); // Components const MDNLink = createFactory(require("./MdnLink")); +const RequestListHeader = createFactory(require("./RequestListHeader")); const { button, div, span } = DOM; @@ -46,8 +47,9 @@ class RequestListEmptyNotice extends Component { { className: "request-list-empty-notice", }, - div({ className: "notice-reload-message" }, - span(null, RELOAD_NOTICE_1), + RequestListHeader(), + div({ className: "notice-reload-message empty-notice-element" }, + span(null, L10N.getStr("netmonitor.reloadNotice1")), button( { className: "devtools-button requests-list-reload-notice-button", diff --git a/devtools/client/netmonitor/src/components/RequestListHeader.js b/devtools/client/netmonitor/src/components/RequestListHeader.js index 8a88a1d452c3..15161f1b2a7b 100644 --- a/devtools/client/netmonitor/src/components/RequestListHeader.js +++ b/devtools/client/netmonitor/src/components/RequestListHeader.js @@ -104,47 +104,49 @@ class RequestListHeader extends Component { let { columns, scale, sort, sortBy, waterfallWidth } = this.props; return ( - div({ className: "devtools-toolbar requests-list-headers" }, - HEADERS.filter((header) => columns.get(header.name)).map((header) => { - let name = header.name; - let boxName = header.boxName || name; - let label = header.noLocalization - ? name : L10N.getStr(`netmonitor.toolbar.${header.label || name}`); - let sorted, sortedTitle; - let active = sort.type == name ? true : undefined; + div({ className: "devtools-toolbar requests-list-headers-wrapper" }, + div({ className: "devtools-toolbar requests-list-headers" }, + HEADERS.filter((header) => columns.get(header.name)).map((header) => { + let name = header.name; + let boxName = header.boxName || name; + let label = header.noLocalization + ? name : L10N.getStr(`netmonitor.toolbar.${header.label || name}`); + let sorted, sortedTitle; + let active = sort.type == name ? true : undefined; - if (active) { - sorted = sort.ascending ? "ascending" : "descending"; - sortedTitle = L10N.getStr(sort.ascending - ? "networkMenu.sortedAsc" - : "networkMenu.sortedDesc"); - } + if (active) { + sorted = sort.ascending ? "ascending" : "descending"; + sortedTitle = L10N.getStr(sort.ascending + ? "networkMenu.sortedAsc" + : "networkMenu.sortedDesc"); + } - return ( - div({ - id: `requests-list-${boxName}-header-box`, - className: `requests-list-column requests-list-${boxName}`, - key: name, - ref: `${name}Header`, - // Used to style the next column. - "data-active": active, - onContextMenu: this.onContextMenu, - }, - button({ - id: `requests-list-${name}-button`, - className: `requests-list-header-button`, - "data-sorted": sorted, - title: sortedTitle ? `${label} (${sortedTitle})` : label, - onClick: () => sortBy(name), + return ( + div({ + id: `requests-list-${boxName}-header-box`, + className: `requests-list-column requests-list-${boxName}`, + key: name, + ref: `${name}Header`, + // Used to style the next column. + "data-active": active, + onContextMenu: this.onContextMenu, }, - name === "waterfall" - ? WaterfallLabel(waterfallWidth, scale, label) - : div({ className: "button-text" }, label), - div({ className: "button-icon" }) + button({ + id: `requests-list-${name}-button`, + className: `requests-list-header-button`, + "data-sorted": sorted, + title: sortedTitle ? `${label} (${sortedTitle})` : label, + onClick: () => sortBy(name), + }, + name === "waterfall" + ? WaterfallLabel(waterfallWidth, scale, label) + : div({ className: "button-text" }, label), + div({ className: "button-icon" }) + ) ) - ) - ); - }) + ); + }) + ) ) ); } diff --git a/devtools/client/netmonitor/test/browser.ini b/devtools/client/netmonitor/test/browser.ini index 17dfa87d4795..b52cd18478b7 100644 --- a/devtools/client/netmonitor/test/browser.ini +++ b/devtools/client/netmonitor/test/browser.ini @@ -119,6 +119,7 @@ skip-if = (os == 'linux' && debug && bits == 32) # Bug 1303439 [browser_net_filter-autocomplete.js] [browser_net_filter-flags.js] [browser_net_footer-summary.js] +[browser_net_headers-alignment.js] [browser_net_headers_sorted.js] [browser_net_icon-preview.js] [browser_net_image-tooltip.js] diff --git a/devtools/client/netmonitor/test/browser_net_autoscroll.js b/devtools/client/netmonitor/test/browser_net_autoscroll.js index 2d0a06832110..360ca5746238 100644 --- a/devtools/client/netmonitor/test/browser_net_autoscroll.js +++ b/devtools/client/netmonitor/test/browser_net_autoscroll.js @@ -5,6 +5,7 @@ /** * Bug 863102 - Automatically scroll down upon new network requests. + * edited to account for changes made to fix Bug 1360457 */ add_task(function* () { requestLongerTimeout(4); @@ -27,10 +28,9 @@ add_task(function* () { yield waitForScroll(); ok(true, "Scrolled to bottom on overflow."); - // (2) Now set the scroll position to the first item and check - // that additional requests do not change the scroll position. - let firstNode = requestsContainer.firstChild; - firstNode.scrollIntoView(); + // (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 @@ -47,12 +47,15 @@ add_task(function* () { yield waitForScroll(); ok(true, "Still scrolled to bottom."); - // (4) Now select an item in the list and check that additional requests - // do not change the scroll position. + // (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(); - is(requestsContainer.scrollTop, 0, "Did not scroll."); + let requestsContainerHeaders = requestsContainer.firstChild; + let headersHeight = requestsContainerHeaders.offsetHeight; + is(requestsContainer.scrollTop, headersHeight, "Did not scroll."); // Done: clean up. return teardown(monitor); diff --git a/devtools/client/netmonitor/test/browser_net_headers-alignment.js b/devtools/client/netmonitor/test/browser_net_headers-alignment.js new file mode 100644 index 000000000000..f9389015e309 --- /dev/null +++ b/devtools/client/netmonitor/test/browser_net_headers-alignment.js @@ -0,0 +1,64 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +/** + * Bug 1360457 - Mis-alignment between headers and columns on overflow + */ + +add_task(function* () { + requestLongerTimeout(4); + + let { 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."); + let headers = document.querySelector(".requests-list-headers"); + ok(headers, "Headers element exists as expected."); + + yield waitForRequestsToOverflowContainer(); + + // Get first request line, not child 0 as this is the headers + let firstRequestLine = requestsContainer.childNodes[1]; + + // Find number of columns + let numberOfColumns = headers.childElementCount; + for (let columnNumber = 0; columnNumber < numberOfColumns; columnNumber++) { + let aHeaderColumn = headers.childNodes[columnNumber]; + let aRequestColumn = firstRequestLine.childNodes[columnNumber]; + is(aHeaderColumn.getBoundingClientRect().left, + aRequestColumn.getBoundingClientRect().left, + "Headers for columns number " + columnNumber + " are aligned." + ); + } + + // 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; + } + } + } +});