Skip to content

Commit

Permalink
Bug 1190047 - [Rule View] Ensure changes to swatches are reverted to …
Browse files Browse the repository at this point in the history 
…the original value on escape r=bgrins
  • Loading branch information
@gabrielluong
gabrielluong committed Aug 4, 2015
1 parent 03c1d93 commit fc897ad
Show file tree
Hide file tree
Showing 5 changed files with 305 additions and 73 deletions.
5 changes: 5 additions & 0 deletions browser/devtools/shared/widgets/Tooltip.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1025,13 +1025,17 @@ SwatchBasedEditorTooltip.prototype = {
* @param {object} callbacks
* Callbacks that will be executed when the editor wants to preview a
* value change, or revert a change, or commit a change.
* - onShow: will be called when one of the swatch tooltip is shown
* - onPreview: will be called when one of the sub-classes calls
* preview
* - onRevert: will be called when the user ESCapes out of the tooltip
* - onCommit: will be called when the user presses ENTER or clicks
* outside the tooltip.
*/
addSwatch: function(swatchEl, callbacks={}) {
if (!callbacks.onShow) {
callbacks.onShow = function() {};
}
if (!callbacks.onPreview) {
callbacks.onPreview = function() {};
}
Expand Down Expand Up @@ -1069,6 +1073,7 @@ SwatchBasedEditorTooltip.prototype = {
if (swatch) {
this.activeSwatch = event.target;
this.show();
swatch.callbacks.onShow();
event.stopPropagation();
}
},
Expand Down
50 changes: 40 additions & 10 deletions browser/devtools/styleinspector/rule-view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2849,6 +2849,9 @@ function TextPropertyEditor(aRuleEditor, aProperty) {
this._onStartEditing = this._onStartEditing.bind(this);
this._onNameDone = this._onNameDone.bind(this);
this._onValueDone = this._onValueDone.bind(this);
this._onSwatchCommit = this._onSwatchCommit.bind(this);
this._onSwatchPreview = this._onSwatchPreview.bind(this);
this._onSwatchRevert = this._onSwatchRevert.bind(this);
this._onValidate = throttle(this._previewValue, 10, this);
this.update = this.update.bind(this);

Expand Down Expand Up @@ -3075,7 +3078,7 @@ TextPropertyEditor.prototype = {

this.warning.hidden = this.editing || this.isValid();

if ((this.prop.overridden || !this.prop.enabled) && !this.editing) {
if (this.prop.overridden || !this.prop.enabled) {
this.element.classList.add("ruleview-overridden");
} else {
this.element.classList.remove("ruleview-overridden");
Expand Down Expand Up @@ -3130,9 +3133,10 @@ TextPropertyEditor.prototype = {
// Adding this swatch to the list of swatches our colorpicker
// knows about
this.ruleView.tooltips.colorPicker.addSwatch(span, {
onPreview: () => this._previewValue(this.valueSpan.textContent),
onCommit: () => this._onValueDone(this.valueSpan.textContent, true),
onRevert: () => this._onValueDone(undefined, false)
onShow: this._onStartEditing,
onPreview: this._onSwatchPreview,
onCommit: this._onSwatchCommit,
onRevert: this._onSwatchRevert
});
}
}
Expand All @@ -3145,9 +3149,10 @@ TextPropertyEditor.prototype = {
// Adding this swatch to the list of swatches our colorpicker
// knows about
this.ruleView.tooltips.cubicBezier.addSwatch(span, {
onPreview: () => this._previewValue(this.valueSpan.textContent),
onCommit: () => this._onValueDone(this.valueSpan.textContent, true),
onRevert: () => this._onValueDone(undefined, false)
onShow: this._onStartEditing,
onPreview: this._onSwatchPreview,
onCommit: this._onSwatchCommit,
onRevert: this._onSwatchRevert
});
}
}
Expand All @@ -3159,9 +3164,10 @@ TextPropertyEditor.prototype = {
parserOptions.filterSwatch = true;

this.ruleView.tooltips.filterEditor.addSwatch(span, {
onPreview: () => this._previewValue(this.valueSpan.textContent),
onCommit: () => this._onValueDone(this.valueSpan.textContent, true),
onRevert: () => this._onValueDone(undefined, false)
onShow: this._onStartEditing,
onPreview: this._onSwatchPreview,
onCommit: this._onSwatchCommit,
onRevert: this._onSwatchRevert
}, outputParser, parserOptions);
}
}
Expand Down Expand Up @@ -3422,6 +3428,30 @@ TextPropertyEditor.prototype = {
}
},

/**
* Called when the swatch editor wants to commit a value change.
*/
_onSwatchCommit: function() {
this._onValueDone(this.valueSpan.textContent, true);
this.update();
},

/**
* Called when the swatch editor wants to preview a value change.
*/
_onSwatchPreview: function() {
this._previewValue(this.valueSpan.textContent);
},

/**
* Called when the swatch editor closes from an ESC. Revert to the original
* value of this property before editing.
*/
_onSwatchRevert: function() {
this.rule.setPropertyEnabled(this.prop, this.prop.enabled);
this.update();
},

/**
* Parse a value string and break it into pieces, starting with the
* first value, and into an array of additional properties (if any).
Expand Down
111 changes: 88 additions & 23 deletions browser/devtools/styleinspector/test/browser_ruleview_colorpicker-revert-on-ESC.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,51 +6,116 @@

// Test that a color change in the color picker is reverted when ESC is pressed

const PAGE_CONTENT = [
'<style type="text/css">',
' body {',
' background-color: #ededed;',
' }',
'</style>',
'Testing the color picker tooltip!'
let TEST_URI = [
"<style type='text/css'>",
" body {",
" background-color: #EDEDED;",
" }",
"</style>",
].join("\n");

add_task(function*() {
yield addTab("data:text/html;charset=utf-8,rule view color picker tooltip test");
content.document.body.innerHTML = PAGE_CONTENT;
let {toolbox, inspector, view} = yield openRuleView();

let swatch = getRuleViewProperty(view, "body", "background-color").valueSpan
.querySelector(".ruleview-colorswatch");
yield testPressingEscapeRevertsChanges(swatch, view);
yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
let {view} = yield openRuleView();
yield testPressingEscapeRevertsChanges(view);
yield testPressingEscapeRevertsChangesAndDisables(view);
});

function* testPressingEscapeRevertsChanges(swatch, ruleView) {
let cPicker = ruleView.tooltips.colorPicker;
function* testPressingEscapeRevertsChanges(view) {
let ruleEditor = getRuleViewRuleEditor(view, 1);
let propEditor = ruleEditor.rule.textProps[0].editor;
let swatch = propEditor.valueSpan.querySelector(".ruleview-colorswatch");
let cPicker = view.tooltips.colorPicker;

let onShown = cPicker.tooltip.once("shown");
swatch.click();
yield onShown;

yield simulateColorPickerChange(ruleView, cPicker, [0, 0, 0, 1], {
yield simulateColorPickerChange(view, cPicker, [0, 0, 0, 1], {
element: content.document.body,
name: "backgroundColor",
value: "rgb(0, 0, 0)"
});

is(swatch.style.backgroundColor, "rgb(0, 0, 0)",
"The color swatch's background was updated");
is(getRuleViewProperty(ruleView, "body", "background-color").valueSpan.textContent,
"#000", "The text of the background-color css property was updated");
is(propEditor.valueSpan.textContent, "#000",
"The text of the background-color css property was updated");

let spectrum = yield cPicker.spectrum;

info("Pressing ESCAPE to close the tooltip");
let onHidden = cPicker.tooltip.once("hidden");
EventUtils.sendKey("ESCAPE", spectrum.element.ownerDocument.defaultView);
yield onHidden;
yield ruleEditor.rule._applyingModifications;

yield waitForComputedStyleProperty("body", null, "background-color",
"rgb(237, 237, 237)");
is(propEditor.valueSpan.textContent, "#EDEDED",
"Got expected property value.");
}

function* testPressingEscapeRevertsChangesAndDisables(view) {
let ruleEditor = getRuleViewRuleEditor(view, 1);
let propEditor = ruleEditor.rule.textProps[0].editor;
let swatch = propEditor.valueSpan.querySelector(".ruleview-colorswatch");
let cPicker = view.tooltips.colorPicker;

info("Disabling background-color property");
propEditor.enable.click();
yield ruleEditor.rule._applyingModifications;

ok(propEditor.element.classList.contains("ruleview-overridden"),
"property is overridden.");
is(propEditor.enable.style.visibility, "visible",
"property enable checkbox is visible.");
ok(!propEditor.enable.getAttribute("checked"),
"property enable checkbox is not checked.");
ok(!propEditor.prop.enabled,
"background-color property is disabled.");
let newValue = yield getRulePropertyValue("background-color");
is(newValue, "", "background-color should have been unset.");

let onShown = cPicker.tooltip.once("shown");
swatch.click();
yield onShown;

ok(!propEditor.element.classList.contains("ruleview-overridden"),
"property overridden is not displayed.");
is(propEditor.enable.style.visibility, "hidden",
"property enable checkbox is hidden.");

let spectrum = yield cPicker.spectrum;
info("Simulating a color picker change in the widget");
spectrum.rgb = [0, 0, 0, 1];
yield ruleEditor.rule._applyingModifications;

// ESC out of the color picker
info("Pressing ESCAPE to close the tooltip");
let onHidden = cPicker.tooltip.once("hidden");
EventUtils.sendKey("ESCAPE", spectrum.element.ownerDocument.defaultView);
yield onHidden;
yield ruleEditor.rule._applyingModifications;

yield waitForSuccess(() => {
return content.getComputedStyle(content.document.body).backgroundColor === "rgb(237, 237, 237)";
}, "The element's background-color was reverted");
ok(propEditor.element.classList.contains("ruleview-overridden"),
"property is overridden.");
is(propEditor.enable.style.visibility, "visible",
"property enable checkbox is visible.");
ok(!propEditor.enable.getAttribute("checked"),
"property enable checkbox is not checked.");
ok(!propEditor.prop.enabled,
"background-color property is disabled.");
newValue = yield getRulePropertyValue("background-color");
is(newValue, "", "background-color should have been unset.");
is(propEditor.valueSpan.textContent, "#EDEDED",
"Got expected property value.");
}

function* getRulePropertyValue(name) {
let propValue = yield executeInContent("Test:GetRulePropertyValue", {
styleSheetIndex: 0,
ruleIndex: 0,
name: name
});
return propValue;
}
118 changes: 92 additions & 26 deletions browser/devtools/styleinspector/test/browser_ruleview_cubicbezier-revert-on-ESC.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,29 @@

"use strict";

// Test that changes made to the cubic-bezier timing-function in the cubic-bezier
// tooltip are reverted when ESC is pressed

const PAGE_CONTENT = [
'<style type="text/css">',
' body {',
' animation-timing-function: linear;',
' }',
'</style>',
// Test that changes made to the cubic-bezier timing-function in the
// cubic-bezier tooltip are reverted when ESC is pressed

let TEST_URI = [
"<style type='text/css'>",
" body {",
" animation-timing-function: linear;",
" }",
"</style>",
].join("\n");

add_task(function*() {
yield addTab("data:text/html;charset=utf-8,rule view cubic-bezier tooltip test");
content.document.body.innerHTML = PAGE_CONTENT;
let {toolbox, inspector, view} = yield openRuleView();

info("Getting the bezier swatch element");
let swatch = getRuleViewProperty(view, "body", "animation-timing-function").valueSpan
.querySelector(".ruleview-bezierswatch");
yield testPressingEscapeRevertsChanges(swatch, view);
yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
let {view} = yield openRuleView();
yield testPressingEscapeRevertsChanges(view);
yield testPressingEscapeRevertsChangesAndDisables(view);
});

function* testPressingEscapeRevertsChanges(swatch, ruleView) {
let bezierTooltip = ruleView.tooltips.cubicBezier;
function* testPressingEscapeRevertsChanges(view) {
let ruleEditor = getRuleViewRuleEditor(view, 1);
let propEditor = ruleEditor.rule.textProps[0].editor;
let swatch = propEditor.valueSpan.querySelector(".ruleview-bezierswatch");
let bezierTooltip = view.tooltips.cubicBezier;

let onShown = bezierTooltip.tooltip.once("shown");
swatch.click();
Expand All @@ -36,18 +35,85 @@ function* testPressingEscapeRevertsChanges(swatch, ruleView) {
let widget = yield bezierTooltip.widget;
info("Simulating a change of curve in the widget");
widget.coordinates = [0.1, 2, 0.9, -1];
let expected = "cubic-bezier(0.1, 2, 0.9, -1)";
yield ruleEditor.rule._applyingModifications;

yield waitForComputedStyleProperty("body", null, "animation-timing-function",
"cubic-bezier(0.1, 2, 0.9, -1)");
is(propEditor.valueSpan.textContent, "cubic-bezier(.1,2,.9,-1)",
"Got expected property value.");

info("Pressing ESCAPE to close the tooltip");
let onHidden = bezierTooltip.tooltip.once("hidden");
EventUtils.sendKey("ESCAPE", widget.parent.ownerDocument.defaultView);
yield onHidden;
yield ruleEditor.rule._applyingModifications;

yield waitForComputedStyleProperty("body", null, "animation-timing-function",
"cubic-bezier(0, 0, 1, 1)");
is(propEditor.valueSpan.textContent, "linear",
"Got expected property value.");
}

function* testPressingEscapeRevertsChangesAndDisables(view) {
let ruleEditor = getRuleViewRuleEditor(view, 1);
let propEditor = ruleEditor.rule.textProps[0].editor;
let swatch = propEditor.valueSpan.querySelector(".ruleview-bezierswatch");
let bezierTooltip = view.tooltips.cubicBezier;

info("Disabling animation-timing-function property");
propEditor.enable.click();
yield ruleEditor.rule._applyingModifications;

yield waitForSuccess(() => {
return content.getComputedStyle(content.document.body).animationTimingFunction === expected;
}, "Waiting for the change to be previewed on the element");
ok(propEditor.element.classList.contains("ruleview-overridden"),
"property is overridden.");
is(propEditor.enable.style.visibility, "visible",
"property enable checkbox is visible.");
ok(!propEditor.enable.getAttribute("checked"),
"property enable checkbox is not checked.");
ok(!propEditor.prop.enabled,
"animation-timing-function property is disabled.");
let newValue = yield getRulePropertyValue("animation-timing-function");
is(newValue, "", "animation-timing-function should have been unset.");

let onShown = bezierTooltip.tooltip.once("shown");
swatch.click();
yield onShown;

ok(!propEditor.element.classList.contains("ruleview-overridden"),
"property overridden is not displayed.");
is(propEditor.enable.style.visibility, "hidden",
"property enable checkbox is hidden.");

let widget = yield bezierTooltip.widget;
info("Simulating a change of curve in the widget");
widget.coordinates = [0.1, 2, 0.9, -1];
yield ruleEditor.rule._applyingModifications;

info("Pressing ESCAPE to close the tooltip");
let onHidden = bezierTooltip.tooltip.once("hidden");
EventUtils.sendKey("ESCAPE", widget.parent.ownerDocument.defaultView);
yield onHidden;
yield ruleEditor.rule._applyingModifications;

ok(propEditor.element.classList.contains("ruleview-overridden"),
"property is overridden.");
is(propEditor.enable.style.visibility, "visible",
"property enable checkbox is visible.");
ok(!propEditor.enable.getAttribute("checked"),
"property enable checkbox is not checked.");
ok(!propEditor.prop.enabled,
"animation-timing-function property is disabled.");
newValue = yield getRulePropertyValue("animation-timing-function");
is(newValue, "", "animation-timing-function should have been unset.");
is(propEditor.valueSpan.textContent, "linear",
"Got expected property value.");
}

yield waitForSuccess(() => {
return content.getComputedStyle(content.document.body).animationTimingFunction === "cubic-bezier(0, 0, 1, 1)";
}, "Waiting for the change to be reverted on the element");
function* getRulePropertyValue(name) {
let propValue = yield executeInContent("Test:GetRulePropertyValue", {
styleSheetIndex: 0,
ruleIndex: 0,
name: name
});
return propValue;
}
Loading

0 comments on commit fc897ad

Please sign in to comment.