Created attachment 257548 [details] [Animated GIF] Option+Up/Down keys don't increment/decrement :( In Styles panel it's possible to increment/decrement numbers by pressing Option+Up/Down arrow keys. It would be very useful for editing SVG too. The attached animated GIF was recorded on this page http://codepen.io/sol0mka/full/jpecs/.
<rdar://problem/22005531>
Created attachment 258017 [details] Patch
Created attachment 258131 [details] [Animated GIF] Editing SVG path by pressing Alt + Up/Down The patch works. It feels great to edit SVG paths. Undo doesn't work.
Comment on attachment 258017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258017&action=review > Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:216 > + } else if (result && result.startsWith("modify-")) { What code is responsible for Option Up/Down behavior in the styles panel and CSS resources? We should try to abstract it away. > Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:229 > + var wordRange = range.startContainer.rangeOfWord(range.startOffset, " \xA0\t\n\"':;,/()", element); Already used in WebInspector.BoxModelDetailsSectionRow.StyleValueDelimiters > Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:230 > + var matches = /(.*?)(-?(?:\d+(?:\.\d+)?|\.\d+))(.*)/.exec(wordRange.toString()); WebInspector.BoxModelDetailsSectionRow.CSSNumberRegex
Comment on attachment 258017 [details] Patch Another bug: pressing down without holding Option modifies the number, pressing up doesn't.
Comment on attachment 258017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258017&action=review >> Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:216 >> + } else if (result && result.startsWith("modify-")) { > > What code is responsible for Option Up/Down behavior in the styles panel and CSS resources? We should try to abstract it away. The code responsible for this behavior in the Styles and Resource panels is a CodeMirror addition. CodeMirror.defineExtension("alterNumberInRange", ...) I agree that it would be nice to combine the two, but the CodeMirror version makes extensive use of the CodeMirror APIs and I'd rather not mess with it too much. I can try to merge the two in a later patch.
Created attachment 258156 [details] Patch
Undo with Cmd+Z still doesn't work. (In reply to comment #5) > Another bug: pressing down without holding Option modifies the number, > pressing up doesn't. This one is fixed. Another bug: "42A3|6" ("|" is a text cursor) Pressing Option Up/Down should change the second number (36), not the first one.
(In reply to comment #8) > Another bug: > "42A3|6" ("|" is a text cursor) > Pressing Option Up/Down should change the second number (36), not the first > one. Good catch! Completely forgot that you could do this.
(In reply to comment #8) > Undo with Cmd+Z still doesn't work. Can you post a video of this or provide some steps to reproduce please? I am not seeing any issues with undo.
Created attachment 258161 [details] Patch
Created attachment 258162 [details] [Video] Cmd+Z isn't working
(In reply to comment #12) > Created attachment 258162 [details] > [Video] Cmd+Z isn't working Hm, this video doesn't work in Safari for me — it gets stuck on "Loading". It should work if you download it.
Making Undo/Redo work would be nice, but I am not sure we have the editing hooks for do that. These are content editable fields, not CodeMirror.
Created attachment 258225 [details] Patch
With the most recent patch applied, I do see that the undo works once the editable field is blurred. It's not the best but it works. Also, this should now also work with inline styles and any other instance of a number in an attribute field.
(In reply to comment #16) > With the most recent patch applied, I do see that the undo works once the > editable field is blurred. It's not the best but it works. Also, this > should now also work with inline styles and any other instance of a number > in an attribute field. Undo when blurred is using the DOM editing undo stack we talked about yesterday.
Comment on attachment 258225 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258225&action=review > Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:219 > + var modifyValue = result.substring(7) === "up" ? 1 : -1; > + if (event.shiftKey) > + modifyValue *= 10; What about the other modifiers we support in CodeMirror? They should be the same. "Alt-Up": alterNumber.bind(null, 1), "Ctrl-Alt-Up": alterNumber.bind(null, 0.1), "Shift-Alt-Up": alterNumber.bind(null, 10), "Alt-PageUp": alterNumber.bind(null, 10), "Shift-Alt-PageUp": alterNumber.bind(null, 100), "Alt-Down": alterNumber.bind(null, -1), "Ctrl-Alt-Down": alterNumber.bind(null, -0.1), "Shift-Alt-Down": alterNumber.bind(null, -10), "Alt-PageDown": alterNumber.bind(null, -10), "Shift-Alt-PageDown": alterNumber.bind(null, -100), > Source/WebInspectorUI/UserInterface/Views/EditingSupport.js:253 > + wordRange.deleteContents(); > + wordRange.insertNode(replacementTextNode); You should be able to use document.execCommand("insertText", false, wordPrefix + replacement + wordSuffix); that would in theory make Undo/Redo work. https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand
Created attachment 258457 [details] Patch
Comment on attachment 258457 [details] Patch Clearing flags on attachment: 258457 Committed r188138: <http://trac.webkit.org/changeset/188138>
All reviewed patches have been landed. Closing bug.