1055217 - Add front end for disabling/bypassing audio nodes

Status: RESOLVED FIXED

(DevTools Graveyard :: Web Audio Editor, defect)

Description

[Jordan Santell [:jsantell] [@jsantell]]

The front end component of bug 1055211 -- will need a style applied to the node to indicate it's bypassed, as well as a way to toggle this per node, either as a right click/context menu on the node (how will people discover this?) or as a stand alone button on the node, or the inspector for the node.

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Possibly the first feature containing individual node "controls", which should be done via hover over a node in the graph, displaying the name, as well as a bypass button, similar to the [ev] icon targets in inspector showing event handlers.

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1055217-bypass-audio-nodes.patch

This is a working patch for adding UI to bypassing audio nodes.

Concerns are some visual ones -- by default, nodes are "on" (power icon), so should we have an icon that denotes the opposite of this? How can we display in the graph which nodes are bypassed? The demo has these painted with a lower opacity, but very hard to see, and harder to spot if something is bypassed.

http://i.imgur.com/jTk6WBF.gif

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1055217-bypass-audio-nodes.patch

Finally got this completed (mostly). Everything is in here, working, tests. Only outstanding thing is tweaking with styling, but sending it over for review now.

Thoughts on what a bypassed node should look like? Opacity doesn't help much as there's not enough noticible contrast, and if we show 'dead nodes', that may be more appropriate for that.

Comment 4

[Victor Porof [:vporof][:vp]]

another 50K patch jordan

Comment 5

[Victor Porof [:vporof][:vp]]

Comment on attachment 8550064 [details] [diff] [review]
1055217-bypass-audio-nodes.patch

Review of attachment 8550064 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/webaudioeditor/controller.js
@@ +61,5 @@
>      // with CSS
>      gDevTools.on("pref-changed", this._onThemeChange);
> +
> +    // Store the AudioNode definitions from the WebAudioFront
> +    DEFINITION = yield gFront.getDefinition();

Maybe rename this to AUDIO_NODE_DEFINITION?

::: browser/devtools/webaudioeditor/test/head.js
@@ +319,5 @@
>  }
>  
> +function command (button) {
> +  let ev = button.ownerDocument.createEvent("XULCommandEvent");
> +  ev.initCommandEvent("command", true, true, button.ownerDocument.defaultView, 0, false, false, false, false, null);

Ugh.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1055217-bypass-audio-nodes.patch

Fixed nit -- added all the styling
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b8f907c9448

Comment 7

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1055217-bypass-audio-nodes.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=33c198b125f4

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/a242901a5347

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a242901a5347