Make it possible to edit property names and values. <rdar://problem/33525247>
Created attachment 321498 [details] Patch Goals of this patch: - Provide a primitive UI to make sure editing works, changes apply live, and CSS doesn’t get corrupted. - Changes in CSSProperty model don’t break the current styles sidebar. Non-goals: - Make a polished UI for editing names and values. I’ll work on it in: - Bug 177314 Web Inspector: Styles Redesign: support undo/redo of manual edits - Bug 177313 Web Inspector: Styles Redesign: hook up autocompletion to property names and values
Attachment 321498 [details] did not pass style-queue: ERROR: Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:196: Line contains single-quote character. [js/syntax] [5] ERROR: Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:322: Line contains single-quote character. [js/syntax] [5] ERROR: Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:328: Line contains single-quote character. [js/syntax] [5] Total errors found: 3 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 321500 [details] [Animated GIF] With patch applied
Created attachment 321501 [details] Patch
Comment on attachment 321501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=321501&action=review > Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:164 > + set name(newName) Nit: You can just use `name`. > Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:166 > + this._name = newName; Nit: Lets bail if things did not change: if (this._name === name) return; this._name = name; ... > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:138 > + nameElement.addEventListener("input", () => { > + this._property.name = nameElement.textContent.trim(); > + }); > + > + valueElement.contentEditable = "plaintext-only"; > + valueElement.addEventListener("input", () => { > + this._property.rawValue = valueElement.textContent.trim(); > + }); As far as I can, by using the "input" event that means we are going to replace the CSS every single keystroke. For our model objects that may be fine, but it does look like that synchronously triggers a backend update. That is probably is not as performant as we want. If I'm typing "color: red" I'm able to type that pretty quickly, all of the intermediate "c", "co", "col", "colo" etc inputs don't need to be sent to the backend because they are about to be obsoleted very very quickly. We probably want to limit/throttle/debounce backend updates. Example strategies could be: • Maximum 1 update every 100ms. => If the user types super fast this would only update at most 10 times a second • Only update if no user change in 100ms. => As long as the user is typing fast there are no updates until they stop typing. The second strategy seems to be what the sidebar does right now. That said, the first strategy seems pretty reasonable. For now this seems fine, but lets add a FIXME somewhere in order to throttle updates.
Comment on attachment 321501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=321501&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:138 >> + }); > > As far as I can, by using the "input" event that means we are going to replace the CSS every single keystroke. For our model objects that may be fine, but it does look like that synchronously triggers a backend update. That is probably is not as performant as we want. If I'm typing "color: red" I'm able to type that pretty quickly, all of the intermediate "c", "co", "col", "colo" etc inputs don't need to be sent to the backend because they are about to be obsoleted very very quickly. > > We probably want to limit/throttle/debounce backend updates. Example strategies could be: > > • Maximum 1 update every 100ms. > => If the user types super fast this would only update at most 10 times a second > > • Only update if no user change in 100ms. > => As long as the user is typing fast there are no updates until they stop typing. > > The second strategy seems to be what the sidebar does right now. That said, the first strategy seems pretty reasonable. > > For now this seems fine, but lets add a FIXME somewhere in order to throttle updates. The existing delay is 250ms. WI.CSSStyleDeclarationTextEditor.CommitCoalesceDelay = 250 I used 250ms debounce for a bit, and it didn't feel too slow. For incrementing/decrementing numeric values using arrow keys and changing colors via color picker I plan to use a shorter delay (~100ms) and throttle instead of debounce.
Created attachment 321592 [details] Patch
Comment on attachment 321592 [details] Patch Clearing flags on attachment: 321592 Committed r222408: <http://trac.webkit.org/changeset/222408>
All reviewed patches have been landed. Closing bug.