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:

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-	Details Formatted Diff Diff
[PATCH] Comments addressed (42.65 KB, patch) 2011-01-14 07:10 PST, Alexander Pavlov (apavlov)	no flags	Details Formatted Diff Diff
[PATCH] Inherited TextPrompt to override key handlers (45.02 KB, patch) 2011-01-14 08:51 PST, Alexander Pavlov (apavlov)	pfeldman: review-	Details Formatted Diff Diff
[PATCH] Comments from pfeldman addressed (39.65 KB, patch) 2011-01-17 03:04 PST, Alexander Pavlov (apavlov)	pfeldman: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

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

Landed with minor fixes: http://trac.webkit.org/changeset/76116

Note You need to log in before you can comment on or make changes to this bug.