Skip to content

Commit

Permalink
arc/gio: Clean up ActionLabel and improve readability
Browse files Browse the repository at this point in the history 
- The settings for ActionLabel are partially in ActionView, which
makes it inconvenient to change the ActionLabel, so move these settings
back to ActionLabel to improve the readability.
- Wrapped duplicated code into utils.
- Rename to improve readability.

Bug: b/260868602
Test: Manual test to check the behavior.
Test: No unittest is failed.
Change-Id: Idbb992002cda6113b4ef33bdbdd2212240a3c9c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4193144
Reviewed-by: David Jacobo <djacobo@chromium.org>
Auto-Submit: Cici Ruan <cuicuiruan@google.com>
Commit-Queue: Cici Ruan <cuicuiruan@google.com>
Cr-Commit-Position: refs/heads/main@{#1099066}
  • Loading branch information
Cici Ruan authored and Chromium LUCI CQ committed Jan 31, 2023
1 parent a82892f commit d729908
Show file tree
Hide file tree
Showing 10 changed files with 295 additions and 170 deletions.
70 changes: 18 additions & 52 deletions chrome/browser/ash/arc/input_overlay/actions/action_move.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "chrome/browser/ash/arc/input_overlay/actions/action.h"
#include "chrome/browser/ash/arc/input_overlay/touch_id_manager.h"
#include "chrome/browser/ash/arc/input_overlay/ui/action_label.h"
#include "chrome/browser/ash/arc/input_overlay/util.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/events/keycodes/dom/dom_code.h"
#include "ui/events/keycodes/dom/keycode_converter.h"
Expand All @@ -25,17 +26,6 @@ constexpr char kKeys[] = "keys";
constexpr char kTargetArea[] = "target_area";
constexpr char kTopLeft[] = "top_left";
constexpr char kBottomRight[] = "bottom_right";
// TODO(cuicuiruan): remove this and replace it with image asset.
constexpr char kMouseCursorLock[] = "mouse cursor lock (esc)";

constexpr int kAxisSize = 2;
constexpr int kDirection[kActionMoveKeysSize][kAxisSize] = {{0, -1},
{-1, 0},
{0, 1},
{1, 0}};
// UI specs.
// Offset by label center.
constexpr int kLabelOffset = 49;

std::unique_ptr<Position> ParseApplyAreaPosition(const base::Value& value,
base::StringPiece key) {
Expand Down Expand Up @@ -66,13 +56,18 @@ class ActionMove::ActionMoveMouseView : public ActionView {
ActionMoveMouseView& operator=(const ActionMoveMouseView&) = delete;
~ActionMoveMouseView() override = default;

// TODO(cuicuiruan): rewrite for post MVP once design is ready.
// TODO(b/241966781): rewrite for Beta once design is ready.
void SetViewContent(BindingOption binding_option) override {
auto label = ActionLabel::CreateTextActionLabel(kMouseCursorLock);
labels_.emplace_back(AddChildView(std::move(label)));
InputElement* input_binding =
GetInputBindingByBindingOption(action_, binding_option);
if (!input_binding)
return;

int radius = std::max(kActionMoveMinRadius, action_->GetUIRadius());
labels_ = ActionLabel::Show(this, ActionType::MOVE, *input_binding, radius);
}

// TODO(cuicuiruan): rewrite for post MVP once design is ready.
// TODO(b/241966781): rewrite for Beta once design is ready.
void OnKeyBindingChange(ActionLabel* action_label,
ui::DomCode code) override {
NOTIMPLEMENTED();
Expand All @@ -83,16 +78,12 @@ class ActionMove::ActionMoveMouseView : public ActionView {
void AddTouchPoint() override { NOTIMPLEMENTED(); }

void ChildPreferredSizeChanged(View* child) override {
DCHECK_EQ(labels_.size(), 1u);
if (static_cast<ActionLabel*>(child) != labels_[0])
return;

auto label_size = labels_[0]->CalculatePreferredSize();
labels_[0]->SetSize(label_size);
labels_[0]->SetPosition(gfx::Point());
center_.set_x(label_size.width() / 2);
center_.set_y(label_size.height() / 2);
UpdateTrashButtonPosition();
SetSize(label_size);
SetSize(labels_[0]->size());
SetPositionFromCenterPosition(action_->GetUICenterPosition());
}
};
Expand All @@ -113,34 +104,18 @@ class ActionMove::ActionMoveKeyView : public ActionView {
int radius = std::max(kActionMoveMinRadius, action_->GetUIRadius());
auto* action_move = static_cast<ActionMove*>(action_);
action_move->set_move_distance(radius / 2);
center_.set_x(radius);
center_.set_y(radius);
set_touch_point_center(gfx::Point(radius, radius));
UpdateTrashButtonPosition();

InputElement* input_binding = nullptr;
switch (binding_option) {
case BindingOption::kCurrent:
input_binding = action_->current_input();
break;
case BindingOption::kOriginal:
input_binding = action_->original_input();
break;

case BindingOption::kPending:
input_binding = action_->pending_input();
break;
default:
NOTREACHED();
}
InputElement* input_binding =
GetInputBindingByBindingOption(action_, binding_option);
if (!input_binding)
return;

const auto& keys = input_binding->keys();
if (labels_.empty()) {
for (const auto& key : keys) {
auto label = ActionLabel::CreateTextActionLabel(GetDisplayText(key));
labels_.emplace_back(AddChildView(std::move(label)));
}
labels_ =
ActionLabel::Show(this, ActionType::MOVE, *input_binding, radius);
} else {
DCHECK(labels_.size() == keys.size());
for (size_t i = 0; i < keys.size(); i++)
Expand Down Expand Up @@ -205,16 +180,6 @@ class ActionMove::ActionMoveKeyView : public ActionView {
if (label_index == -1)
return;

const int radius = std::max(kActionMoveMinRadius, action_->GetUIRadius());
auto label_size = child_label->CalculatePreferredSize();
child_label->SetSize(label_size);
int x = kDirection[label_index][0];
int y = kDirection[label_index][1];
auto pos = gfx::Point(
radius + x * (radius - kLabelOffset) - label_size.width() / 2,
radius + y * (radius - kLabelOffset) - label_size.height() / 2);
child_label->SetPosition(pos);

// Calculate minimum size of the |ActionMoveKeyView|.
int left = INT_MAX, right = 0, top = INT_MAX, bottom = 0;
for (const auto* label : labels_) {
Expand All @@ -226,6 +191,7 @@ class ActionMove::ActionMoveKeyView : public ActionView {
DCHECK_LT(left, right);
DCHECK_LT(top, bottom);

const int radius = std::max(kActionMoveMinRadius, action_->GetUIRadius());
auto size = gfx::Size(radius * 2, radius * 2);
size.SetToMax(gfx::Size(right - left, bottom - top));
SetSize(size);
Expand Down
70 changes: 17 additions & 53 deletions chrome/browser/ash/arc/input_overlay/actions/action_tap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "chrome/browser/ash/arc/input_overlay/constants.h"
#include "chrome/browser/ash/arc/input_overlay/touch_id_manager.h"
#include "chrome/browser/ash/arc/input_overlay/ui/action_label.h"
#include "chrome/browser/ash/arc/input_overlay/util.h"
#include "ui/aura/window.h"
#include "ui/events/base_event_utils.h"
#include "ui/events/keycodes/dom/dom_code.h"
Expand All @@ -18,23 +19,15 @@

namespace arc::input_overlay {
namespace {
// UI specs.
constexpr int kLabelPositionToSide = 36;
constexpr int kLabelMargin = 2;

// Create |ActionLabel| for |ActionTap|.
std::unique_ptr<ActionLabel> CreateActionLabel(InputElement& input_element) {
std::unique_ptr<ActionLabel> label;
if (IsKeyboardBound(input_element)) {
DCHECK_EQ(1u, input_element.keys().size());
label = ActionLabel::CreateTextActionLabel(
GetDisplayText(input_element.keys()[0]));
} else if (IsMouseBound(input_element)) {
label = ActionLabel::CreateImageActionLabel(input_element.mouse_action());
} else {
label = ActionLabel::CreateTextActionLabel(kUnknownBind);

gfx::Size GetBoundingBoxOfChildren(views::View* view) {
int x = 0;
int y = 0;
for (auto* child : view->children()) {
x = std::max(x, child->bounds().right());
y = std::max(y, child->bounds().bottom());
}
return label;
return gfx::Size(x, y);
}

} // namespace
Expand All @@ -52,27 +45,17 @@ class ActionTap::ActionTapView : public ActionView {
~ActionTapView() override = default;

void SetViewContent(BindingOption binding_option) override {
InputElement* input_binding = nullptr;
switch (binding_option) {
case BindingOption::kCurrent:
input_binding = action_->current_input();
break;
case BindingOption::kOriginal:
input_binding = action_->original_input();
break;
case BindingOption::kPending:
input_binding = action_->pending_input();
break;
default:
NOTREACHED();
}
InputElement* input_binding =
GetInputBindingByBindingOption(action_, binding_option);
if (!input_binding)
return;

if (labels_.empty()) {
// Create new action label when initializing.
auto label = CreateActionLabel(*input_binding);
labels_.emplace_back(AddChildView(std::move(label)));
labels_ = ActionLabel::Show(
this, ActionType::TAP, *input_binding, action_->GetUIRadius(),
action_->on_left_or_middle_side() ? TapLabelPosition::kBottomRight
: TapLabelPosition::kBottomLeft);
} else if (!IsInputBound(*input_binding)) {
// Action label exists but without any bindings.
labels_[0]->SetTextActionLabel(
Expand Down Expand Up @@ -138,28 +121,9 @@ class ActionTap::ActionTapView : public ActionView {
DCHECK_EQ(1u, labels_.size());
if (static_cast<ActionLabel*>(child) != labels_[0])
return;

int radius = action_->GetUIRadius();
auto* label = labels_[0];
auto label_size = label->CalculatePreferredSize();
int width = std::max(
radius * 2, radius * 2 - kLabelPositionToSide + label_size.width());
if (action_->on_left_or_middle_side()) {
label->SetPosition(
gfx::Point(label_size.width() > kLabelPositionToSide
? width - label_size.width()
: width - kLabelPositionToSide,
radius * 2 - label_size.height() - kLabelMargin));
center_.set_x(radius);
center_.set_y(radius);
} else {
label->SetPosition(
gfx::Point(0, radius * 2 - label_size.height() - kLabelMargin));
center_.set_x(width - radius);
center_.set_y(radius);
}
UpdateTrashButtonPosition();
label->SetSize(label_size);
int radius = action_->GetUIRadius();
int width = std::max(radius * 2, GetBoundingBoxOfChildren(this).width());
SetSize(gfx::Size(width, radius * 2));
SetPositionFromCenterPosition(action_->GetUICenterPosition());
}
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/ash/arc/input_overlay/actions/input_element.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ constexpr char kHoverMove[] = "hover_move";
constexpr char kPrimaryDragMove[] = "primary_drag_move";
constexpr char kSecondaryDragMove[] = "secondary_drag_move";

// Total key size for ActionMoveKey.
constexpr size_t kActionMoveKeysSize = 4;
// Gets the event flags for the modifier domcode. Return ui::DomCode::NONE if
// |code| is not modifier DomCode.
int ModifierDomCodeToEventFlag(ui::DomCode code);
Expand Down
26 changes: 26 additions & 0 deletions chrome/browser/ash/arc/input_overlay/constants.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,22 @@
#ifndef CHROME_BROWSER_ASH_ARC_INPUT_OVERLAY_CONSTANTS_H_
#define CHROME_BROWSER_ASH_ARC_INPUT_OVERLAY_CONSTANTS_H_

#include <stddef.h>

namespace arc::input_overlay {

// The coordinates number, including Axis x and y.
constexpr int kAxisSize = 2;

// Total key size for ActionMoveKey.
constexpr size_t kActionMoveKeysSize = 4;

// Directions from up, left, down, right.
constexpr int kDirection[kActionMoveKeysSize][kAxisSize] = {{0, -1},
{-1, 0},
{0, 1},
{1, 0}};

// Display mode for display overlay.
enum class DisplayMode {
kNone,
Expand Down Expand Up @@ -67,6 +81,18 @@ enum class PositionType {
kDependent = 1,
};

// The label position related to touch center for ActionTap.
enum class TapLabelPosition {
// Top-left of touch point. Starts to use in AlphaV2.
kTopLeft = 0,
// Top-right of touch point. Starts to use in AlphaV2.
kTopRight = 1,
// Bottom-left of touch point. Starts to use in Alpha.
kBottomLeft = 2,
// Bottom-right of touch point. Starts to use in Alpha.
kBottomRight = 3,
};

} // namespace arc::input_overlay

#endif // CHROME_BROWSER_ASH_ARC_INPUT_OVERLAY_CONSTANTS_H_

0 comments on commit d729908

Please sign in to comment.