Iterating through possible style values with up/down is cool, could a autocomplete combobox work? Good for discoverability.
Let's handle the CSS properties suggestions in this bug, and I've filed bug 66044 for the console prompt.
Created attachment 103635 [details] [PATCH] Suggested solution
Comment on attachment 103635 [details] [PATCH] Suggested solution View in context: https://bugs.webkit.org/attachment.cgi?id=103635&action=review > Source/WebCore/inspector/front-end/StylesSidebarPane.js:956 > + if (!curSection) { Looks like too much copypaste. > Source/WebCore/inspector/front-end/SuggestBox.js:92 > + var bodyElement = this.inputElement.ownerDocument.body; I've seen this in Popover helper. Is there anything to reuse? > Source/WebCore/inspector/front-end/suggestBox.css:57 > +/* Vertical Scrollbar Styles */ Can we reuse existing ones?
(In reply to comment #3) > (From update of attachment 103635 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=103635&action=review > > > Source/WebCore/inspector/front-end/StylesSidebarPane.js:956 > > + if (!curSection) { > > Looks like too much copypaste. Removed dead code. > > Source/WebCore/inspector/front-end/SuggestBox.js:92 > > + var bodyElement = this.inputElement.ownerDocument.body; > > I've seen this in Popover helper. Is there anything to reuse? Yes, extracted into utilities.js. > > Source/WebCore/inspector/front-end/suggestBox.css:57 > > +/* Vertical Scrollbar Styles */ > > Can we reuse existing ones? Done (created 2 separate custom scrollbar styles (for vertical and horizontal ones) in inspector.css and used them both in Popover and SuggestBox). A fixed patch is to be attached shortly.
Created attachment 104048 [details] [PATCH] Comments addressed
Comment on attachment 104048 [details] [PATCH] Comments addressed View in context: https://bugs.webkit.org/attachment.cgi?id=104048&action=review Too much copypaste. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:930 > + if (!curSection) { Could you submit this as a separate change? > Source/WebCore/inspector/front-end/StylesSidebarPane.js:952 > + if (!curSection) { same other change > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1139 > + this._selectorElement.scrollIntoViewIfNeeded(false); same other change > Source/WebCore/inspector/front-end/StylesSidebarPane.js:2047 > + else if (moveDirection === "backward") { same other change > Source/WebCore/inspector/front-end/StylesSidebarPane.js:2248 > + //this._clearAutoComplete(); Should this code be commented? > Source/WebCore/inspector/front-end/SuggestBox.js:32 > + this.parentElement = inputElement.ownerDocument.body; Please make these fields private > Source/WebCore/inspector/front-end/SuggestBox.js:43 > + window.addEventListener("scroll", this._boundOnScroll, true); No need for scroll / resize listeners. > Source/WebCore/inspector/front-end/SuggestBox.js:85 > + const anchorBox = this.inputElement.boxInWindow(window, this.parentElement); Could you style it using CSS instead? > Source/WebCore/inspector/front-end/SuggestBox.js:135 > + if (event.handled) event.handled is not standard. how can this happen? The one setting it should stop propagation instead as you do below. > Source/WebCore/inspector/front-end/SuggestBox.js:146 > + this.tabKeyPressed(event); These all should be private. > Source/WebCore/inspector/front-end/SuggestBox.js:192 > + this.inputElement.pruneEmptyTextNodes(); This looks a lot like copypaste from the TextPrompt. > Source/WebCore/inspector/front-end/SuggestBox.js:222 > + acceptAutoComplete: function(event) This method looks exactly as the one in the TextPrompt. I think the way you should implement it is to extend TextPrompt instead. It could be TextPrompt + drop box. > Source/WebCore/inspector/front-end/SuggestBox.js:378 > + _completionsReady: function(selection, completions) Again giant copypaste?
Created attachment 112319 [details] [PATCH] A major rewrite, which extends the TextPrompt functionality by aggregating SuggestBox
Comment on attachment 112319 [details] [PATCH] A major rewrite, which extends the TextPrompt functionality by aggregating SuggestBox View in context: https://bugs.webkit.org/attachment.cgi?id=112319&action=review > Source/WebCore/inspector/front-end/ConsoleView.js:353 > + if (force && !expressionString) You should nuke the prefix check below instead. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:110 > +WebInspector.StylesSidebarPane.StyleValueDelimiters = " \xA0\t\n\"':;,/("; You should not suggest upon ")" type. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1878 > + if (isEnterKey(event)) { It would be nice if TextPrompt could be configured to handle it internally. Worst case, you should do: if (this._prompt.handleEvent(event)) return; in all entry points. > Source/WebCore/inspector/front-end/TextPrompt.js:605 > + this.completions = completions; please make things private. > Source/WebCore/inspector/front-end/inspector.css:2507 > +.suggest-box { please move this to the newly added textPrompt.css and require it from ConsoleView and ElementsPanel. Alternatively you could make TextPrompt a view, but I am not sure it is worth it.
Created attachment 112344 [details] [PATCH] Comments addressed
Created attachment 112494 [details] [PATCH] Got rid of FocusRestorer
Created attachment 113170 [details] [PATCH] Rebased on the refactored TextPrompt
Created attachment 113459 [details] [PATCH] Rebased on the committed SuggestBox
Comment on attachment 113459 [details] [PATCH] Rebased on the committed SuggestBox View in context: https://bugs.webkit.org/attachment.cgi?id=113459&action=review > Source/WebCore/inspector/front-end/ConsoleView.js:114 > + const suggestBoxConfig = new WebInspector.TextPrompt.SuggestBoxConfig("generic-suggest"); We should not use configs, please add corresponding events to the TextPrompt. SuggestBox should be entirely hidden.
Created attachment 113835 [details] [PATCH] Got rid of SuggestBoxConfig
Comment on attachment 113835 [details] [PATCH] Got rid of SuggestBoxConfig View in context: https://bugs.webkit.org/attachment.cgi?id=113835&action=review > Source/WebCore/inspector/front-end/ConsoleView.js:115 > + this.prompt.setSuggestBoxEnabled("generic-suggest"); enableSuggestBox?
Committed r99407: <http://trac.webkit.org/changeset/99407>
*** Bug 18651 has been marked as a duplicate of this bug. ***