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
2017-10-26 19:41:10 +02:00 · 2017-10-26 19:41:10 +02:00 · 487c19c1d8
--- 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;
 }
--- 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,
 };

--- 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,
--- 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",
--- 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" })
+                )
              )
-            )
-          );
-        })
+            );
+          })
+        )
      )
    );
  }
--- 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]
--- 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);
--- a/devtools/client/netmonitor/test/browser_net_headers-alignment.js
+++ 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;
+      }
+    }
+  }
+});