New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 841600 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Add "Lock screen" item to power button menu

Project Member Reported by derat@chromium.org, May 9 2018

Issue description

The power button menu should include an additional "Lock screen" item when it's shown while a user is logged in and the screen is unlocked. chromeos-ui-review discussion/approval: http://g/chromeos-ui-review/qES_Y_xCA6o

It would be nice if the item also included the Search+L accelerator that can be used to lock the screen. We display accelerators in existing menus using lighter, right-justified text.

Looking at the code, I think that the menu currently lists "Power off" followed by "Sign out" (if a user is signed in). Since locking feels conceptually closer to signing out to me, I think it probably makes sense to add the item at the bottom of the menu. Elizabeth or Jenn, does that sound right?

(If the "Sign out" item doesn't already contain the Ctrl+Shift+Q accelerator, we may want to add that at the same time. Hmm, it looks like Ctrl+Shift+Q is also missing from Ctrl+Alt+/.)
 

Comment 1 by minch@chromium.org, May 10 2018

Cc: wutao@chromium.org
"it looks like Ctrl+Shift+Q is also missing from Ctrl+Alt+/"
I guess Ctrl+Shift+Q is in Ctrl+Alt+/ opened menu? in "System & Display Settings" section. Dan, is that what you mean?
+wutao who did the Ctrl+Alt+/ menu to confirm.

Comment 2 by derat@chromium.org, May 10 2018

Hmm, I don't see it there on a stable-channel eve (66.0.3359.137). I see other issues there too (e.g. I'm using Dvorak and everything is wrong; it's like we're rewriting each accelerator to show the corresponding Dvorak key in that position), so I should file separate bugs for that. Sorry for the noise.
Users on Chromebook Central frequently indicate that they never turn off their Chromebooks. This power button menu will help.

Now, we need to figure out a way to teach owners of convertible devices such as the Acer R11 that they actually do have a power button!

Addition of the "Lock screen" option is important because we encourage proper security practices, especially for kids who are getting pranked by other kids. We want them to lock their screens whenever they leave their Chromebook unattended. Everyone else should, also.

#CBC-RS/TC-watchlist

Comment 5 by minch@chromium.org, May 15 2018

HI Dan,
"If the "Sign out" item doesn't already contain the Ctrl+Shift+Q accelerator, we may want to add that at the same time."

For this, does that mean we want to add EXIT accelerator when user is logged in? Since currently we only enabled 
 BRIGHTNESS_DOWN, BRIGHTNESS_UP, VOLUME_DOWN, VOLUME_UP, VOLUME_MUTE,
accelerators when menu is shown.

And also add LOCK_SCREEN accelerator if screen is unlocked when menu is shown?

Comment 6 by minch@chromium.org, May 15 2018

Status: Started (was: Assigned)

Comment 7 by derat@chromium.org, May 15 2018

Oh, hmm. Yeah, good point. I was mostly just suggesting adding those to educate users about the accelerators, but yes, if we display them in the menu then they should probably work while the menu is being displayed. :-)

Re displaying the accelerators in the menu, you might want to make sure that Elizabeth or someone else in UX agrees first (since it'll change the appearance of the menu).

Comment 8 by warx@chromium.org, May 16 2018

For "Ctrl+Shift+Q" signing out accelerator, the code path here: https://cs.chromium.org/chromium/src/ash/accelerators/accelerator_controller.cc?type=cs&sq=package:chromium&g=0&l=1385, will trigger warning dialog: `Press Ctrl+Shift+Q twice to sign out`. We need UX opinions here as well. We probably don't need to display the dialog, instead focus on signout menu item?
Project Member

Comment 9 by bugdroid1@chromium.org, May 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0

commit b0de5924c3d91aa6be3ab9f0587c873abddeb6f0
Author: Min Chen <minch@google.com>
Date: Thu May 17 17:12:16 2018

Add "lock screen" item to power button menu.

- Add "Lock screen" item to power button menu when user is logged in and
  screen is unlocked.

- Update the text font size from 14px to 12px.

- Move the icon 1px down and text 1px up.

Bug:  841600 
Change-Id: I2bdefc2d571bcfa1fe121bc08d6694c6fb14412d
Reviewed-on: https://chromium-review.googlesource.com/1060528
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Qiang Xu <warx@google.com>
Cr-Commit-Position: refs/heads/master@{#559580}
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/ash_strings.grd
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/resources/vector_icons/system_power_button_menu_lock_screen.icon
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_controller.cc
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_controller_test_api.cc
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_controller_test_api.h
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_controller_unittest.cc
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_menu_item_view.cc
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_menu_metrics_type.h
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_menu_screen_view.cc
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_menu_screen_view.h
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_menu_view.cc
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/ash/system/power/power_button_menu_view.h
[modify] https://crrev.com/b0de5924c3d91aa6be3ab9f0587c873abddeb6f0/tools/metrics/histograms/enums.xml

Comment 10 by minch@chromium.org, May 22 2018

Status: Fixed (was: Started)
Split the accelerators related issue into another crbug https://bugs.chromium.org/p/chromium/issues/detail?id=845545

So I am going to close this issue which is targeted to m68.

Sign in to add a comment