Skip to content

Commit

Permalink
Fix login shelf buttons highlight.
Browse files Browse the repository at this point in the history 
Bug: b:326131119
Change-Id: Id0a62acc44ed3efb5be3aedbfc74fca37ac57235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5385864
Reviewed-by: Elie Maamari <emaamari@google.com>
Commit-Queue: Istvan Nagy <iscsi@google.com>
Cr-Commit-Position: refs/heads/main@{#1277564}
  • Loading branch information
Istvan Nagy authored and Chromium LUCI CQ committed Mar 25, 2024
1 parent 1ed605f commit e5b4774
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 60 deletions.
18 changes: 16 additions & 2 deletions ash/login/login_screen_test_api.cc
View file
Expand Up @@ -85,6 +85,15 @@ views::View* GetShutDownButton() {
return shelf_view->GetViewByID(LoginShelfView::kShutdown);
}

views::View* GetShutDownButtonContainer() {
LoginShelfView* shelf_view = GetLoginShelfView();
if (!shelf_view) {
return nullptr;
}

return shelf_view->GetButtonContainerByID(LoginShelfView::kShutdown);
}

views::View* GetAppsButton() {
LoginShelfView* shelf_view = GetLoginShelfView();
if (!shelf_view) {
Expand Down Expand Up @@ -854,12 +863,17 @@ gfx::Rect LoginScreenTestApi::GetShutDownButtonTargetBounds() {

// static
gfx::Rect LoginScreenTestApi::GetShutDownButtonMirroredBounds() {
views::View* button_container = GetShutDownButtonContainer();
views::View* button = GetShutDownButton();
if (!button) {
return gfx::Rect();
}

return button->GetMirroredBounds();
gfx::Point button_container_origin =
button_container->GetMirroredBounds().origin();
gfx::Rect button_mirrored_bounds = button->GetMirroredBounds();
button_mirrored_bounds.set_origin(button_container_origin +
button_mirrored_bounds.OffsetFromOrigin());
return button_mirrored_bounds;
}

// static
Expand Down
9 changes: 5 additions & 4 deletions ash/shelf/login_shelf_button.cc
View file
Expand Up @@ -33,6 +33,7 @@ namespace {
// The highlight radius of the button.
// The large pill buttons height is 36 and the radius should be half of that.
constexpr int kButtonHighlightRadiusDp = 18;
constexpr int kButtonHighlightWidthDp = 1;

} // namespace

Expand All @@ -51,9 +52,9 @@ LoginShelfButton::LoginShelfButton(PressedCallback callback,
SetFocusPainter(nullptr);

views::InkDrop::Get(this)->SetMode(views::InkDropHost::InkDropMode::OFF);
SetBorder(std::make_unique<views::HighlightBorder>(
kButtonHighlightRadiusDp,
views::HighlightBorder::Type::kHighlightBorderNoShadow));
SetBorder(views::CreateThemedRoundedRectBorder(
kButtonHighlightWidthDp, kButtonHighlightRadiusDp,
ui::kColorCrosSystemHighlight));
}

LoginShelfButton::~LoginShelfButton() = default;
Expand All @@ -64,7 +65,7 @@ int LoginShelfButton::text_resource_id() const {

std::u16string LoginShelfButton::GetTooltipText(const gfx::Point& p) const {
if (label()->IsDisplayTextTruncated()) {
return label()->GetText();
return GetText();
}
return std::u16string();
}
Expand Down
118 changes: 73 additions & 45 deletions ash/shelf/login_shelf_view.cc
View file
Expand Up @@ -62,19 +62,29 @@
#include "ui/views/animation/ink_drop_mask.h"
#include "ui/views/controls/highlight_path_generator.h"
#include "ui/views/focus/focus_search.h"
#include "ui/views/highlight_border.h"
#include "ui/views/layout/box_layout.h"
#include "ui/views/layout/fill_layout.h"
#include "ui/views/view_class_properties.h"
#include "ui/views/widget/widget.h"

using session_manager::SessionState;

namespace ash {
namespace {
const char* kLoginShelfButtonClassName = "LoginShelfButton";

// Skip only that many to avoid blocking users in case of any subtle bugs.
const int kMaxDroppedCallsWhenDisplaysOff = 3;

// The LoginShelfButton's outer border radius.
const int kButtonHighlightBorderRadius = 19;

// The LoginShelfButton's outer border width.
const int kButtonHighlightBorderWidth = 1;

// To can retrieve the button and the container ID, we need to shift
// the container IDs.
const int kButtonContainerDiff = 100;

constexpr LoginShelfView::ButtonId kButtonIds[] = {
LoginShelfView::kShutdown,
LoginShelfView::kRestart,
Expand Down Expand Up @@ -242,15 +252,27 @@ LoginShelfView::LoginShelfView(
gfx::Insets::TLBR(0, ShelfConfig::Get()->button_spacing(), 0, 0));
SetLayoutManager(std::move(box_layout));

auto add_button = [this](ButtonId id, base::RepeatingClosure callback,
int text_resource_id, const gfx::VectorIcon& icon) {
LoginShelfButton* button = new LoginShelfButton(
auto add_button_common = [this](std::unique_ptr<LoginShelfButton> button,
ButtonId id) -> LoginShelfButton* {
View* button_container = AddChildView(std::make_unique<View>());
button_container->SetLayoutManager(std::make_unique<views::FillLayout>());
button_container->SetBorder(views::CreateThemedRoundedRectBorder(
kButtonHighlightBorderWidth, kButtonHighlightBorderRadius,
ui::kColorCrosSystemHighlightBorder));
button_container->SetID(kButtonContainerDiff + id);
button->SetID(id);
return button_container->AddChildView(std::move(button));
};

auto add_button = [this, &add_button_common](
ButtonId id, base::RepeatingClosure callback,
int text_resource_id, const gfx::VectorIcon& icon) {
auto button = std::make_unique<LoginShelfButton>(
base::BindRepeating(&ButtonPressed, id, std::move(callback)),
text_resource_id, icon);
button->SetID(id);
AddChildView(button);
login_shelf_buttons_.push_back(button);
login_shelf_buttons_.push_back(add_button_common(std::move(button), id));
};

const auto shutdown_callback = base::BindRepeating(
&LoginShelfView::RequestShutdown, weak_ptr_factory_.GetWeakPtr());
add_button(
Expand All @@ -275,9 +297,12 @@ LoginShelfView::LoginShelfView(
Shell::Get()->session_controller()->RequestSignOut();
})),
IDS_ASH_SHELF_SIGN_OUT_BUTTON, kShelfSignOutButtonIcon);
kiosk_apps_button_ = new KioskAppsButton();
kiosk_apps_button_->SetID(kApps);
AddChildView(kiosk_apps_button_.get());

kiosk_apps_button_ = static_cast<KioskAppsButton*>(
add_button_common(std::make_unique<KioskAppsButton>(), kApps));
login_shelf_buttons_.push_back(
static_cast<LoginShelfButton*>(kiosk_apps_button_));

add_button(kCloseNote,
base::BindRepeating(
&TrayAction::CloseLockScreenNote,
Expand Down Expand Up @@ -596,12 +621,9 @@ void LoginShelfView::OnDeviceEnterpriseInfoChanged() {
void LoginShelfView::OnEnterpriseAccountDomainChanged() {}

void LoginShelfView::HandleLocaleChange() {
for (views::View* child : children()) {
if (!std::strcmp(child->GetClassName(), kLoginShelfButtonClassName)) {
auto* button = static_cast<LoginShelfButton*>(child);
button->SetText(l10n_util::GetStringUTF16(button->text_resource_id()));
button->SetAccessibleName(button->GetText());
}
for (LoginShelfButton* button : login_shelf_buttons_) {
button->SetText(l10n_util::GetStringUTF16(button->text_resource_id()));
button->SetAccessibleName(button->GetText());
}
}

Expand All @@ -622,6 +644,11 @@ bool LoginShelfView::LockScreenActionBackgroundAnimating() const {
LockScreenActionBackgroundState::kHiding;
}

void LoginShelfView::SetButtonVisible(ButtonId button_id, bool visible) {
GetButtonContainerByID(button_id)->SetVisible(visible);
GetViewByID(button_id)->SetVisible(visible);
}

void LoginShelfView::UpdateUi() {
// Make sure observers are notified.
base::ScopedClosureRunner fire_observer(base::BindOnce(
Expand Down Expand Up @@ -655,38 +682,35 @@ void LoginShelfView::UpdateUi() {
tray_action_state == mojom::TrayActionState::kLaunching) &&
!LockScreenActionBackgroundAnimating();

GetViewByID(kShutdown)->SetVisible(!show_reboot &&
!is_lock_screen_note_in_foreground &&
ShouldShowShutdownButton());
GetViewByID(kRestart)->SetVisible(show_reboot &&
!is_lock_screen_note_in_foreground &&
ShouldShowShutdownButton());
GetViewByID(kSignOut)->SetVisible(is_locked &&
!is_lock_screen_note_in_foreground);
GetViewByID(kCloseNote)
->SetVisible(is_locked && is_lock_screen_note_in_foreground);
GetViewByID(kCancel)->SetVisible(session_state ==
SessionState::LOGIN_SECONDARY);
GetViewByID(kParentAccess)->SetVisible(is_locked && show_parent_access_);

GetViewByID(kBrowseAsGuest)->SetVisible(ShouldShowGuestButton());
GetViewByID(kEnterpriseEnrollment)
->SetVisible(ShouldShowEnterpriseEnrollmentButton());

GetViewByID(kSchoolEnrollment)
->SetVisible(ShouldShowSchoolEnrollmentButton());

GetViewByID(kSignIn)->SetVisible(ShouldShowSignInButton());

GetViewByID(kAddUser)->SetVisible(ShouldShowAddUserButton());
kiosk_apps_button_->SetVisible(kiosk_apps_button_->HasApps() &&
ShouldShowAppsButton());
SetButtonVisible(kShutdown, !show_reboot &&
!is_lock_screen_note_in_foreground &&
ShouldShowShutdownButton());

SetButtonVisible(kRestart, show_reboot &&
!is_lock_screen_note_in_foreground &&
ShouldShowShutdownButton());
SetButtonVisible(kSignOut, is_locked && !is_lock_screen_note_in_foreground);
SetButtonVisible(kCloseNote, is_locked && is_lock_screen_note_in_foreground);
SetButtonVisible(kCancel, session_state == SessionState::LOGIN_SECONDARY);
SetButtonVisible(kParentAccess, is_locked && show_parent_access_);

SetButtonVisible(kBrowseAsGuest, ShouldShowGuestButton());
SetButtonVisible(kEnterpriseEnrollment,
ShouldShowEnterpriseEnrollmentButton());

SetButtonVisible(kSchoolEnrollment, ShouldShowSchoolEnrollmentButton());

SetButtonVisible(kSignIn, ShouldShowSignInButton());

SetButtonVisible(kAddUser, ShouldShowAddUserButton());
SetButtonVisible(kApps,
kiosk_apps_button_->HasApps() && ShouldShowAppsButton());
if (kiosk_license_mode_) {
// Create the bubble once the login shelf view is available for anchoring.
if (!kiosk_instruction_bubble_) {
Shelf* shelf = Shelf::ForWindow(GetWidget()->GetNativeWindow());
kiosk_instruction_bubble_ =
new KioskAppInstructionBubble(GetViewByID(kApps), shelf->alignment());
new KioskAppInstructionBubble(kiosk_apps_button_, shelf->alignment());
}
if (kiosk_instruction_bubble_) {
// Show kiosk instructions if the kiosk app button is visible and the menu
Expand All @@ -700,7 +724,7 @@ void LoginShelfView::UpdateUi() {
}
}

GetViewByID(kOsInstall)->SetVisible(ShouldShowOsInstallButton());
SetButtonVisible(kOsInstall, ShouldShowOsInstallButton());

// If there is no visible (and thus focusable) buttons, we shouldn't focus
// LoginShelfView. We update it here, so we don't need to check visibility
Expand Down Expand Up @@ -896,6 +920,10 @@ bool LoginShelfView::ShouldShowOsInstallButton() const {
return true;
}

views::View* LoginShelfView::GetButtonContainerByID(ButtonId button_id) {
return GetViewByID(button_id + kButtonContainerDiff);
}

BEGIN_METADATA(LoginShelfView)
END_METADATA

Expand Down
5 changes: 5 additions & 0 deletions ash/shelf/login_shelf_view.h
View file
Expand Up @@ -162,6 +162,9 @@ class ASH_EXPORT LoginShelfView : public views::View,
// Returns scoped object to temporarily block Browse as Guest login button.
std::unique_ptr<ScopedGuestButtonBlocker> GetScopedGuestButtonBlocker();

// Returns the button container.
views::View* GetButtonContainerByID(ButtonId button_id);

// TrayActionObserver:
void OnLockScreenNoteStateChanged(mojom::TrayActionState state) override;

Expand Down Expand Up @@ -237,6 +240,8 @@ class ASH_EXPORT LoginShelfView : public views::View,

bool ShouldShowOsInstallButton() const;

void SetButtonVisible(ButtonId id, bool visible);

// Helper function which calls `closure` when device display is on. Or if the
// number of dropped calls exceeds 'kMaxDroppedCallsWhenDisplaysOff'
void CallIfDisplayIsOn(const base::RepeatingClosure& closure);
Expand Down
14 changes: 7 additions & 7 deletions ash/shelf/login_shelf_view_pixeltest.cc
View file
Expand Up @@ -77,28 +77,28 @@ TEST_F(LoginShelfViewPixelTest, FocusTraversalFromLockContents) {
PressAndReleaseKey(ui::VKEY_TAB);
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"focus_on_login_user_expand_button",
/*revision_number=*/10, primary_big_user_view_.get(),
/*revision_number=*/12, primary_big_user_view_.get(),
primary_shelf_window));

// Trigger the tab key. Check that the login shelf shutdown button is focused.
PressAndReleaseKey(ui::VKEY_TAB);
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"focus_on_shutdown_button",
/*revision_number=*/10, primary_big_user_view_.get(),
/*revision_number=*/12, primary_big_user_view_.get(),
primary_shelf_window));

// Trigger the tab key. Check that the browser as guest button is focused.
PressAndReleaseKey(ui::VKEY_TAB);
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"focus_on_browser_as_guest_button",
/*revision_number=*/10, primary_big_user_view_.get(),
/*revision_number=*/12, primary_big_user_view_.get(),
primary_shelf_window));

// Trigger the tab key. Check that the add person button is focused.
PressAndReleaseKey(ui::VKEY_TAB);
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"focus_on_add_person_button",
/*revision_number=*/10, primary_big_user_view_.get(),
/*revision_number=*/12, primary_big_user_view_.get(),
primary_shelf_window));
}

Expand All @@ -113,21 +113,21 @@ TEST_F(LoginShelfViewPixelTest, FocusTraversalWithinShelf) {
aura::Window* primary_shelf_window = GetPrimaryShelf()->GetWindow();
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"focus_on_calendar_view",
/*revision_number=*/6, primary_shelf_window));
/*revision_number=*/8, primary_shelf_window));

// Focus on the time view.
PressAndReleaseKey(ui::VKEY_TAB);
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"focus_on_time_view.rev_0",
/*revision_number=*/6, primary_shelf_window));
/*revision_number=*/8, primary_shelf_window));

PressAndReleaseKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
PressAndReleaseKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);

// Move the focus back to the add person button.
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"refocus_on_login_shelf",
/*revision_number=*/6, primary_shelf_window));
/*revision_number=*/8, primary_shelf_window));
}

class LoginShelfWithPolicyWallpaperPixelTestWithRTL
Expand Down
10 changes: 8 additions & 2 deletions ash/shelf/login_shelf_view_unittest.cc
View file
Expand Up @@ -154,8 +154,14 @@ class LoginShelfViewTest : public LoginTestBase {
auto* latter_button_view = login_shelf_view_->GetViewByID(latter);
EXPECT_TRUE(former_button_view->GetVisible() &&
latter_button_view->GetVisible());
return login_shelf_view_->GetIndexOf(former_button_view) <
login_shelf_view_->GetIndexOf(latter_button_view);
auto* former_button_view_container =
login_shelf_view_->GetButtonContainerByID(former);
auto* latter_button_view_container =
login_shelf_view_->GetButtonContainerByID(latter);
EXPECT_TRUE(former_button_view_container->GetVisible() &&
latter_button_view_container->GetVisible());
return login_shelf_view_->GetIndexOf(former_button_view_container) <
login_shelf_view_->GetIndexOf(latter_button_view_container);
}

// Check whether the button is enabled.
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ash/login/login_screen_policy_browsertest.cc
View file
Expand Up @@ -233,6 +233,7 @@ IN_PROC_BROWSER_TEST_F(LoginScreenButtonsLocalePolicy,
std::u16string expected_text =
l10n_util::GetStringUTF16(IDS_ASH_SHELF_SHUTDOWN_BUTTON);

// Login shelf button's should be updated to the current locale.
EXPECT_EQ(expected_text, actual_text);

// Check if the shelf buttons are correctly aligned for RTL locale.
Expand Down

0 comments on commit e5b4774

Please sign in to comment.