1006875-graph-styling-web-audio-editor.patch

From e35ac4a0618985e2743c34e8a26ea902e8c2c9c0 Mon Sep 17 00:00:00 2001
From: Jordan Santell <jsantell@gmail.com>
Date: Mon, 19 May 2014 15:47:58 -0700
Subject: Bug 1006875 - Better styles for graphs in web audio editor, r=vp

---
 browser/devtools/webaudioeditor/test/browser.ini   |  1 +
 .../test/browser_wa_graph-markers.js               | 76 ++++++++++++++++++++++
 .../webaudioeditor/webaudioeditor-controller.js    | 21 +++++-
 .../devtools/webaudioeditor/webaudioeditor-view.js | 36 ++++++++--
 .../themes/shared/devtools/webaudioeditor.inc.css  | 51 ++++++---------
 5 files changed, 149 insertions(+), 36 deletions(-)
 create mode 100644 browser/devtools/webaudioeditor/test/browser_wa_graph-markers.js

diff --git a/browser/devtools/webaudioeditor/test/browser.ini b/browser/devtools/webaudioeditor/test/browser.ini
index 30cd892..84c5479 100644
--- a/browser/devtools/webaudioeditor/test/browser.ini
+++ b/browser/devtools/webaudioeditor/test/browser.ini
@@ -14,14 +14,15 @@ support-files =
 [browser_audionode-actor-is-source.js]
 [browser_webaudio-actor-simple.js]
 
 [browser_wa_first-run.js]
 
 [browser_wa_graph-click.js]
 [browser_wa_graph-render-01.js]
 [browser_wa_graph-render-02.js]
+[browser_wa_graph-markers.js]
 
 [browser_wa_properties-view.js]
 [browser_wa_properties-view-edit.js]
 
 [browser_wa_inspector.js]
 [browser_wa_inspector-toggle.js]
diff --git a/browser/devtools/webaudioeditor/test/browser_wa_graph-markers.js b/browser/devtools/webaudioeditor/test/browser_wa_graph-markers.js
new file mode 100644
index 0000000..c04a15e
--- /dev/null
+++ b/browser/devtools/webaudioeditor/test/browser_wa_graph-markers.js
@@ -0,0 +1,76 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests that the SVG marker styling is updated when devtools theme changes.
+ */
+
+function spawnTest() {
+  let [target, debuggee, panel] = yield initWebAudioEditor(SIMPLE_CONTEXT_URL);
+  let { panelWin } = panel;
+  let { gFront, $, $$, EVENTS, MARKER_STYLING } = panelWin;
+
+  let currentTheme = Services.prefs.getCharPref("devtools.theme");
+
+  ok(MARKER_STYLING.light, "Marker styling exists for light theme.");
+  ok(MARKER_STYLING.dark, "Marker styling exists for dark theme.");
+
+  let started = once(gFront, "start-context");
+
+  reload(target);
+
+  let [actors] = yield Promise.all([
+    get3(gFront, "create-node"),
+    waitForGraphRendered(panelWin, 3, 2)
+  ]);
+
+  is(getFill($("#arrowhead")), MARKER_STYLING[currentTheme],
+    "marker initially matches theme.");
+
+  // Switch to light
+  setTheme("light");
+  is(getFill($("#arrowhead")), MARKER_STYLING.light,
+    "marker styling matches light theme on change.");
+
+  // Switch to dark
+  setTheme("dark");
+  is(getFill($("#arrowhead")), MARKER_STYLING.dark,
+    "marker styling matches dark theme on change.");
+
+  // Switch to dark again
+  setTheme("dark");
+  is(getFill($("#arrowhead")), MARKER_STYLING.dark,
+    "marker styling remains dark.");
+
+  // Switch to back to light again
+  setTheme("light");
+  is(getFill($("#arrowhead")), MARKER_STYLING.light,
+    "marker styling switches back to light once again.");
+
+  yield teardown(panel);
+  finish();
+}
+
+/**
+ * Returns a hex value found in styling for an element. So parses
+ * <marker style="fill: #abcdef"> and returns "#abcdef"
+ */
+function getFill (el) {
+  return el.getAttribute("style").match(/(#.*)$/)[1];
+}
+
+/**
+ * Mimics selecting the theme selector in the toolbox;
+ * sets the preference and emits an event on gDevTools to trigger
+ * the themeing.
+ */
+function setTheme (newTheme) {
+  let oldTheme = Services.prefs.getCharPref("devtools.theme");
+  info("Setting `devtools.theme` to \"" + newTheme + "\"");
+  Services.prefs.setCharPref("devtools.theme", newTheme);
+  gDevTools.emit("pref-changed", {
+    pref: "devtools.theme",
+    newValue: newTheme,
+    oldValue: oldTheme
+  });
+}
diff --git a/browser/devtools/webaudioeditor/webaudioeditor-controller.js b/browser/devtools/webaudioeditor/webaudioeditor-controller.js
index 2454cb7..d288eeb 100644
--- a/browser/devtools/webaudioeditor/webaudioeditor-controller.js
+++ b/browser/devtools/webaudioeditor/webaudioeditor-controller.js
@@ -3,16 +3,17 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource:///modules/devtools/ViewHelpers.jsm");
+Cu.import("resource:///modules/devtools/gDevTools.jsm");
 
 // Override DOM promises with Promise.jsm helpers
 const { defer, all } = Cu.import("resource://gre/modules/Promise.jsm", {}).Promise;
 
 const { Task } = Cu.import("resource://gre/modules/Task.jsm", {});
 const require = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}).devtools.require;
 const EventEmitter = require("devtools/toolkit/event-emitter");
 const STRINGS_URI = "chrome://browser/locale/devtools/webaudioeditor.properties"
@@ -32,16 +33,19 @@ const EVENTS = {
   DISCONNECT_NODE: "WebAudioEditor:DisconnectNode",
 
   // When a node gets GC'd.
   DESTROY_NODE: "WebAudioEditor:DestroyNode",
 
   // On a node parameter's change.
   CHANGE_PARAM: "WebAudioEditor:ChangeParam",
 
+  // When the devtools theme changes.
+  THEME_CHANGE: "WebAudioEditor:ThemeChange",
+
   // When the UI is reset from tab navigation.
   UI_RESET: "WebAudioEditor:UIReset",
 
   // When a param has been changed via the UI and successfully
   // pushed via the actor to the raw audio node.
   UI_SET_PARAM: "WebAudioEditor:UISetParam",
 
   // When a node is to be set in the InspectorView.
@@ -136,24 +140,30 @@ function shutdownWebAudioEditor() {
  * Functions handling target-related lifetime events.
  */
 let WebAudioEditorController = {
   /**
    * Listen for events emitted by the current tab target.
    */
   initialize: function() {
     this._onTabNavigated = this._onTabNavigated.bind(this);
+    this._onThemeChange = this._onThemeChange.bind(this);
     gTarget.on("will-navigate", this._onTabNavigated);
     gTarget.on("navigate", this._onTabNavigated);
     gFront.on("start-context", this._onStartContext);
     gFront.on("create-node", this._onCreateNode);
     gFront.on("connect-node", this._onConnectNode);
     gFront.on("disconnect-node", this._onDisconnectNode);
     gFront.on("change-param", this._onChangeParam);
 
+    // Hook into theme change so we can change
+    // the graph's marker styling, since we can't do this
+    // with CSS
+    gDevTools.on("pref-changed", this._onThemeChange);
+
     // Set up events to refresh the Graph view
     window.on(EVENTS.CREATE_NODE, this._onUpdatedContext);
     window.on(EVENTS.CONNECT_NODE, this._onUpdatedContext);
     window.on(EVENTS.DISCONNECT_NODE, this._onUpdatedContext);
   },
 
   /**
    * Remove events emitted by the current tab target.
@@ -164,27 +174,37 @@ let WebAudioEditorController = {
     gFront.off("start-context", this._onStartContext);
     gFront.off("create-node", this._onCreateNode);
     gFront.off("connect-node", this._onConnectNode);
     gFront.off("disconnect-node", this._onDisconnectNode);
     gFront.off("change-param", this._onChangeParam);
     window.off(EVENTS.CREATE_NODE, this._onUpdatedContext);
     window.off(EVENTS.CONNECT_NODE, this._onUpdatedContext);
     window.off(EVENTS.DISCONNECT_NODE, this._onUpdatedContext);
+    gDevTools.off("pref-changed", this._onThemeChange);
   },
 
   /**
    * Called when a new audio node is created, or the audio context
    * routing changes.
    */
   _onUpdatedContext: function () {
     WebAudioGraphView.draw();
   },
 
   /**
+   * Fired when the devtools theme changes (light, dark, etc.)
+   * so that the graph can update marker styling, as that
+   * cannot currently be done with CSS.
+   */
+  _onThemeChange: function (event, data) {
+    window.emit(EVENTS.THEME_CHANGE, data.newValue);
+  },
+
+  /**
    * Called for each location change in the debugged tab.
    */
   _onTabNavigated: function(event) {
     switch (event) {
       case "will-navigate": {
         Task.spawn(function() {
           // Make sure the backend is prepared to handle audio contexts.
           yield gFront.setup({ reload: false });
@@ -313,9 +333,8 @@ function getViewNodeByActor (actor) {
 }
 
 /**
  * Returns the corresponding ViewNode by actorID
  */
 function getViewNodeById (id) {
   return getViewNodeByActor({ actorID: id });
 }
-
diff --git a/browser/devtools/webaudioeditor/webaudioeditor-view.js b/browser/devtools/webaudioeditor/webaudioeditor-view.js
index 72ecac5..e23d7e9 100644
--- a/browser/devtools/webaudioeditor/webaudioeditor-view.js
+++ b/browser/devtools/webaudioeditor/webaudioeditor-view.js
@@ -20,16 +20,22 @@ const INSPECTOR_WIDTH = 300;
 // TODO investigate to see how this works in other host types bug 994257
 const WIDTH = 1000;
 const HEIGHT = 400;
 
 // Sizes of SVG arrows in graph
 const ARROW_HEIGHT = 5;
 const ARROW_WIDTH = 8;
 
+// Styles for markers as they cannot be done with CSS.
+const MARKER_STYLING = {
+  light: "#AAA",
+  dark: "#CED3D9"
+};
+
 const GRAPH_DEBOUNCE_TIMER = 100;
 
 const GENERIC_VARIABLES_VIEW_SETTINGS = {
   searchEnabled: false,
   editableValueTooltip: "",
   editableNameTooltip: "",
   preventDisableOnChange: true,
   preventDescriptorModifiers: true,
@@ -40,28 +46,33 @@ const GENERIC_VARIABLES_VIEW_SETTINGS = {
  * Functions handling the graph UI.
  */
 let WebAudioGraphView = {
   /**
    * Initialization function, called when the tool is started.
    */
   initialize: function() {
     this._onGraphNodeClick = this._onGraphNodeClick.bind(this);
+    this._onThemeChange = this._onThemeChange.bind(this);
+
     this.draw = debounce(this.draw.bind(this), GRAPH_DEBOUNCE_TIMER);
     $('#graph-target').addEventListener('click', this._onGraphNodeClick, false);
+
+    window.on(EVENTS.THEME_CHANGE, this._onThemeChange);
   },
 
   /**
    * Destruction function, called when the tool is closed.
    */
   destroy: function() {
     if (this._zoomBinding) {
       this._zoomBinding.on("zoom", null);
     }
     $('#graph-target').removeEventListener('click', this._onGraphNodeClick, false);
+    window.off(EVENTS.THEME_CHANGE, this._onThemeChange);
   },
 
   /**
    * Called when a page is reloaded and waiting for a "start-context" event
    * and clears out old content
    */
   resetUI: function () {
     $("#reload-notice").hidden = true;
@@ -170,33 +181,37 @@ let WebAudioGraphView = {
         return edge.target;
       });
       return svgNodes;
     });
 
     // Override Dagre-d3's post render function by passing in our own.
     // This way we can leave styles out of it.
     renderer.postRender(function (graph, root) {
-      // TODO change arrowhead color depending on theme-dark/theme-light
-      // and possibly refactor rendering this as it's ugly
-      // Bug 994256
-      // let color = window.classList.contains("theme-dark") ? "#f5f7fa" : "#585959";
+      // We have to manually set the marker styling since we cannot
+      // do this currently with CSS, although it is in spec for SVG2
+      // https://svgwg.org/svg2-draft/painting.html#VertexMarkerProperties
+      // For now, manually set it on creation, and the `_onThemeChange`
+      // function will fire when the devtools theme changes to update the
+      // styling manually.
+      let theme = Services.prefs.getCharPref("devtools.theme");
+      let markerColor = MARKER_STYLING[theme];
       if (graph.isDirected() && root.select("#arrowhead").empty()) {
         root
           .append("svg:defs")
           .append("svg:marker")
           .attr("id", "arrowhead")
           .attr("viewBox", "0 0 10 10")
           .attr("refX", ARROW_WIDTH)
           .attr("refY", ARROW_HEIGHT)
           .attr("markerUnits", "strokewidth")
           .attr("markerWidth", ARROW_WIDTH)
           .attr("markerHeight", ARROW_HEIGHT)
           .attr("orient", "auto")
-          .attr("style", "fill: #f5f7fa")
+          .attr("style", "fill: " + markerColor)
           .append("svg:path")
           .attr("d", "M 0 0 L 10 5 L 0 10 z");
       }
 
       // Fire an event upon completed rendering
       window.emit(EVENTS.UI_GRAPH_RENDERED, AudioNodes.length, edges.length);
     });
 
@@ -215,16 +230,27 @@ let WebAudioGraphView = {
     }
   },
 
   /**
    * Event handlers
    */
 
   /**
+   * Fired when the devtools theme changes.
+   */
+  _onThemeChange: function (eventName, theme) {
+    let markerColor = MARKER_STYLING[theme];
+    let marker = $("#arrowhead");
+    if (marker) {
+      marker.setAttribute("style", "fill: " + markerColor);
+    }
+  },
+
+  /**
    * Fired when a node in the svg graph is clicked. Used to handle triggering the AudioNodePane.
    *
    * @param Event e
    *        Click event.
    */
   _onGraphNodeClick: function (e) {
     let node = findGraphNodeParent(e.target);
     // If node not found (clicking outside of an audio node in the graph),
diff --git a/browser/themes/shared/devtools/webaudioeditor.inc.css b/browser/themes/shared/devtools/webaudioeditor.inc.css
index f482ab4..1a913ea 100644
--- a/browser/themes/shared/devtools/webaudioeditor.inc.css
+++ b/browser/themes/shared/devtools/webaudioeditor.inc.css
@@ -41,76 +41,67 @@
 /* Context Graph */
 svg {
   position: fixed;
   overflow: hidden;
 }
 
 /* Edges in graph */
 .edgePath path {
-  stroke-width: 1.5px;
+  stroke-width: 1px;
   fill: none;
 }
 
-.theme-dark .edgePath path { stroke: #f5f7fa; }
-.theme-light .edgePath path { stroke: #585959; }
+.theme-dark .edgePath path {
+  stroke: #b6babf; /* Grey foreground text */
+}
+.theme-light .edgePath path {
+  stroke: #aaaaaa; /* Splitters */
+}
 
 /* Audio Nodes */
 .nodes rect {
-  stroke-width: 2px;
+  stroke-width: 1px;
   cursor: pointer;
 }
 
 .theme-dark .nodes rect {
-  stroke: #585959;
-  fill: #f5f7fa;
+  stroke: #252c33; /* Tab toolbar */
+  fill: #343c45; /* Toolbars */
 }
 
 .theme-light .nodes rect {
-  fill: #585959;
-  stroke: #f5f7fa;
+  stroke: #ebeced; /* Tab toolbar */
+  fill: #f0f1f2; /* Toolbar */
 }
 
-.nodes g.selected rect {
-  stroke-width: 1.5px;
-  animation-duration: 1s;
-  animation-iteration-count: infinite;
-  animation-timing-function: ease-in-out;
-  animation-direction: alternate;
+.theme-dark .nodes g.selected rect {
+  fill: #1d4f73; /* Select Highlight Blue */
 }
 
-.theme-dark .nodes g.selected rect { animation-name: dark-selected-glow; }
-.theme-light .nodes g.selected rect { animation-name: light-selected-glow; }
+.theme-light .nodes g.selected rect {
+  fill: #4c9ed9; /* Select Highlight Blue */
+}
 
 /* Text in nodes */
 text {
   cursor: pointer;
   font-weight: 300;
   font-family: "Helvetica Neue", Helvetica, Arial, sans-serf;
   font-size: 14px;
 }
 
 .theme-dark text {
-  fill: #333;
-  /* text-shadow: 0 1px 0 #fff, 1px 0 0 #fff, 0 -1px 0 #fff, -1px 0 0 #fff; */
+  fill: #b6babf; /* Grey foreground text */
 }
 .theme-light text {
-  fill: #111;
-}
-
-/**
- * Animations for graphs
- */
-@keyframes dark-selected-glow {
-  0% { fill: #1d4f73; }
-  100% { fill: #3CA3ED; }
+  fill: #585959; /* Grey foreground text */
 }
-@keyframes light-selected-glow {
-  0% { fill: #4c9ed9; }
-  100% { fill: #59BAFF; }
+.theme-light g.selected text {
+  fill: #f0f1f2; /* Toolbars */
 }
 
 /**
  * Inspector Styles
  */
 
 .web-audio-inspector .error {
   background-image: url(alerticon-warning.png);
-- 
1.8.4.2