Bug 147579 - Web Inspector: Add autocomplete controller for Visual property editors
Summary: Web Inspector: Add autocomplete controller for Visual property editors
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks: 147563 147570 147578 147711
  Show dependency treegraph
 
Reported: 2015-08-03 11:50 PDT by Devin Rousso
Modified: 2015-08-14 11:44 PDT (History)
8 users (show)

See Also:


Attachments
Patch (8.70 KB, patch)
2015-08-03 11:58 PDT, Devin Rousso
timothy: review+
Details | Formatted Diff | Diff
Patch (8.62 KB, patch)
2015-08-14 00:19 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2015-08-03 11:50:46 PDT
For use with properties that are able to have autocompletion in the new Visual style details panel in the CSS sidebar.
Comment 1 Devin Rousso 2015-08-03 11:58:21 PDT
Created attachment 258096 [details]
Patch
Comment 2 Timothy Hatcher 2015-08-11 22:57:10 PDT
Comment on attachment 258096 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=258096&action=review

> Source/WebInspectorUI/UserInterface/Controllers/VisualStyleCompletionsController.js:35
> +        if (this._delegate && typeof this._delegate.completionSuggestionsOnItemCreated === "function")
> +            this.completionSuggestionsOnItemCreated = this._delegate.completionSuggestionsOnItemCreated;

Delegates should be prefixed with the class name. So visualStyleCompletionsControllerCompletionSuggestionsOnItemCreated. It is also bad mojo to pass through a delegate. It would be better to either expose suggestionsView publicly so some other object can be its delegate, so likely the best option is to just bounce the delegate call though a similar delegate here. Delegates should also take "this" as the first argument, which ties to the reason the class name is in the method. It should not be assumes that the delegate is a 1-to-1 relationship, and they might need to know who is calling them.

Also completionSuggestionsOnItemCreated does not make much sense to me currently. Would visualStyleCompletionsControllerCustomizeCompletionElement make sense?

> Source/WebInspectorUI/UserInterface/Views/CompletionSuggestionsView.js:180
> +                this._delegate.completionSuggestionsOnItemCreated(itemElement, completions[i]);

This would have a new method signature that includes the class name. It should also pass this to that method. So:

this._delegate.completionSuggestionsViewCustomizeCompletionElement(this, itemElement, completions[i]);

Then VisualStyleCompletionsController would implement that, and fire off visualStyleCompletionsControllerCustomizeCompletionElement.
Comment 3 Devin Rousso 2015-08-14 00:19:58 PDT
Created attachment 258992 [details]
Patch
Comment 4 Radar WebKit Bug Importer 2015-08-14 00:20:47 PDT
<rdar://problem/22283821>
Comment 5 WebKit Commit Bot 2015-08-14 11:44:10 PDT
Comment on attachment 258992 [details]
Patch

Clearing flags on attachment: 258992

Committed r188480: <http://trac.webkit.org/changeset/188480>
Comment 6 WebKit Commit Bot 2015-08-14 11:44:15 PDT
All reviewed patches have been landed.  Closing bug.