Issue 5440001: Enable to show the keyboard overlay by shortcut key.

Issue 5440001: Enable to show the keyboard overlay by shortcut key. (Closed)

Created:
10 years ago by mazda

Modified:
9 years, 7 months ago

Reviewers:
xiyuan

CC:
chromium-reviews, ben+cc_chromium.org, Paweł Hajdan Jr., satorux1

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

Enable to show the keyboard overlay by shortcut key. BUG=chromium-os:6268 TEST=manually on the netbook Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67962

Patch Set 6 : '' #

Created: 10 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+148 lines, -0 lines)			Patch
M	chrome/app/chrome_command_ids.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/browser/ui/browser.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/browser/ui/browser.cc	View	1 2 3 4 5	3 chunks	+6 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/browser_window.h	View		1 chunk	+5 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/views/accelerator_table_gtk.cc	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/views/frame/browser_view.h	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/views/frame/browser_view.cc	View	1 2 3 4 5	2 chunks	+10 lines, -0 lines	0 comments	Download
A	chrome/browser/ui/views/keyboard_overlay_delegate.h	View		1 chunk	+39 lines, -0 lines	0 comments	Download
A	chrome/browser/ui/views/keyboard_overlay_delegate.cc	View		1 chunk	+70 lines, -0 lines	0 comments	Download
M	chrome/chrome_browser.gypi	View	1 2 3 4 5	4 chunks	+7 lines, -0 lines	0 comments	Download
M	chrome/test/test_browser_window.h	View	1 2 3 4	1 chunk	+3 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

mazda

Hello Xiyuan, Can I ask you to review my change? The keyboard overlay is a ...

10 years ago (2010-12-01 13:14:50 UTC) #1

xiyuan

LGTM http://codereview.chromium.org/5440001/diff/15001/chrome/browser/ui/browser.cc File chrome/browser/ui/browser.cc (right): http://codereview.chromium.org/5440001/diff/15001/chrome/browser/ui/browser.cc#newcode1466 chrome/browser/ui/browser.cc:1466: window_->ShowKeyboardOverlay(window()->GetNativeHandle()); nit: It's better to be consistent, either ...

10 years ago (2010-12-01 17:20:41 UTC) #2

xiyuan

Please make sure that ESC would dismiss the keyboard overlay. On 2010/12/01 17:20:41, xiyuan wrote: ...

10 years ago (2010-12-01 17:26:39 UTC) #3

mazda

10 years ago (2010-12-02 04:13:31 UTC) #4

Thank you for the review.

> Please make sure that ESC would dismiss the keyboard overlay.
Yes, I confirmed that ESC dismissed the keyboard overlay.

http://codereview.chromium.org/5440001/diff/15001/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

http://codereview.chromium.org/5440001/diff/15001/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:1466:
window_->ShowKeyboardOverlay(window()->GetNativeHandle());
On 2010/12/01 17:20:41, xiyuan wrote:
> nit: It's better to be consistent, either use window_ in all places or use
> window().

Done.

Expand Messages | Collapse Messages