Bug 52212

Summary: Web Inspector: Employ TextPrompt for CSS property name/value autocompletion
Product: WebKit Reporter: Alexander Pavlov (apavlov) <apavlov>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Suggested solution
yurys: review-
[PATCH] Comments addressed
none
[PATCH] Inherited TextPrompt to override key handlers
pfeldman: review-
[PATCH] Comments from pfeldman addressed pfeldman: review+

Alexander Pavlov (apavlov)
Reported 2011-01-11 07:58:52 PST
TextPrompt features text autocompletion, which is a great feature we could use for autocompleting CSS property names/values instead of writing ad-hoc autocompletions.
Attachments
[PATCH] Suggested solution (42.48 KB, patch)
2011-01-13 09:39 PST, Alexander Pavlov (apavlov)
yurys: review-
[PATCH] Comments addressed (42.65 KB, patch)
2011-01-14 07:10 PST, Alexander Pavlov (apavlov)
no flags
[PATCH] Inherited TextPrompt to override key handlers (45.02 KB, patch)
2011-01-14 08:51 PST, Alexander Pavlov (apavlov)
pfeldman: review-
[PATCH] Comments from pfeldman addressed (39.65 KB, patch)
2011-01-17 03:04 PST, Alexander Pavlov (apavlov)
pfeldman: review+
Alexander Pavlov (apavlov)
Comment 1 2011-01-13 09:39:55 PST
Created attachment 78819 [details] [PATCH] Suggested solution
Yury Semikhatsky
Comment 2 2011-01-14 06:02:43 PST
Comment on attachment 78819 [details] [PATCH] Suggested solution View in context: https://bugs.webkit.org/attachment.cgi?id=78819&action=review > Source/WebCore/inspector/front-end/CSSKeywordCompletions.js:2 > + * Copyright (C) 2010 Google Inc. All rights reserved. 2011 > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1630 > + return { keyIdentifier: "___", handled: false }; ___ looks weird, can you use null or undefined instead? > Source/WebCore/inspector/front-end/TextPrompt.js:76 > + if (this._customKeyDownHandler) { Let's extract Up, Down and Tab handlers into their own methods to allow overriding them. r- for this. > Source/WebCore/inspector/front-end/TextPrompt.js:400 > + _tabKeyPressed: function(reverse) This method can be inlined.
Alexander Pavlov (apavlov)
Comment 3 2011-01-14 07:10:26 PST
Created attachment 78939 [details] [PATCH] Comments addressed Introduced a customKeyHandlers parameter into the TextPrompt ctor to pass in custom Up, Down, and Tab handlers.
Alexander Pavlov (apavlov)
Comment 4 2011-01-14 08:51:58 PST
Created attachment 78946 [details] [PATCH] Inherited TextPrompt to override key handlers
Pavel Feldman
Comment 5 2011-01-17 01:45:48 PST
Comment on attachment 78946 [details] [PATCH] Inherited TextPrompt to override key handlers View in context: https://bugs.webkit.org/attachment.cgi?id=78946&action=review r- is for managing selection in the sidebar, rest seems fine. > Source/WebCore/inspector/front-end/CSSKeywordCompletions.js:45 > +WebInspector.CSSKeywordCompletions._colors = [ Nit: please compress to ~120 chars per line. > Source/WebCore/inspector/front-end/CSSKeywordCompletions.js:197 > +WebInspector.CSSKeywordCompletions._colorAwareProperties = [ ditto > Source/WebCore/inspector/front-end/CSSKeywordCompletions.js:224 > +WebInspector.CSSKeywordCompletions._propertyKeywordMap = { ditto > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1690 > + finalSelectionRange.setStart(replacementTextNode, 0); I would expect all the selection management code to move to the prompt. Why is it still here? > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1902 > + Nit: blank line.
Alexander Pavlov (apavlov)
Comment 6 2011-01-17 03:03:57 PST
(In reply to comment #5) > (From update of attachment 78946 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=78946&action=review > > r- is for managing selection in the sidebar, rest seems fine. > > > Source/WebCore/inspector/front-end/CSSKeywordCompletions.js:45 > > +WebInspector.CSSKeywordCompletions._colors = [ > > Nit: please compress to ~120 chars per line. Done. > > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1690 > > + finalSelectionRange.setStart(replacementTextNode, 0); > > I would expect all the selection management code to move to the prompt. Why is it still here? I don't understand why you would expect this. I'd rather understand you expect all the _prompt_ management code move there. But scrolling numeric values is a whole different thing, isn't it? It does not match the TextPrompt responsibilities altogether. > > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1902 > > + > > Nit: blank line. Done.
Alexander Pavlov (apavlov)
Comment 7 2011-01-17 03:04:32 PST
Created attachment 79146 [details] [PATCH] Comments from pfeldman addressed
Alexander Pavlov (apavlov)
Comment 8 2011-01-19 05:26:22 PST
Note You need to log in before you can comment on or make changes to this bug.