Bug 1485125 - Lazy load the Device Modal in RDM. r=miker
authorGabriel Luong <gabriel.luong@gmail.com>
Tue, 21 Aug 2018 22:03:09 -0400
changeset 481021 3e4d7495204cf054f5083506f02a93e787956796
parent 481020 569397778fcde1ff2243e7d83a263ce2f51674f0
child 481022 809610242278ab22abde54134f7ec6520d44b0b9
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersmiker
bugs1485125
milestone63.0a1
Bug 1485125 - Lazy load the Device Modal in RDM. r=miker
devtools/client/responsive.html/components/App.js
devtools/client/responsive.html/components/DeviceAdder.js
devtools/client/responsive.html/components/DeviceModal.js
devtools/client/responsive.html/test/browser/browser_device_modal_exit.js
devtools/client/responsive.html/test/browser/browser_device_modal_submit.js
devtools/client/responsive.html/test/browser/head.js
--- a/devtools/client/responsive.html/components/App.js
+++ b/devtools/client/responsive.html/components/App.js
@@ -6,20 +6,22 @@
 
 "use strict";
 
 const { Component, createFactory } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const { connect } = require("devtools/client/shared/vendor/react-redux");
 
-const DeviceModal = createFactory(require("./DeviceModal"));
 const Toolbar = createFactory(require("./Toolbar"));
 const Viewports = createFactory(require("./Viewports"));
 
+loader.lazyGetter(this, "DeviceModal",
+  () => createFactory(require("./DeviceModal")));
+
 const {
   addCustomDevice,
   removeCustomDevice,
   updateDeviceDisplayed,
   updateDeviceModal,
   updatePreferredDevices,
 } = require("../actions/devices");
 const { changeNetworkThrottling } = require("devtools/client/shared/components/throttling/actions");
@@ -250,22 +252,25 @@ class App extends Component {
       Viewports({
         screenshot,
         viewports,
         onBrowserMounted,
         onContentResize,
         onRemoveDeviceAssociation,
         onResizeViewport,
       }),
-      DeviceModal({
-        deviceAdderViewportTemplate,
-        devices,
-        onAddCustomDevice,
-        onDeviceListUpdate,
-        onRemoveCustomDevice,
-        onUpdateDeviceDisplayed,
-        onUpdateDeviceModal,
-      })
+      devices.isModalOpen ?
+        DeviceModal({
+          deviceAdderViewportTemplate,
+          devices,
+          onAddCustomDevice,
+          onDeviceListUpdate,
+          onRemoveCustomDevice,
+          onUpdateDeviceDisplayed,
+          onUpdateDeviceModal,
+        })
+        :
+        null
     );
   }
 }
 
 module.exports = connect(state => state)(App);
--- a/devtools/client/responsive.html/components/DeviceAdder.js
+++ b/devtools/client/responsive.html/components/DeviceAdder.js
@@ -22,35 +22,32 @@ class DeviceAdder extends PureComponent 
       viewportTemplate: PropTypes.shape(Types.viewport).isRequired,
       onAddCustomDevice: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
 
-    this.state = {};
+    const {
+      height,
+      width,
+    } = this.props.viewportTemplate;
+
+    this.state = {
+      deviceAdderDisplayed: false,
+      height,
+      width,
+    };
 
     this.onChangeSize = this.onChangeSize.bind(this);
     this.onDeviceAdderShow = this.onDeviceAdderShow.bind(this);
     this.onDeviceAdderSave = this.onDeviceAdderSave.bind(this);
   }
 
-  componentWillReceiveProps(nextProps) {
-    const {
-      width,
-      height,
-    } = nextProps.viewportTemplate;
-
-    this.setState({
-      width,
-      height,
-    });
-  }
-
   onChangeSize(_, width, height) {
     this.setState({
       width,
       height,
     });
   }
 
   onDeviceAdderShow() {
@@ -63,24 +60,26 @@ class DeviceAdder extends PureComponent 
     const {
       devices,
       onAddCustomDevice,
     } = this.props;
 
     if (!this.pixelRatioInput.checkValidity()) {
       return;
     }
+
     if (devices.custom.find(device => device.name == this.nameInput.value)) {
       this.nameInput.setCustomValidity("Device name already in use");
       return;
     }
 
     this.setState({
       deviceAdderDisplayed: false,
     });
+
     onAddCustomDevice({
       name: this.nameInput.value,
       width: this.state.width,
       height: this.state.height,
       pixelRatio: parseFloat(this.pixelRatioInput.value),
       userAgent: this.userAgentInput.value,
       touch: this.touchInput.checked,
     });
--- a/devtools/client/responsive.html/components/DeviceModal.js
+++ b/devtools/client/responsive.html/components/DeviceModal.js
@@ -27,47 +27,32 @@ class DeviceModal extends PureComponent 
       onUpdateDeviceModal: PropTypes.func.isRequired,
     };
   }
 
   constructor(props) {
     super(props);
 
     this.state = {};
+    for (const type of this.props.devices.types) {
+      for (const device of this.props.devices[type]) {
+        this.state[device.name] = device.displayed;
+      }
+    }
 
     this.onAddCustomDevice = this.onAddCustomDevice.bind(this);
     this.onDeviceCheckboxChange = this.onDeviceCheckboxChange.bind(this);
     this.onDeviceModalSubmit = this.onDeviceModalSubmit.bind(this);
     this.onKeyDown = this.onKeyDown.bind(this);
   }
 
   componentDidMount() {
     window.addEventListener("keydown", this.onKeyDown, true);
   }
 
-  componentWillReceiveProps(nextProps) {
-    const {
-      devices: oldDevices,
-    } = this.props;
-    const {
-      devices,
-    } = nextProps;
-
-    // Refresh component state only when model transitions from closed to open
-    if (!oldDevices.isModalOpen && devices.isModalOpen) {
-      for (const type of devices.types) {
-        for (const device of devices[type]) {
-          this.setState({
-            [device.name]: device.displayed,
-          });
-        }
-      }
-    }
-  }
-
   componentWillUnmount() {
     window.removeEventListener("keydown", this.onKeyDown, true);
   }
 
   onAddCustomDevice(device) {
     this.props.onAddCustomDevice(device);
     // Default custom devices to enabled
     this.setState({
--- a/devtools/client/responsive.html/test/browser/browser_device_modal_exit.js
+++ b/devtools/client/responsive.html/test/browser/browser_device_modal_exit.js
@@ -4,42 +4,37 @@ http://creativecommons.org/publicdomain/
 "use strict";
 
 // Test submitting display device changes on the device modal
 
 const TEST_URL = "data:text/html;charset=utf-8,";
 const Types = require("devtools/client/responsive.html/types");
 
 addRDMTask(TEST_URL, async function({ ui }) {
-  const { store, document } = ui.toolWindow;
-  const modal = document.querySelector("#device-modal-wrapper");
-  const closeButton = document.querySelector("#device-close-button");
+  const { document, store } = ui.toolWindow;
 
   // Wait until the viewport has been added and the device list has been loaded
   await waitUntilState(store, state => state.viewports.length == 1
     && state.devices.listState == Types.loadableState.LOADED);
 
   await openDeviceModal(ui);
 
   const preferredDevicesBefore = _loadPreferredDevices();
 
   info("Check the first unchecked device and exit the modal.");
   const uncheckedCb = [...document.querySelectorAll(".device-input-checkbox")]
     .filter(cb => !cb.checked)[0];
   const value = uncheckedCb.value;
   uncheckedCb.click();
-  closeButton.click();
+  document.getElementById("device-close-button").click();
 
-  ok(modal.classList.contains("closed") && !modal.classList.contains("opened"),
-    "The device modal is closed on exit.");
+  ok(!store.getState().devices.isModalOpen, "The device modal is closed on exit.");
 
   info("Check that the device list remains unchanged after exitting.");
   const preferredDevicesAfter = _loadPreferredDevices();
 
   is(preferredDevicesBefore.added.size, preferredDevicesAfter.added.size,
     "Got expected number of added devices.");
-
   is(preferredDevicesBefore.removed.size, preferredDevicesAfter.removed.size,
     "Got expected number of removed devices.");
-
   ok(!preferredDevicesAfter.removed.has(value),
     value + " was not added to removed device list.");
 });
--- a/devtools/client/responsive.html/test/browser/browser_device_modal_submit.js
+++ b/devtools/client/responsive.html/test/browser/browser_device_modal_submit.js
@@ -18,20 +18,18 @@ const addedDevice = {
   "featured": true,
 };
 
 const TEST_URL = "data:text/html;charset=utf-8,";
 const Types = require("devtools/client/responsive.html/types");
 
 addRDMTask(TEST_URL, async function({ ui }) {
   const { toolWindow } = ui;
-  const { store, document } = toolWindow;
+  const { document, store } = toolWindow;
   const deviceSelector = document.getElementById("device-selector");
-  const modal = document.getElementById("device-modal-wrapper");
-  const submitButton = document.getElementById("device-submit-button");
 
   // Wait until the viewport has been added and the device list has been loaded
   await waitUntilState(store, state => state.viewports.length == 1
     && state.devices.listState == Types.loadableState.LOADED);
 
   await openDeviceModal(ui);
 
   info("Checking displayed device checkboxes are checked in the device modal.");
@@ -55,47 +53,47 @@ addRDMTask(TEST_URL, async function({ ui
   }
 
   // Tests where the user adds a non-featured device
   info("Check the first unchecked device and submit new device list.");
   const uncheckedCb = [...document.querySelectorAll(".device-input-checkbox")]
     .filter(cb => !cb.checked)[0];
   const value = uncheckedCb.value;
   uncheckedCb.click();
-  submitButton.click();
+  document.getElementById("device-submit-button").click();
 
-  ok(modal.classList.contains("closed") && !modal.classList.contains("opened"),
-    "The device modal is closed on submit.");
+  ok(!store.getState().devices.isModalOpen, "The device modal is closed on submit.");
 
   info("Checking that the new device is added to the user preference list.");
   let preferredDevices = _loadPreferredDevices();
   ok(preferredDevices.added.has(value), value + " in user added list.");
 
   info("Checking new device is added to the device selector.");
   await testMenuItems(toolWindow, deviceSelector, menuItems => {
     is(menuItems.length - 2, featuredCount + 1,
       "Got expected number of devices in device selector.");
 
     const menuItem = menuItems.find(item => item.getAttribute("label") === name);
     ok(menuItem, value + " added to the device selector.");
   });
 
   info("Reopen device modal and check new device is correctly checked");
   await openDeviceModal(ui);
+
   ok([...document.querySelectorAll(".device-input-checkbox")]
     .filter(cb => cb.checked && cb.value === value)[0],
     value + " is checked in the device modal.");
 
   // Tests where the user removes a featured device
   info("Uncheck the first checked device different than the previous one");
   const checkedCb = [...document.querySelectorAll(".device-input-checkbox")]
     .filter(cb => cb.checked && cb.value != value)[0];
   const checkedVal = checkedCb.value;
   checkedCb.click();
-  submitButton.click();
+  document.getElementById("device-submit-button").click();
 
   info("Checking that the device is removed from the user preference list.");
   preferredDevices = _loadPreferredDevices();
   ok(preferredDevices.removed.has(checkedVal), checkedVal + " in removed list");
 
   info("Checking that the device is not in the device selector.");
   await testMenuItems(toolWindow, deviceSelector, menuItems => {
     is(menuItems.length - 2, featuredCount,
@@ -112,17 +110,17 @@ addRDMTask(TEST_URL, async function({ ui
     checkedVal + " is unchecked in the device modal.");
 
   // Let's add a dummy device to simulate featured flag changes for next test
   addDeviceForTest(addedDevice);
 });
 
 addRDMTask(TEST_URL, async function({ ui }) {
   const { toolWindow } = ui;
-  const { store, document } = toolWindow;
+  const { document, store } = toolWindow;
 
   // Wait until the viewport has been added and the device list has been loaded
   await waitUntilState(store, state => state.viewports.length == 1
     && state.devices.listState == Types.loadableState.LOADED);
 
   await openDeviceModal(ui);
 
   const remoteList = await getDevices();
--- a/devtools/client/responsive.html/test/browser/head.js
+++ b/devtools/client/responsive.html/test/browser/head.js
@@ -223,26 +223,24 @@ async function testViewportResize(ui, se
   const endRect = getElRect(selector, win);
   is(endRect.left - startRect.left, expectedHandleMove[0],
     `The x move of ${selector} is as expected`);
   is(endRect.top - startRect.top, expectedHandleMove[1],
     `The y move of ${selector} is as expected`);
 }
 
 async function openDeviceModal(ui) {
-  const { document } = ui.toolWindow;
-  const modal = document.getElementById("device-modal-wrapper");
-
-  info("Checking initial device modal state");
-  ok(modal.classList.contains("closed") && !modal.classList.contains("opened"),
-    "The device modal is closed by default.");
+  const { document, store } = ui.toolWindow;
 
   info("Opening device modal through device selector.");
+  const onModalOpen = waitUntilState(store, state => state.devices.isModalOpen);
   await selectMenuItem(ui, "#device-selector", getStr("responsive.editDeviceList2"));
+  await onModalOpen;
 
+  const modal = document.getElementById("device-modal-wrapper");
   ok(modal.classList.contains("opened") && !modal.classList.contains("closed"),
     "The device modal is displayed.");
 }
 
 async function selectMenuItem({ toolWindow }, selector, value) {
   const { document } = toolWindow;
 
   const button = document.querySelector(selector);
@@ -409,17 +407,17 @@ async function testUserAgentFromBrowser(
 
 /**
  * Assuming the device modal is open and the device adder form is shown, this helper
  * function adds `device` via the form, saves it, and waits for it to appear in the store.
  */
 function addDeviceInModal(ui, device) {
   const { Simulate } =
     ui.toolWindow.require("devtools/client/shared/vendor/react-dom-test-utils");
-  const { store, document } = ui.toolWindow;
+  const { document, store } = ui.toolWindow;
 
   const nameInput = document.querySelector("#device-adder-name input");
   const [ widthInput, heightInput ] =
     document.querySelectorAll("#device-adder-size input");
   const pixelRatioInput = document.querySelector("#device-adder-pixel-ratio input");
   const userAgentInput = document.querySelector("#device-adder-user-agent input");
   const touchInput = document.querySelector("#device-adder-touch input");