If I have a single property within a rule, there's no way for me to highlight it so I can copy/paste it :(
Created attachment 358823 [details] [Animated GIF] Select single property It's possible, but it's cumbersome. 1. Mousedown on the property 2. Move mouse cursor away from the property 3. Hover the property again
*** Bug 191126 has been marked as a duplicate of this bug. ***
(In reply to Nikita Vasilyev from comment #1) > It's possible, but it's cumbersome. Oh wow, yeah that's not fun :( Glad to know that there is some way though :)
Created attachment 361542 [details] Patch
Created attachment 361544 [details] [Video] With patch applied
Comment on attachment 361542 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361542&action=review r=me, nice! > Source/WebInspectorUI/ChangeLog:8 > + Start property selection if mouse cursor moved more than 8px after mousedown. Why 8px? I'm guessing it's related to the size of a single character, but perhaps we should be a bit more "exact" about it. Some explanation would be useful. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:49 > + this._boundHandleWindowMouseMove = this._handleWindowMouseMove.bind(this); Please lazy initialize this when it's first needed (although keep it as null in the constructor). > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:411 > + this._mouseDownPoint = WI.Point.fromEvent(event); We should also define `_mouseDownPoint` as a `null` value in the constructor. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:412 > + window.addEventListener("mousemove", this._boundHandleWindowMouseMove, false); You can drop the `false`. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:433 > + console.assert(this._mouseDownPoint); Style: add a newline after. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:434 > + if (WI.Point.fromEvent(event).distance(this._mouseDownPoint) > 8) { NIT: I'd rather you put the "known" value first and compare it to the current value, like `this._mouseDownPoint.distance(WI.Point.fromEvent(event))`. This way, it's a little less "chaining" and a bit easier to read. NIT: we should invert this and make it an early return. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:435 > + if (!this._propertiesEditor.hasSelectedProperties()) Is it actually possible for `hasSelectedProperties()` to be true? We clear any selection when we "mousedown" so it should be cleared before are able to move the mouse (and therefore fire this listener). > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:436 > + this._propertiesEditor.selectProperties(this._mouseDownIndex, this._mouseDownIndex); Should we check that `_mouseDownIndex` isn't `NaN`? We only set it to a value when there's a property underneath the mouse in "mousedown". I know we set `display: none` when there are no properties, but maybe we should assert just in case.
Comment on attachment 361542 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361542&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:435 >> + if (!this._propertiesEditor.hasSelectedProperties()) > > Is it actually possible for `hasSelectedProperties()` to be true? We clear any selection when we "mousedown" so it should be cleared before are able to move the mouse (and therefore fire this listener). Yes, it's possible if we hover another property before moving 8px.
Created attachment 361570 [details] Patch for landing
Created attachment 361571 [details] Patch for landing
Comment on attachment 361571 [details] Patch for landing Clearing flags on attachment: 361571 Committed r241226: <https://trac.webkit.org/changeset/241226>
All reviewed patches have been landed. Closing bug.
<rdar://problem/47936525>