1) Remove userEnteredText from delegate interface. Pass prefix to updateSuggestions instead. 2) Clarify that suggestions array is never null. 3) Add option to align suggestion box to element specified during construction. 4) Avoid possible NPE in StylesSidebarPane. 5) Add some JSDocs to CSSMetadata and StylesSidebarPane.CSSPropertyPrompt. 6) Remove unused parameter of StylesSidebarPane.CSSPropertyPrompt constructor.
Created attachment 191203 [details] Patch
Comment on attachment 191203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191203&action=review It is quite hard to track the changes in this patch even for the author of this code. Let us separate the patch into a few others, and not land provisional code (you can make the respective changes in the patch that uses the new code.) > Source/WebCore/inspector/front-end/StylesSidebarPane.js:2649 > + * @param {?WebInspector.CSSMetadata} cssCompletions Why is it nullable??? > Source/WebCore/inspector/front-end/StylesSidebarPane.js:-2714 > - if (!word) Unnecessary change. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:2730 > + var cssCompletions = this._cssCompletions; Ditto. > Source/WebCore/inspector/front-end/SuggestBox.js:60 > + this._anchorElement = anchorElement; This field is unused, hence may be removed altogether. > Source/WebCore/inspector/front-end/SuggestBox.js:98 > + * @param {AnchorBox=} anchorBox Any cases where the anchorBox is not supplied to _updateBoxPosition()? > Source/WebCore/inspector/front-end/SuggestBox.js:330 > + if (this._canShowBox(completions, !!canShowForSingleItem, userEnteredText)) { Why "!!canShowForSingleItem" if it is mandatory?
Comment on attachment 191203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191203&action=review >> Source/WebCore/inspector/front-end/SuggestBox.js:60 >> + this._anchorElement = anchorElement; > > This field is unused, hence may be removed altogether. _anchorElement is now used in _updateBoxPosition >> Source/WebCore/inspector/front-end/SuggestBox.js:98 >> + * @param {AnchorBox=} anchorBox > > Any cases where the anchorBox is not supplied to _updateBoxPosition()? My next patch uses SuggestionBox this way. But I think you are right - this change could be separate from refactoring. >> Source/WebCore/inspector/front-end/SuggestBox.js:330 >> + if (this._canShowBox(completions, !!canShowForSingleItem, userEnteredText)) { > > Why "!!canShowForSingleItem" if it is mandatory? oops. That was mandatory for previous signature. I'll revert this change.
Created attachment 191490 [details] Patch
Comment on attachment 191490 [details] Patch Clearing flags on attachment: 191490 Committed r145054: <http://trac.webkit.org/changeset/145054>
All reviewed patches have been landed. Closing bug.