Web Inspector: Styles: shift-clicking on a property should extend selection
authornvasilyev@apple.com <nvasilyev@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 22:28:52 +0000 (22:28 +0000)
committernvasilyev@apple.com <nvasilyev@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Nov 2018 22:28:52 +0000 (22:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191575
<rdar://problem/46012855>

Reviewed by Devin Rousso.

When there's at least one property is selected, Shift-clicking another property should extend the selection.
Pressing Up or Down keys while holding Shift also extends the selection.

Remove unnecessary `_startedSelection`. Use `hasSelectedProperties` and `_isMousePressed` instead.

* UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:
`_isMousePressed` was set to `true` by mistake, causing numerous assertion fails.

(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.set focusIndex):
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.hasSelectedProperties): Renamed from _hasSelectedProperties.
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.extendSelectedProperties): Added.
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetStylePropertyMouseLeave): Removed.
(WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetStylePropertyCopy):

(WI.SpreadsheetCSSStyleDeclarationEditor.prototype._handleKeyDown):
Implement Shift-ArrowUp/ArrowDown keys.

* UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:
(WI.SpreadsheetCSSStyleDeclarationSection):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetCSSStyleDeclarationEditorPropertyBlur):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetCSSStyleDeclarationEditorPropertyMouseLeave): Removed.
This was only used to set _startedSelection.

(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleMouseDown):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleWindowClick):
(WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleClick):
Remove _startedSelection and use hasSelectedProperties method instead.

* UserInterface/Views/SpreadsheetStyleProperty.js:
(WI.SpreadsheetStyleProperty):
Remove spreadsheetStylePropertyMouseLeave since it is no longer used.

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

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js
Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js

index be4e9bb..c358155 100644 (file)
@@ -1,3 +1,43 @@
+2018-11-14  Nikita Vasilyev  <nvasilyev@apple.com>
+
+        Web Inspector: Styles: shift-clicking on a property should extend selection
+        https://bugs.webkit.org/show_bug.cgi?id=191575
+        <rdar://problem/46012855>
+
+        Reviewed by Devin Rousso.
+
+        When there's at least one property is selected, Shift-clicking another property should extend the selection.
+        Pressing Up or Down keys while holding Shift also extends the selection.
+
+        Remove unnecessary `_startedSelection`. Use `hasSelectedProperties` and `_isMousePressed` instead.
+
+        * UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:
+        `_isMousePressed` was set to `true` by mistake, causing numerous assertion fails.
+
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.set focusIndex):
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.hasSelectedProperties): Renamed from _hasSelectedProperties.
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.extendSelectedProperties): Added.
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetStylePropertyMouseLeave): Removed.
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.spreadsheetStylePropertyCopy):
+
+        (WI.SpreadsheetCSSStyleDeclarationEditor.prototype._handleKeyDown):
+        Implement Shift-ArrowUp/ArrowDown keys.
+
+        * UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:
+        (WI.SpreadsheetCSSStyleDeclarationSection):
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetCSSStyleDeclarationEditorPropertyBlur):
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype.spreadsheetCSSStyleDeclarationEditorPropertyMouseLeave): Removed.
+        This was only used to set _startedSelection.
+
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleMouseDown):
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleWindowClick):
+        (WI.SpreadsheetCSSStyleDeclarationSection.prototype._handleClick):
+        Remove _startedSelection and use hasSelectedProperties method instead.
+
+        * UserInterface/Views/SpreadsheetStyleProperty.js:
+        (WI.SpreadsheetStyleProperty):
+        Remove spreadsheetStylePropertyMouseLeave since it is no longer used.
+
 2018-11-14  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Canvas: send a call stack with each action instead of an array of call frames
index b42a44c..98b3d19 100644 (file)
@@ -320,6 +320,11 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
         this.needsLayout();
     }
 
+    hasSelectedProperties()
+    {
+        return !isNaN(this._anchorIndex) && !isNaN(this._focusIndex);
+    }
+
     selectProperties(anchorIndex, focusIndex)
     {
         console.assert(anchorIndex < this._propertyViews.length, `anchorIndex (${anchorIndex}) is greater than the last property index (${this._propertyViews.length})`);
@@ -348,6 +353,11 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
         this._suppressBlur = false;
     }
 
+    extendSelectedProperties(focusIndex)
+    {
+        this.selectProperties(this._anchorIndex, focusIndex);
+    }
+
     deselectProperties()
     {
         for (let propertyView  of this._propertyViews)
@@ -394,11 +404,6 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
         this._delegate.spreadsheetCSSStyleDeclarationEditorPropertyMouseEnter(event, property);
     }
 
-    spreadsheetStylePropertyMouseLeave(event, property)
-    {
-        this._delegate.spreadsheetCSSStyleDeclarationEditorPropertyMouseLeave(event, property);
-    }
-
     spreadsheetStylePropertyFocusMoved(propertyView, {direction, willRemoveProperty})
     {
         this._updatePropertiesStatus();
@@ -455,7 +460,7 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
 
     spreadsheetStylePropertyCopy(event)
     {
-        if (!this._hasSelectedProperties())
+        if (!this.hasSelectedProperties())
             return;
 
         let formattedProperties = [];
@@ -499,26 +504,26 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
 
     _handleKeyDown(event)
     {
+        if (!this.hasSelectedProperties() || !this._propertyViews.length)
+            return;
+
         if (event.key === "ArrowUp" || event.key === "ArrowDown") {
             let delta = event.key === "ArrowUp" ? -1 : 1;
             let focusIndex = Number.constrain(this._focusIndex + delta, 0, this._propertyViews.length - 1);
 
-            this.selectProperties(focusIndex, focusIndex);
+            if (event.shiftKey)
+                this.extendSelectedProperties(focusIndex);
+            else
+                this.selectProperties(focusIndex, focusIndex);
+
             event.stop();
         } else if (event.key === "Tab" || event.key === "Enter") {
-            if (!this._hasSelectedProperties())
-                return;
-
             let property = this._propertyViews[this._focusIndex];
             if (property && property.enabled) {
                 event.stop();
                 property.nameTextField.startEditing();
             }
-
         } else if (event.key === "Backspace") {
-            if (!this._hasSelectedProperties())
-                return;
-
             let [startIndex, endIndex] = this.selectionRange;
 
             let propertyIndexToSelect = NaN;
@@ -538,9 +543,6 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
             event.stop();
 
         } else if ((event.code === "Space" && !event.shiftKey && !event.metaKey && !event.ctrlKey) || (event.key === "/" && event.commandOrControlKey && !event.shiftKey)) {
-            if (!this._hasSelectedProperties())
-                return;
-
             let [startIndex, endIndex] = this.selectionRange;
 
             // Toggle the first selected property and set this state to all selected properties.
@@ -552,12 +554,9 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
                 propertyView.update();
             }
 
-            event.preventDefault();
+            event.stop();
 
         } else if (event.key === "a" && event.commandOrControlKey) {
-            // FIXME: Check this.editing instead of _hasSelectedProperties() once <https://webkit.org/b/191567> is resolved.
-            if (!this._hasSelectedProperties() || !this._propertyViews.length)
-                return;
 
             this.selectProperties(0, this._propertyViews.length - 1);
             event.stop();
@@ -566,11 +565,6 @@ WI.SpreadsheetCSSStyleDeclarationEditor = class SpreadsheetCSSStyleDeclarationEd
             this.deselectProperties();
     }
 
-    _hasSelectedProperties()
-    {
-        return !isNaN(this._anchorIndex) && !isNaN(this._focusIndex);
-    }
-
     _editablePropertyAfter(propertyIndex)
     {
         for (let index = propertyIndex + 1; index < this._propertyViews.length; index++) {
index 120f7fa..adf3500 100644 (file)
@@ -49,9 +49,8 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
         this._shouldFocusSelectorElement = false;
         this._wasEditing = false;
 
-        this._isMousePressed = true;
+        this._isMousePressed = false;
         this._mouseDownIndex = NaN;
-        this._startedSelection = false;
     }
 
     // Public
@@ -217,24 +216,18 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
 
     spreadsheetCSSStyleDeclarationEditorPropertyBlur(event, property)
     {
-        if (!this._startedSelection)
+        if (!this._isMousePressed)
             this._propertiesEditor.deselectProperties();
     }
 
     spreadsheetCSSStyleDeclarationEditorPropertyMouseEnter(event, property)
     {
-        if (this._isMousePressed && this._startedSelection) {
+        if (this._isMousePressed) {
             let index = parseInt(property.element.dataset.propertyIndex);
             this._propertiesEditor.selectProperties(this._mouseDownIndex, index);
         }
     }
 
-    spreadsheetCSSStyleDeclarationEditorPropertyMouseLeave(event, property)
-    {
-        if (this._isMousePressed)
-            this._startedSelection = true;
-    }
-
     applyFilter(filterText)
     {
         this._filterText = filterText;
@@ -471,7 +464,6 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
             return;
 
         this._isMousePressed = true;
-        this._startedSelection = false;
 
         // Disable text selection on mousemove.
         event.preventDefault();
@@ -480,21 +472,24 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
         if (this._wasEditing && document.activeElement)
             document.activeElement.blur();
 
-        window.addEventListener("mouseup", this._handleWindowMouseUp.bind(this), {capture: true, once: true});
+        // Prevent name/value fields from editing when properties selected.
+        window.addEventListener("click", this._handleWindowClick.bind(this), {capture: true, once: true});
 
         let propertyIndex = parseInt(propertyElement.dataset.propertyIndex);
-        this._propertiesEditor.deselectProperties();
-        this._mouseDownIndex = propertyIndex;
+        if (event.shiftKey && this._propertiesEditor.hasSelectedProperties())
+            this._propertiesEditor.extendSelectedProperties(propertyIndex);
+        else
+            this._propertiesEditor.deselectProperties();
 
+        this._mouseDownIndex = propertyIndex;
         this._element.classList.add("selecting");
     }
 
-    _handleWindowMouseUp(event)
+    _handleWindowClick(event)
     {
-        if (this._startedSelection) {
+        if (this._propertiesEditor.hasSelectedProperties()) {
             // Don't start editing name/value if there's selection.
             event.stop();
-            this._startedSelection = false;
         }
 
         this._isMousePressed = false;
@@ -505,7 +500,7 @@ WI.SpreadsheetCSSStyleDeclarationSection = class SpreadsheetCSSStyleDeclarationS
 
     _handleClick(event)
     {
-        if (this._wasEditing || this._startedSelection)
+        if (this._wasEditing || this._propertiesEditor.hasSelectedProperties())
             return;
 
         if (window.getSelection().type === "Range")
index dd418f5..91b49d4 100644 (file)
@@ -62,10 +62,6 @@ WI.SpreadsheetStyleProperty = class SpreadsheetStyleProperty extends WI.Object
                 this._delegate.spreadsheetStylePropertyMouseEnter(event, this);
             });
 
-            this._element.addEventListener("mouseleave", (event) => {
-                this._delegate.spreadsheetStylePropertyMouseLeave(event, this);
-            });
-
             this._element.copyHandler = this;
         }
     }