Bug 1360457 - moved header into content, created wrapper around header, adapted css, fixed an old test, created new regression test. draft
authortera_1225@hotmail.com <tera_1225@hotmail.com>
Thu, 26 Oct 2017 19:41:10 +0200
changeset 689167 b4a682dac706b470c47f32d80c5c8cf954396252
parent 687040 7290c8fce8a1f5c4bce930d951eaf1d6326bd788
child 738248 ebd5fcba7f293b6f1acdafe74a533139212c1c62
push id86944
push userbmo:tera_1225@hotmail.com
push dateTue, 31 Oct 2017 06:32:18 +0000
bugs1360457
milestone58.0a1
Bug 1360457 - moved header into content, created wrapper around header, adapted css, fixed an old test, created new regression test. Post-review fixes applied x2 + lint MozReview-Commit-ID: AXNQFZC3fih
devtools/client/netmonitor/src/assets/styles/netmonitor.css
devtools/client/netmonitor/src/components/RequestList.js
devtools/client/netmonitor/src/components/RequestListContent.js
devtools/client/netmonitor/src/components/RequestListEmptyNotice.js
devtools/client/netmonitor/src/components/RequestListHeader.js
devtools/client/netmonitor/test/browser.ini
devtools/client/netmonitor/test/browser_net_autoscroll.js
devtools/client/netmonitor/test/browser_net_headers-alignment.js
--- a/devtools/client/netmonitor/src/assets/styles/netmonitor.css
+++ b/devtools/client/netmonitor/src/assets/styles/netmonitor.css
@@ -158,22 +158,27 @@ body,
 .status-bar-label.load {
   color: var(--theme-highlight-red);
 }
 
 /* Request list empty panel */
 
 .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;
   align-items: center;
 }
 
 .requests-list-perf-notice-button {
   min-width: 30px;
@@ -244,20 +249,29 @@ body,
 }
 
 .theme-firebug .requests-list-column {
   padding: 1px;
 }
 
 /* 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 {
   border-width: 0;
 }
 
 .requests-list-header-button {
   background-color: transparent;
@@ -737,16 +751,17 @@ body,
 
 .requests-list-timings-total:dir(rtl) {
   transform-origin: right center;
 }
 
 /* Request list item */
 
 .request-list-item {
+  position: relative;
   display: table-row;
   height: 24px;
 }
 
 .request-list-item.selected {
   background-color: var(--theme-selection-background);
   color: var(--theme-selection-color);
 }
--- a/devtools/client/netmonitor/src/components/RequestList.js
+++ b/devtools/client/netmonitor/src/components/RequestList.js
@@ -8,37 +8,34 @@ const {
   createFactory,
   DOM,
   PropTypes,
 } = require("devtools/client/shared/vendor/react");
 
 // Components
 const RequestListContent = createFactory(require("./RequestListContent"));
 const RequestListEmptyNotice = createFactory(require("./RequestListEmptyNotice"));
-const RequestListHeader = createFactory(require("./RequestListHeader"));
 const StatusBar = createFactory(require("./StatusBar"));
 
 const { div } = DOM;
 
 /**
  * Request panel component
  */
 function RequestList({
   connector,
   isEmpty,
 }) {
   return (
     div({ className: "request-list-container" },
-      RequestListHeader(),
       isEmpty ? RequestListEmptyNotice({connector}) : RequestListContent({connector}),
-      StatusBar({connector}),
+      StatusBar(),
     )
   );
 }
 
 RequestList.displayName = "RequestList";
 
 RequestList.propTypes = {
-  connector: PropTypes.object.isRequired,
   isEmpty: PropTypes.bool.isRequired,
 };
 
 module.exports = RequestList;
--- a/devtools/client/netmonitor/src/components/RequestListContent.js
+++ b/devtools/client/netmonitor/src/components/RequestListContent.js
@@ -15,16 +15,17 @@ const { HTMLTooltip } = require("devtool
 const Actions = require("../actions/index");
 const { setTooltipImageContent } = require("../request-list-tooltip");
 const {
   getDisplayedRequests,
   getWaterfallScale,
 } = require("../selectors/index");
 
 // Components
+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;
 
@@ -253,16 +254,17 @@ class RequestListContent extends Compone
       div({ className: "requests-list-wrapper"},
         div({ className: "requests-list-table"},
           div({
             ref: "contentEl",
             className: "requests-list-contents",
             tabIndex: 0,
             onKeyDown: this.onKeyDown,
           },
+            RequestListHeader(),
             displayedRequests.map((item, index) => RequestListItem({
               firstRequestStartedMillis,
               fromCache: item.status === "304" || item.fromCache,
               columns,
               item,
               index,
               isSelected: item.id === selectedRequestId,
               key: item.id,
--- a/devtools/client/netmonitor/src/components/RequestListEmptyNotice.js
+++ b/devtools/client/netmonitor/src/components/RequestListEmptyNotice.js
@@ -13,16 +13,17 @@ const {
 const { connect } = require("devtools/client/shared/vendor/react-redux");
 const Actions = require("../actions/index");
 const { ACTIVITY_TYPE } = require("../constants");
 const { L10N } = require("../utils/l10n");
 const { getPerformanceAnalysisURL } = require("../utils/mdn-utils");
 
 // Components
 const MDNLink = createFactory(require("./MdnLink"));
+const RequestListHeader = createFactory(require("./RequestListHeader"));
 
 const { button, div, span } = DOM;
 
 const RELOAD_NOTICE_1 = L10N.getStr("netmonitor.reloadNotice1");
 const RELOAD_NOTICE_2 = L10N.getStr("netmonitor.reloadNotice2");
 const RELOAD_NOTICE_3 = L10N.getStr("netmonitor.reloadNotice3");
 const PERFORMANCE_NOTICE_1 = L10N.getStr("netmonitor.perfNotice1");
 const PERFORMANCE_NOTICE_2 = L10N.getStr("netmonitor.perfNotice2");
@@ -41,17 +42,18 @@ class RequestListEmptyNotice extends Com
     };
   }
 
   render() {
     return div(
       {
         className: "request-list-empty-notice",
       },
-      div({ className: "notice-reload-message" },
+      RequestListHeader(),
+      div({ className: "notice-reload-message empty-notice-element" },
         span(null, RELOAD_NOTICE_1),
         button(
           {
             className: "devtools-button requests-list-reload-notice-button",
             "data-standalone": true,
             onClick: this.props.onReloadClick,
           },
           RELOAD_NOTICE_2
--- a/devtools/client/netmonitor/src/components/RequestListHeader.js
+++ b/devtools/client/netmonitor/src/components/RequestListHeader.js
@@ -99,57 +99,59 @@ class RequestListHeader extends Componen
         this.props.resizeWaterfall(waterfallHeader.getBoundingClientRect().width));
     }
   }
 
   render() {
     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" })
+                )
               )
-            )
-          );
-        })
+            );
+          })
+        )
       )
     );
   }
 }
 
 /**
  * Build the waterfall header - timing tick marks with the right spacing
  */
--- a/devtools/client/netmonitor/test/browser.ini
+++ b/devtools/client/netmonitor/test/browser.ini
@@ -114,16 +114,17 @@ skip-if = (os == 'linux' && debug && bit
 [browser_net_filter-01.js]
 skip-if = (os == 'linux' && debug && bits == 32) # Bug 1303439
 [browser_net_filter-02.js]
 [browser_net_filter-03.js]
 [browser_net_filter-04.js]
 [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]
 [browser_net_json-b64.js]
 [browser_net_json-null.js]
 [browser_net_json-long.js]
 [browser_net_json-malformed.js]
 [browser_net_json_custom_mime.js]
--- a/devtools/client/netmonitor/test/browser_net_autoscroll.js
+++ b/devtools/client/netmonitor/test/browser_net_autoscroll.js
@@ -1,15 +1,16 @@
 /* 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 { monitor } = yield initNetMonitor(INFINITE_GET_URL, true);
   let { document, windowRequire, store } = monitor.panelWin;
   let Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
 
@@ -22,42 +23,44 @@ add_task(function* () {
   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 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
   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 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);
 
   function waitForRequestListToAppear() {
     info("Waiting until the empty notice disappears and is replaced with the list");
     return waitUntil(() => !!document.querySelector(".requests-list-contents"));
   }
new file mode 100644
--- /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;
+      }
+    }
+  }
+});