Darryl James (Gerrit)

unread,

3:55 PM (2 hours ago) 3:55 PM

to Kunal Daftari, Chromium LUCI CQ, Eshwar Stalin, android-bu...@system.gserviceaccount.com, luci-bi...@appspot.gserviceaccount.com, chromium...@chromium.org, Chromium Metrics Reviews, mac-r...@chromium.org, asvitkine...@chromium.org

Attention needed from Eshwar Stalin and Kunal Daftari

Darryl James voted and added 2 comments

Votes added by Darryl James

Code-Review

+1

2 comments

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Darryl James . resolved

lgtm!

Commit Message

Line 11, Patchset 8 (Latest):Reason for revert was that it was failing a test on Mac, fixed that by

Darryl James . resolved

I am really surprised this was not caught in the CQ originally (and also whoops my bad for missing this in the initial review!)

Open in Gerrit

Related details

Attention is currently required from:

Eshwar Stalin
Kunal Daftari

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Kunal Daftari (Gerrit)

unread,

3:56 PM (2 hours ago) 3:56 PM

to Darryl James, Chromium LUCI CQ, Eshwar Stalin, android-bu...@system.gserviceaccount.com, luci-bi...@appspot.gserviceaccount.com, chromium...@chromium.org, Chromium Metrics Reviews, mac-r...@chromium.org, asvitkine...@chromium.org

Attention needed from Eshwar Stalin

Kunal Daftari voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Eshwar Stalin

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Eshwar Stalin (Gerrit)

unread,

3:56 PM (2 hours ago) 3:56 PM

to Kunal Daftari, Darryl James, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, luci-bi...@appspot.gserviceaccount.com, chromium...@chromium.org, Chromium Metrics Reviews, mac-r...@chromium.org, asvitkine...@chromium.org

Attention needed from Kunal Daftari

Eshwar Stalin voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Kunal Daftari

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

4:03 PM (2 hours ago) 4:03 PM

to Kunal Daftari, Eshwar Stalin, Darryl James, android-bu...@system.gserviceaccount.com, luci-bi...@appspot.gserviceaccount.com, chromium...@chromium.org, Chromium Metrics Reviews, mac-r...@chromium.org, asvitkine...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Reland "[Vertical Tabs] Implementing Keyboard Shortcut for Collapsing Tabs"

This is a reland of commit 36969da4015a28d803ca4a3471c41b6ddc952518



Reason for revert was that it was failing a test on Mac, fixed that by

registering the shortcut in accelerators_cocoa.mm.

Fixed: 532001161

Also modifies the chrome branded accelerator based off of:
https://chromium-review.git.corp.google.com/c/chromium/src/+/8052287

Original change's description:
> [Vertical Tabs] Implementing Keyboard Shortcut for Collapsing Tabs
>
> This change implements (Ctrl/Command) + Shift + L as a keyboard shortcut to collapse/expand the vertical tab strip.
>
> We also update the Mac View menu to show the keyboard shortcut.
>
> Bug: 488215856
> Change-Id: I2799165fb34e8eee3cd6e683ff1be8d2f59715f7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8020434
> Commit-Queue: Kunal Daftari <kunald...@google.com>
> Reviewed-by: Darryl James <dlj...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1657564}

Bug: 488215856

Change-Id: I79f66296a5870f4b62294dfb13c7b155058dc93b

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8065785

Reviewed-by: Eshwar Stalin <est...@chromium.org>

Reviewed-by: Darryl James <dlj...@chromium.org>

Commit-Queue: Kunal Daftari <kunald...@google.com>

Cr-Commit-Position: refs/heads/main@{#1659012}

Files:

M chrome/browser/ui/accelerator_table.cc
M chrome/browser/ui/ash/test/accelerator_metadata_unittest.cc
M chrome/browser/ui/browser_actions.cc
M chrome/browser/ui/browser_command_controller.cc
M chrome/browser/ui/cocoa/accelerators_cocoa.mm
M chrome/browser/ui/cocoa/accelerators_cocoa_browsertest.mm
M chrome/browser/ui/cocoa/main_menu_builder.mm
M chrome/browser/ui/tabs/vertical_tab_strip_state_controller_interactive_uitest.cc
M chrome/browser/ui/views/frame/browser_native_widget_mac.mm
M chrome/browser/ui/views/frame/system_menu_model_builder.cc
M chrome/browser/ui/views/frame/system_menu_model_builder.h
M chrome/browser/ui/views/frame/system_menu_model_builder_browsertest.cc
M chrome/browser/ui/views/frame/system_menu_model_delegate.cc
M tools/metrics/actions/actions.xml

Change size: M

Delta: 14 files changed, 204 insertions(+), 13 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Darryl James, +1 by Eshwar Stalin

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Reland "[Vertical Tabs] Implementing Keyboard Shortcut for Collapsing Tabs" [chromium/src : main]

Darryl James (Gerrit)

Darryl James voted and added 2 comments

Votes added by Darryl James

2 comments

Related details

Kunal Daftari (Gerrit)

Kunal Daftari voted Commit-Queue+2

Related details

Eshwar Stalin (Gerrit)

Eshwar Stalin voted Code-Review+1

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information