Background: Selecting, editing and manipulating text in Chrome inspector has always been a joy, but Safari/Webkit has lacked some of those refinements. One issue I brought up previously was in Chrome selecting a css attribute in Chrome inspector automatically selects that text rather then putting the cursor there unless your double click, which as a developer seems the better use case rather than the reverse of having a single click put cursor and double click highlight. While that idea was shot down previously here, I see in the latest Webkit it now selects text the Chrome way by default and the user can disable that in settings. Awesome! Issue: However, in Chrome if I select an attribute such as "3em" with a single click I can then up arrow to increase that to say 4em, 5em, etc, or down arrow to 2em, 1em, -1em etc. which is incredible helpful in easily seeing how a layout changes with a single key but currently does not work in Webkit. With the new default selection, hopefully the up/down for number based items. Happy to provide additional details as needed and loving all the great progress in Webkit Inspector.
In Web Inspector you can do the same by holding Option key. It also works for CSS files in Resources. https://webkit.org/blog/2518/state-of-web-inspector/#styles
Thanks Nikita, though this requests is for allowing adjusting with solely the up/down keys without having to hold additional keys as the extra keys would seem to be an unnecessary more complication process than Chrome's method. Unless Safari requires option because they have future plans for up/down on numbers without a additional keys to carry out a different function?
(In reply to Chris Chiera from comment #2) > Thanks Nikita, though this requests is for allowing adjusting with solely > the up/down keys without having to hold additional keys as the extra keys > would seem to be an unnecessary more complication process than Chrome's > method. > > Unless Safari requires option because they have future plans for up/down on > numbers without a additional keys to carry out a different function? It would be great to simplify this. I don't use this feature that often, so when I do it takes me a few tries to figure out the right modifier.
<rdar://problem/33170633>
Created attachment 323845 [details] Patch Now the modifier keys match Chrome: - Option modifies the value by 0.1 - Shift modifies the value by 10 - Command modifies the value by 100
Created attachment 323846 [details] [Animated GIF] With patch applied Up/down keys behavior outside of the styles sidebar was unchanged (up/down doesn't modify numeric values, requires holding Option key).
Comment on attachment 323845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323845&action=review r-, only because this needs a test for the new EditingSupport function. > Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:244 > + } else I think this would be cleaner as: if (!modified) return; event.preventDefault(); ... Actually, preventDefault gets called on line 250 below, so you should only need the early return. > Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:280 > +WI.modifyNumericValue = function(element, modifyValue) This was existing code, but it would still be good to add a layout test now that it's included in EditingSupport. I think it could use a better name too. I didn't really know what `modifyValue` was, or what this function did to `element` until I got to the new code in SpreadsheetTextField later. What do you think about: WI.incrementElementValue = function(element, amount) { .... } I think "numeric" is implied, but it could be `incrementNumericElementValue` if you think that's more descriptive. Kind of a long name though. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js:236 > + modifyValue /= 10; Instead of multiplying and dividing, you could do: let amount; if (event.metaKey) amount = 100; else if (event.shiftKey) amount = 10; else if (event.altKey) amount = 0.1; else amount = 1; if (event.key === "ArrowDown") amount = -amount;
Comment on attachment 323845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323845&action=review At this point matt reviewed so I'll just leave my comments. >> Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:244 >> + if (modified) { >> + event.preventDefault(); >> + } else > > I think this would be cleaner as: > > if (!modified) > return; > > event.preventDefault(); > ... > > Actually, preventDefault gets called on line 250 below, so you should only need the early return. Style: No braces. Better yet, you could reformat this as: let modified = WI.modifyNumericValue(element, modifyValue); if (!modified) return; event.preventDefault(); ... >> Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:280 >> +WI.modifyNumericValue = function(element, modifyValue) > > This was existing code, but it would still be good to add a layout test now that it's included in EditingSupport. > > I think it could use a better name too. I didn't really know what `modifyValue` was, or what this function did to `element` until I got to the new code in SpreadsheetTextField later. What do you think about: > > WI.incrementElementValue = function(element, amount) > { > .... > } > > I think "numeric" is implied, but it could be `incrementNumericElementValue` if you think that's more descriptive. Kind of a long name though. I think `modifyDelta` or just `delta` would be clearer than `modifyValue`. At first I thought `modifyValue` was the value that was going to be modified. What do you think? You could probably write tests for this. Even though its in Views, its essentially just a Utility function given an element.
Created attachment 323865 [details] Patch
Comment on attachment 323865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323865&action=review r=me, with a minor nit. > Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:240 > + let modified = WI.incrementElementValue(element, modifyValue); Nit: change `modifyValue` to `delta` to be consistent with other caller, and function signature.
Created attachment 323874 [details] Patch
Comment on attachment 323874 [details] Patch Clearing flags on attachment: 323874 Committed r223336: <https://trac.webkit.org/changeset/223336>
All reviewed patches have been landed. Closing bug.