Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1611783002:
Devtools Console: Tab to autocomplete and enter to execute command (Closed)
|
Created:
4 years, 11 months ago by samli Modified:
4 years, 11 months ago Reviewers:
pfeldman CC:
apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, chromium-reviews, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, pfeldman, sergeyv+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDevtools Console: Tab to autocomplete and enter to execute command
Redone to avoid affecting suggest box in Sources pane.
BUG=578901
Committed: https://crrev.com/71f4ff03d781881d1e661e507f7b48d5f949651e
Cr-Commit-Position: refs/heads/master@{#370912}
Patch Set 1 #
Total comments: 4
Patch Set 2 : #
Total comments: 6
Patch Set 3 : #Messages
Total messages: 18 (5 generated)
samli@chromium.org changed reviewers: + pfeldman@chromium.org
The CQ bit was checked by samli@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1611783002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1611783002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1611783002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js (right): https://codereview.chromium.org/1611783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:291: handled = this._suggestBox.enterKeyPressed(); Do you want to dispatch tabKeyPressed otherwise? https://codereview.chromium.org/1611783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:323: if (!handled && this.isSuggestBoxVisible() && event.keyIdentifier !== "Enter") isEnterKey(event) is how we check for Enter. Why don't you let Enter in suggestBox? I might need to poke around this code to figure out what it does. It carries legacy :(
https://codereview.chromium.org/1611783002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js (right): https://codereview.chromium.org/1611783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:291: handled = this._suggestBox.enterKeyPressed(); On 2016/01/21 at 19:23:38, pfeldman wrote: > Do you want to dispatch tabKeyPressed otherwise? Yes, done. https://codereview.chromium.org/1611783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:323: if (!handled && this.isSuggestBoxVisible() && event.keyIdentifier !== "Enter") On 2016/01/21 at 19:23:38, pfeldman wrote: > isEnterKey(event) is how we check for Enter. Why don't you let Enter in suggestBox? I might need to poke around this code to figure out what it does. It carries legacy :( Done. Allowing enter in suggest box will do the old behaviour, (accept first suggestion in suggest box), then consume the event we have here, meaning it won't propagate the event up to ConsoleView._promptKeyDown() where it handles Enter key to execute command.
https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js (right): https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:289: // Accept autocomplete suggestion. Looked at the code. So you should leave this as is, but in the tabKeyPressed, you can replace the _completeCommonPrefix with the acceptSuggestion. That'll replace present behavior with what you are aiming for. https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:325: if (!handled && this.isSuggestBoxVisible() && !isEnterKey(event)) You should simply start onKeyDown with if (isEnterKey(event)) return;
https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js (right): https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:289: // Accept autocomplete suggestion. On 2016/01/22 at 01:09:08, pfeldman wrote: > Looked at the code. So you should leave this as is, but in the tabKeyPressed, you can replace the _completeCommonPrefix with the acceptSuggestion. That'll replace present behavior with what you are aiming for. Done - used acceptAutoComplete() instead since that's what we want. https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:325: if (!handled && this.isSuggestBoxVisible() && !isEnterKey(event)) On 2016/01/22 at 01:09:08, pfeldman wrote: > You should simply start onKeyDown with > > if (isEnterKey(event)) > return; Done.
https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js (right): https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:299: case "Right": Or even better add Tab to the bunch and it'll do the same thing. Maybe we don't even need the isCaret check because i could not make it trigger to false.
https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js (right): https://codereview.chromium.org/1611783002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/TextPrompt.js:299: case "Right": On 2016/01/22 at 01:21:20, pfeldman wrote: > Or even better add Tab to the bunch and it'll do the same thing. Maybe we don't even need the isCaret check because i could not make it trigger to false. StylesSidebarPane overrides tabKeyPressed(). Also able to trigger isCaretAtEnd() check by typing then left, right keys.
lgtm
The CQ bit was checked by samli@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1611783002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1611783002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Devtools Console: Tab to autocomplete and enter to execute command Redone to avoid affecting suggest box in Sources pane. BUG=578901 ========== to ========== Devtools Console: Tab to autocomplete and enter to execute command Redone to avoid affecting suggest box in Sources pane. BUG=578901 Committed: https://crrev.com/71f4ff03d781881d1e661e507f7b48d5f949651e Cr-Commit-Position: refs/heads/master@{#370912} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/71f4ff03d781881d1e661e507f7b48d5f949651e Cr-Commit-Position: refs/heads/master@{#370912} |
