Web Inspector: [PARITY] Styles Redesign: clicking on the white space after the proper...
authornvasilyev@apple.com <nvasilyev@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Oct 2017 08:43:53 +0000 (08:43 +0000)
committernvasilyev@apple.com <nvasilyev@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Oct 2017 08:43:53 +0000 (08:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178022
<rdar://problem/34861687>

Reviewed by Matt Baker.

- Clicking on the whitespace on the right side of a property should insert a blank property after the clicked one.
- Clicking on the whitespace at the end of a CSS rule should append a blank property.
- Clicking on the whitespace before the first property should insert a blank property before the first one.

* UserInterface/Models/CSSProperty.js:
(WI.CSSProperty.prototype.remove):
(WI.CSSProperty.prototype._updateOwnerStyleText):
Remove method previously didn't do anything for a newly added property.

* UserInterface/Models/CSSStyleDeclaration.js:
(WI.CSSStyleDeclaration.prototype.newBlankProperty):
Update indices of all properties after the newly added property.

* UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:
(WI.SpreadsheetCSSStyleDeclarationEditor):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.layout):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.startEditingFirstProperty):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.startEditingLastProperty):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.isFocused):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.addBlankProperty):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetCSSStyleDeclarationEditorFocusMoved):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype._propertiesChanged):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype._addBlankProperty): Deleted.
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype._isFocused): Deleted.
Re-layout SpreadsheetCSSStyleDeclarationEditor after adding a new property. Preserve edited property
so we can restore editing state after the re-layout.

* UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:
(WI.SpreadsheetCSSStyleDeclarationSection):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.initialLayout):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleMouseDown):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleClick):
Clicking should add a new property only when we aren't editing an existing property.

* UserInterface/Views/SpreadsheetStyleProperty.js:
(WI.SpreadsheetStyleProperty):
(WI.SpreadsheetStyleProperty.prototype.updateClassNames):
(WI.SpreadsheetStyleProperty.prototype.spreadsheetTextFieldDidCommit):
Remove newlyAdded property of SpreadsheetStyleProperty. During layout SpreadsheetCSSStyleDeclarationEditor
recreates SpreadsheetStyleProperty views and newlyAdded property gets lost.

* UserInterface/Views/SpreadsheetTextField.js:
(WI.SpreadsheetTextField.prototype.get valueBeforeEditing):
(WI.SpreadsheetTextField.prototype.startEditing):
(WI.SpreadsheetTextField.prototype.stopEditing):
(WI.SpreadsheetTextField.prototype._discardChange):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224174 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js
Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js
Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js

index 4f489c8..8812c86 100644 (file)
@@ -1,3 +1,58 @@
+2017-10-30  Nikita Vasilyev  <nvasilyev@apple.com>
+
+        Web Inspector: [PARITY] Styles Redesign: clicking on the white space after the property should create a blank property
+        https://bugs.webkit.org/show_bug.cgi?id=178022
+        <rdar://problem/34861687>
+
+        Reviewed by Matt Baker.
+
+        - Clicking on the whitespace on the right side of a property should insert a blank property after the clicked one.
+        - Clicking on the whitespace at the end of a CSS rule should append a blank property.
+        - Clicking on the whitespace before the first property should insert a blank property before the first one.
+
+        * UserInterface/Models/CSSProperty.js:
+        (WI.CSSProperty.prototype.remove):
+        (WI.CSSProperty.prototype._updateOwnerStyleText):
+        Remove method previously didn't do anything for a newly added property.
+
+        * UserInterface/Models/CSSStyleDeclaration.js:
+        (WI.CSSStyleDeclaration.prototype.newBlankProperty):
+        Update indices of all properties after the newly added property.
+
+        * UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:
+        (WI.SpreadsheetCSSStyleDeclarationEditor):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.layout):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.startEditingFirstProperty):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.startEditingLastProperty):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.isFocused):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.addBlankProperty):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetCSSStyleDeclarationEditorFocusMoved):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype._propertiesChanged):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype._addBlankProperty): Deleted.
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype._isFocused): Deleted.
+        Re-layout SpreadsheetCSSStyleDeclarationEditor after adding a new property. Preserve edited property
+        so we can restore editing state after the re-layout.
+
+        * UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:
+        (WI.SpreadsheetCSSStyleDeclarationSection):
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype.initialLayout):
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleMouseDown):
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleClick):
+        Clicking should add a new property only when we aren't editing an existing property.
+
+        * UserInterface/Views/SpreadsheetStyleProperty.js:
+        (WI.SpreadsheetStyleProperty):
+        (WI.SpreadsheetStyleProperty.prototype.updateClassNames):
+        (WI.SpreadsheetStyleProperty.prototype.spreadsheetTextFieldDidCommit):
+        Remove newlyAdded property of SpreadsheetStyleProperty. During layout SpreadsheetCSSStyleDeclarationEditor
+        recreates SpreadsheetStyleProperty views and newlyAdded property gets lost.
+
+        * UserInterface/Views/SpreadsheetTextField.js:
+        (WI.SpreadsheetTextField.prototype.get valueBeforeEditing):
+        (WI.SpreadsheetTextField.prototype.startEditing):
+        (WI.SpreadsheetTextField.prototype.stopEditing):
+        (WI.SpreadsheetTextField.prototype._discardChange):
+
 2017-10-27  Devin Rousso  <webkit@devinrousso.com>
 
         Web Inspector: Canvas Tab: no way to see backtrace of where a canvas context was created
index ad8c7a5..c2671d5 100644 (file)
@@ -121,7 +121,9 @@ WI.CSSProperty = class CSSProperty extends WI.Object
     remove()
     {
         // Setting name or value to an empty string removes the entire CSSProperty.
-        this.name = "";
+        this._name = "";
+        const forceRemove = true;
+        this._updateStyleText(forceRemove);
     }
 
     commentOut(disabled)
@@ -325,7 +327,7 @@ WI.CSSProperty = class CSSProperty extends WI.Object
 
     // Private
 
-    _updateStyleText()
+    _updateStyleText(forceRemove = false)
     {
         let text = "";
 
@@ -334,13 +336,19 @@ WI.CSSProperty = class CSSProperty extends WI.Object
 
         let oldText = this._text;
         this._text = text;
-        this._updateOwnerStyleText(oldText, this._text);
+        this._updateOwnerStyleText(oldText, this._text, forceRemove);
     }
 
-    _updateOwnerStyleText(oldText, newText)
+    _updateOwnerStyleText(oldText, newText, forceRemove = false)
     {
-        if (oldText === newText)
+        if (oldText === newText) {
+            if (forceRemove) {
+                const lineDelta = 0;
+                const columnDelta = 0;
+                this._ownerStyle.shiftPropertiesAfter(this, lineDelta, columnDelta, forceRemove);
+            }
             return;
+        }
 
         let styleText = this._ownerStyle.text || "";
 
index 3787415..09541d0 100644 (file)
@@ -357,17 +357,19 @@ WI.CSSStyleDeclaration = class CSSStyleDeclaration extends WI.Object
         return !!this._properties.length;
     }
 
-    newBlankProperty(insertAfterIndex)
+    newBlankProperty(propertyIndex)
     {
         let text, name, value, priority, overridden, implicit, anonymous;
         let enabled = true;
         let valid = false;
-        let styleSheetTextRange = this._rangeAfterPropertyAtIndex(insertAfterIndex);
+        let styleSheetTextRange = this._rangeAfterPropertyAtIndex(propertyIndex - 1);
 
-        let propertyIndex = insertAfterIndex + 1;
         let property = new WI.CSSProperty(propertyIndex, text, name, value, priority, enabled, overridden, implicit, anonymous, valid, styleSheetTextRange);
 
         this._allProperties.insertAtIndex(property, propertyIndex);
+        for (let index = propertyIndex + 1; index < this._allProperties.length; index++)
+            this._allProperties[index].index = index;
+
         const suppressEvents = true;
         this.update(this._text, this._allProperties, this._styleSheetTextRange, suppressEvents);
 
index 6333b8f..ad75785 100644 (file)
@@ -34,6 +34,7 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
         this._delegate = delegate;
         this.style = style;
         this._propertyViews = [];
+        this._propertyPendingStartEditing = null;
     }
 
     // Public
@@ -47,12 +48,23 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
         let properties = this._propertiesToRender;
         this.element.classList.toggle("no-properties", !properties.length);
 
+        // FIXME: Only re-layout properties that have been modified and preserve focus whenever possible.
         this._propertyViews = [];
+
+        let propertyViewPendingStartEditing = null;
         for (let index = 0; index < properties.length; index++) {
             let property = properties[index];
-            let propertyView = new WI.SpreadsheetStyleProperty(this, property);
+            let propertyView = new WI.SpreadsheetStyleProperty(this, property, index);
             this.element.append(propertyView.element);
             this._propertyViews.push(propertyView);
+
+            if (property === this._propertyPendingStartEditing)
+                propertyViewPendingStartEditing = propertyView;
+        }
+
+        if (propertyViewPendingStartEditing) {
+            propertyViewPendingStartEditing.nameTextField.startEditing();
+            this._propertyPendingStartEditing = null;
         }
     }
 
@@ -89,7 +101,7 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
             this._propertyViews[0].nameTextField.startEditing();
         else {
             let index = 0;
-            this._addBlankProperty(index);
+            this.addBlankProperty(index);
         }
     }
 
@@ -100,7 +112,7 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
             lastProperty.valueTextField.startEditing();
         else {
             let index = 0;
-            this._addBlankProperty(index);
+            this.addBlankProperty(index);
         }
     }
 
@@ -145,6 +157,27 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
         return false;
     }
 
+    isFocused()
+    {
+        let focusedElement = document.activeElement;
+
+        if (!focusedElement || focusedElement.tagName === "BODY")
+            return false;
+
+        return focusedElement.isSelfOrDescendant(this.element);
+    }
+
+    addBlankProperty(index)
+    {
+        if (index === -1) {
+            // Append to the end.
+            index = this._propertyViews.length;
+        }
+
+        this._propertyPendingStartEditing = this._style.newBlankProperty(index);
+        this.needsLayout();
+    }
+
     spreadsheetCSSStyleDeclarationEditorFocusMoved({direction, movedFromProperty, willRemoveProperty})
     {
         let movedFromIndex = this._propertyViews.indexOf(movedFromProperty);
@@ -167,7 +200,7 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
                     let reverse = false;
                     this._delegate.cssStyleDeclarationEditorStartEditingAdjacentRule(reverse);
                 } else
-                    this._addBlankProperty(movedFromIndex);
+                    this.addBlankProperty(index);
             }
         } else {
             let index = movedFromIndex - 1;
@@ -199,29 +232,9 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
         return this._style.allProperties;
     }
 
-    _addBlankProperty(afterIndex)
-    {
-        let blankProperty = this._style.newBlankProperty(afterIndex);
-        const newlyAdded = true;
-        let propertyView = new WI.SpreadsheetStyleProperty(this, blankProperty, newlyAdded);
-        this.element.append(propertyView.element);
-        this._propertyViews.push(propertyView);
-        propertyView.nameTextField.startEditing();
-    }
-
-    _isFocused()
-    {
-        let focusedElement = document.activeElement;
-
-        if (!focusedElement || focusedElement.tagName === "BODY")
-            return false;
-
-        return focusedElement.isSelfOrDescendant(this.element);
-    }
-
     _propertiesChanged(event)
     {
-        if (this._isFocused()) {
+        if (this.isFocused()) {
             for (let propertyView of this._propertyViews)
                 propertyView.updateClassNames();
         } else
index 59c59b7..ce71d0b 100644 (file)
@@ -37,6 +37,7 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
         this._style = style;
         this._propertiesEditor = null;
         this._selectorElements = [];
+        this._wasFocused = false;
     }
 
     // Public
@@ -54,7 +55,7 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
     {
         super.initialLayout();
 
-        this._headerElement = document.createElement("span");
+        this._headerElement = document.createElement("div");
         this._headerElement.classList.add("header");
 
         this._originElement = document.createElement("span");
@@ -65,6 +66,11 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
         this._selectorElement.classList.add("selector");
         this._headerElement.append(this._selectorElement);
 
+        let openBrace = document.createElement("span");
+        openBrace.classList.add("open-brace");
+        openBrace.textContent = " {";
+        this._headerElement.append(openBrace);
+
         if (this._style.selectorEditable) {
             this._selectorTextField = new WI.SpreadsheetSelectorField(this, this._selectorElement);
             this._selectorElement.tabIndex = 0;
@@ -73,15 +79,11 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
         this._propertiesEditor = new WI.SpreadsheetCSSStyleDeclarationEditor(this, this._style);
         this._propertiesEditor.element.classList.add("properties");
 
-        let openBrace = document.createElement("span");
-        openBrace.classList.add("open-brace");
-        openBrace.textContent = " {";
-
         let closeBrace = document.createElement("span");
         closeBrace.classList.add("close-brace");
         closeBrace.textContent = "}";
 
-        this._element.append(this._createMediaHeader(), this._headerElement, openBrace);
+        this._element.append(this._createMediaHeader(), this._headerElement);
         this.addSubview(this._propertiesEditor);
         this._propertiesEditor.needsLayout();
         this._element.append(closeBrace);
@@ -90,6 +92,11 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
             this._element.classList.add("locked");
         else if (!this._style.ownerRule)
             this._element.classList.add("selector-locked");
+
+        if (this._style.editable) {
+            this.element.addEventListener("click", this._handleClick.bind(this));
+            this.element.addEventListener("mousedown", this._handleMouseDown.bind(this));
+        }
     }
 
     layout()
@@ -335,6 +342,33 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
 
         return mediaElement;
     }
+
+    _handleMouseDown(event)
+    {
+        this._wasFocused = this._propertiesEditor.isFocused();
+    }
+
+    _handleClick(event)
+    {
+        if (this._wasFocused)
+            return;
+
+        event.stop();
+
+        if (event.target.classList.contains(WI.SpreadsheetStyleProperty.StyleClassName)) {
+            let propertyIndex = parseInt(event.target.dataset.propertyIndex);
+            this._propertiesEditor.addBlankProperty(propertyIndex + 1);
+            return;
+        }
+
+        if (event.target.isSelfOrDescendant(this._headerElement)) {
+            this._propertiesEditor.addBlankProperty(0);
+            return;
+        }
+
+        const appendAfterLast = -1;
+        this._propertiesEditor.addBlankProperty(appendAfterLast);
+    }
 };
 
 WI.SpreadsheetCSSStyleDeclarationSection.MatchedSelectorElementStyleClassName = "matched";
index 0d0f840..f1fdf07 100644 (file)
@@ -25,7 +25,7 @@
 
 WI.SpreadsheetStyleProperty = class SpreadsheetStyleProperty extends WI.Object
 {
-    constructor(delegate, property, newlyAdded = false)
+    constructor(delegate, property, index)
     {
         super();
 
@@ -33,8 +33,8 @@ WI.SpreadsheetStyleProperty = class SpreadsheetStyleProperty extends WI.Object
 
         this._delegate = delegate || null;
         this._property = property;
-        this._newlyAdded = newlyAdded;
         this._element = document.createElement("div");
+        this._element.dataset.propertyIndex = index;
 
         this._nameElement = null;
         this._valueElement = null;
@@ -86,7 +86,7 @@ WI.SpreadsheetStyleProperty = class SpreadsheetStyleProperty extends WI.Object
             return false;
         };
 
-        let classNames = ["property"];
+        let classNames = [WI.SpreadsheetStyleProperty.StyleClassName];
 
         if (this._property.overridden)
             classNames.push("overridden");
@@ -199,11 +199,12 @@ WI.SpreadsheetStyleProperty = class SpreadsheetStyleProperty extends WI.Object
         let propertyName = this._nameTextField.value.trim();
         let propertyValue = this._valueTextField.value.trim();
         let willRemoveProperty = false;
+        let newlyAdded = this._valueTextField.valueBeforeEditing === "";
 
         // Remove a property with an empty name or value. However, a newly added property
         // has an empty name and value at first. Don't remove it when moving focus from
         // the name to the value for the first time.
-        if (!propertyName || (!this._newlyAdded && !propertyValue))
+        if (!propertyName || (!newlyAdded && !propertyValue))
             willRemoveProperty = true;
 
         let isEditingName = textField === this._nameTextField;
@@ -211,9 +212,6 @@ WI.SpreadsheetStyleProperty = class SpreadsheetStyleProperty extends WI.Object
         if (!isEditingName && !willRemoveProperty)
             this._renderValue(propertyValue);
 
-        if (propertyName && isEditingName)
-            this._newlyAdded = false;
-
         if (direction === "forward") {
             if (isEditingName && !willRemoveProperty) {
                 // Move focus from the name to the value.
@@ -488,4 +486,6 @@ WI.SpreadsheetStyleProperty = class SpreadsheetStyleProperty extends WI.Object
     }
 };
 
+WI.SpreadsheetStyleProperty.StyleClassName = "property";
+
 WI.SpreadsheetStyleProperty.CommitCoalesceDelay = 250;
index 73c2bd6..4f8e44d 100644 (file)
@@ -46,7 +46,7 @@ WI.SpreadsheetTextField = class SpreadsheetTextField
         this._element.addEventListener("input", this._handleInput.bind(this));
 
         this._editing = false;
-        this._startEditingValue = "";
+        this._valueBeforeEditing = "";
     }
 
     // Public
@@ -58,6 +58,8 @@ WI.SpreadsheetTextField = class SpreadsheetTextField
     get value() { return this._element.textContent; }
     set value(value) { this._element.textContent = value; }
 
+    get valueBeforeEditing() { return this._valueBeforeEditing; }
+
     get suggestionHint()
     {
         return this._suggestionHintElement.textContent;
@@ -83,7 +85,7 @@ WI.SpreadsheetTextField = class SpreadsheetTextField
             this._delegate.spreadsheetTextFieldWillStartEditing(this);
 
         this._editing = true;
-        this._startEditingValue = this.value;
+        this._valueBeforeEditing = this.value;
 
         this._element.classList.add("editing");
         this._element.contentEditable = "plaintext-only";
@@ -102,7 +104,7 @@ WI.SpreadsheetTextField = class SpreadsheetTextField
             return;
 
         this._editing = false;
-        this._startEditingValue = "";
+        this._valueBeforeEditing = "";
         this._element.classList.remove("editing");
         this._element.contentEditable = false;
 
@@ -169,8 +171,8 @@ WI.SpreadsheetTextField = class SpreadsheetTextField
 
     _discardChange()
     {
-        if (this._startEditingValue !== this.value) {
-            this.value = this._startEditingValue;
+        if (this._valueBeforeEditing !== this.value) {
+            this.value = this._valueBeforeEditing;
             this._selectText();
 
             if (this._delegate && typeof this._delegate.spreadsheetTextFieldDidChange === "function")