Ensure correct theming and lack of ripple animations for non-clickable rows in the login/lock screen |
||||
Issue descriptionCertain default rows in the Ash system menu are not clickable in the login/lock screens. These rows should have the correct 'disabled' theming (color of icon and text) and should not show a ripple when clicked/tapped. As a first step, an audit of all impacted rows will be required.
,
Feb 10 2017
I'm a bit confused. Why are we trying to disable rows that are already disabled ?
,
Feb 10 2017
To clarify, certain rows/actions are already disabled. However, some are not disabled but we just prevent the user from accessing the detailed view. Right now the arrow is removed, but the question is whether we should also gray out the row to indicate that clicking won't do anything. See the screenshot, eg. Network or Cast rows.
,
Feb 10 2017
Re c#2: Consider network row: 1) If the user is logged in and connected to a network, the row is not grayed out and the user can click on it to switch to the detailed network view. 2) If the user is logged in, but does not have any network connection, the row is grayed out, however the user still can click on it to switch to the detailed view and possibly select a network to connect to. 3) If the device is locked, regardless of network connectivity, the user cannot click on the network row and switch to the detailed view. The row might be grayed out or not depending on the connectivity status. This bug is about the third case. The row is not clickable, however a ripple is shown if user taps on it.
,
Feb 10 2017
ah got it thanks. ok so there's 2 "greyed" out version. 0.54a: Row feature is off but the row is clickable 0.36a: Row is disabled regardless of state (this is applied to settings/help/lock on lock screen). So in the current example, Network, connected or not should be marked disabled, i.e 0.36a.
,
Feb 10 2017
this applies to bluetooth as well.
,
Feb 10 2017
Great. Thanks
,
Feb 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/39de08056d4d8cb6a4f2928b4c142f907c37e6b7 commit 39de08056d4d8cb6a4f2928b4c142f907c37e6b7 Author: mohsen <mohsen@chromium.org> Date: Thu Feb 16 23:47:54 2017 Make RotationLockDefaultView inherit directly from ActionableView RotationLockDefaultView was inheriting from TrayItemMore to be able to use its layout. However, it did not use TrayItemMore's chevron or detailed view transition. This patch changes RotationLockDefaultView to inherit directly from ActionableView and use factory functions in TrayPopupUtils to layout its contents. This patch also fixes an issue with rotation lock row where its label was not updated properly when rotation lock status was changed (e.g. due to use of rotation shortcut). This patch is a prerequisite for fixing correct theming and ripple animation for non-clickable rows in the lock screen. BUG= 687689 TEST=TrayRotationLockTest.* in ash_unittests, manual Review-Url: https://codereview.chromium.org/2696703005 Cr-Commit-Position: refs/heads/master@{#451139} [modify] https://crrev.com/39de08056d4d8cb6a4f2928b4c142f907c37e6b7/ash/system/chromeos/rotation/tray_rotation_lock.cc [modify] https://crrev.com/39de08056d4d8cb6a4f2928b4c142f907c37e6b7/ash/system/chromeos/rotation/tray_rotation_lock.h
,
Feb 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/469c3b662e9bb4bfc28853a166420458079cf19d commit 469c3b662e9bb4bfc28853a166420458079cf19d Author: mohsen <mohsen@chromium.org> Date: Wed Feb 22 08:04:57 2017 Modify TrayItemMore to use enabled status With this patch, if a TrayItemMore object is set to be disabled, it will not show the more arrow and ripple animations. Therefore, |show_more_| field is not needed anymore and is removed. Also, non-MD code path is removed from TrayItemMore. BUG= 687689 , 687817 Review-Url: https://codereview.chromium.org/2699443004 Cr-Commit-Position: refs/heads/master@{#451917} [modify] https://crrev.com/469c3b662e9bb4bfc28853a166420458079cf19d/ash/common/system/chromeos/bluetooth/tray_bluetooth.cc [modify] https://crrev.com/469c3b662e9bb4bfc28853a166420458079cf19d/ash/common/system/chromeos/cast/tray_cast.cc [modify] https://crrev.com/469c3b662e9bb4bfc28853a166420458079cf19d/ash/common/system/chromeos/network/tray_network.cc [modify] https://crrev.com/469c3b662e9bb4bfc28853a166420458079cf19d/ash/common/system/chromeos/network/tray_vpn.cc [modify] https://crrev.com/469c3b662e9bb4bfc28853a166420458079cf19d/ash/common/system/ime/tray_ime_chromeos.cc [modify] https://crrev.com/469c3b662e9bb4bfc28853a166420458079cf19d/ash/common/system/tray/tray_item_more.cc [modify] https://crrev.com/469c3b662e9bb4bfc28853a166420458079cf19d/ash/common/system/tray/tray_item_more.h [modify] https://crrev.com/469c3b662e9bb4bfc28853a166420458079cf19d/ash/common/system/tray_accessibility.cc
,
Mar 1 2017
@mohsen is this done?
,
Mar 1 2017
Re #10: Almost done. The only remaining part is that some icons (e.g. network icon) are not themed as disabled, yet. However, the ripples should work (i.e. not work in lock screen) and texts and some icons should be themed correctly.
,
Mar 7 2017
I am going to mark this as done for M-58. Mohsen I would treat what you describe in #11 as a separate issue; can you please file a separate bug for that?
,
Mar 7 2017
Re #12: Issue 699262 filed for the remaining work in icons.
,
May 10 2017
9534.0.0, 60.0.3092.0 |
||||
►
Sign in to add a comment |
||||
Comment 1 by moh...@chromium.org
, Feb 10 2017