Commit 9ad236a9 authored by Leonard Grey's avatar Leonard Grey Committed by Commit Bot

Commander: add annotation and entity type to view model

These are to support displaying hotkeys and tagging different types of
result with specific icons.

Bug: 1014639
Change-Id: I93f432a2368c25285dd714c4f2fbdaef96434a33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2403661Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805887}
parent 40501609
......@@ -24,6 +24,18 @@ struct CommandItem {
// On selection, the user is prompted for further information.
kComposite,
};
// What *the text* of this command represents. For example, in the composite
// command "Move Current Tab To Window", the user will be prompted to select
// a window by name. In that case, the original command will have Entity =
// kCommand, and the follow-up will have Entity kWindow.
// This is used in the UI to give different visual treatments to different
// entity types.
enum Entity {
kCommand,
kBookmark,
kTab,
kWindow,
};
CommandItem();
virtual ~CommandItem();
CommandItem(const CommandItem& other) = delete;
......@@ -34,6 +46,11 @@ struct CommandItem {
Type GetType();
// The title to display to the user.
base::string16 title;
// See Entity documentation above.
Entity entity_type = kCommand;
// Optional secondary text for the command. Typically used to display a
// hotkey.
base::string16 annotation;
// The code to execute if the user selects this option, if it's a one-shot.
// Must be unset if |delegate_factory| is set.
base::Optional<base::OnceClosure> command;
......
......@@ -56,7 +56,7 @@ void CommanderController::OnTextChanged(const base::string16& text,
vm.result_set_id = ++current_result_set_id_;
vm.action = CommanderViewModel::Action::kDisplayResults;
for (auto& item : current_items_) {
vm.items.emplace_back(item->title, item->matched_ranges);
vm.items.emplace_back(*item);
}
callback_.Run(vm);
}
......
......@@ -213,8 +213,10 @@ TEST_F(CommanderControllerTest, ViewModelAggregatesResults) {
auto second = std::make_unique<TestCommandSource>(
base::BindRepeating([](const base::string16&, Browser* browser) {
CommandSource::CommandResults result;
result.push_back(
CreateNoOpCommandItem(base::ASCIIToUTF16("second"), 99));
auto item = CreateNoOpCommandItem(base::ASCIIToUTF16("second"), 99);
item->annotation = base::ASCIIToUTF16("2nd");
item->entity_type = CommandItem::Entity::kBookmark;
result.push_back(std::move(item));
return result;
}));
sources.push_back(std::move(first));
......@@ -232,8 +234,14 @@ TEST_F(CommanderControllerTest, ViewModelAggregatesResults) {
ASSERT_EQ(received_view_models_.size(), 1u);
CommanderViewModel model = received_view_models_.back();
ASSERT_EQ(model.items.size(), 2u);
EXPECT_EQ(model.items[0].title, base::ASCIIToUTF16("first"));
EXPECT_EQ(model.items[0].annotation, base::string16());
EXPECT_EQ(model.items[0].entity_type, CommandItem::Entity::kCommand);
EXPECT_EQ(model.items[1].title, base::ASCIIToUTF16("second"));
EXPECT_EQ(model.items[1].annotation, base::ASCIIToUTF16("2nd"));
EXPECT_EQ(model.items[1].entity_type, CommandItem::Entity::kBookmark);
}
// TODO(lgrey): This will need to change when scoring gets more sophisticated
......
......@@ -7,9 +7,20 @@
namespace commander {
CommandItemViewModel::CommandItemViewModel(
base::string16& title,
std::vector<gfx::Range>& matched_ranges)
: title(title), matched_ranges(matched_ranges) {}
const base::string16& title,
const std::vector<gfx::Range>& matched_ranges,
CommandItem::Entity entity_type,
const base::string16& annotation)
: title(title),
matched_ranges(matched_ranges),
entity_type(entity_type),
annotation(annotation) {}
CommandItemViewModel::CommandItemViewModel(const CommandItem& item)
: CommandItemViewModel(item.title,
item.matched_ranges,
item.entity_type,
item.annotation) {}
CommandItemViewModel::~CommandItemViewModel() = default;
CommandItemViewModel::CommandItemViewModel(const CommandItemViewModel& other) =
default;
......
......@@ -8,6 +8,7 @@
#include <vector>
#include "base/strings/string16.h"
#include "chrome/browser/ui/commander/command_source.h"
#include "ui/gfx/range/range.h"
namespace commander {
......@@ -15,8 +16,12 @@ namespace commander {
// A view model for a single command to be presented by the commander UI.
struct CommandItemViewModel {
public:
CommandItemViewModel(base::string16& title,
std::vector<gfx::Range>& matched_ranges);
CommandItemViewModel(
const base::string16& title,
const std::vector<gfx::Range>& matched_ranges,
CommandItem::Entity entity_type = CommandItem::Entity::kCommand,
const base::string16& annotation = base::string16());
explicit CommandItemViewModel(const CommandItem& item);
~CommandItemViewModel();
CommandItemViewModel(const CommandItemViewModel& other);
// The displayed title of the command.
......@@ -24,10 +29,16 @@ struct CommandItemViewModel {
// The locations of spans in |title| that should be emphasised to
// indicate to the user why the command was surfaced for their input.
std::vector<gfx::Range> matched_ranges;
// See CommandItem::Entity documentation.
CommandItem::Entity entity_type;
// Optional secondary text for the command. Typically used to display a
// hotkey.
base::string16 annotation;
};
// A view model for a set of results to be presented by the commander UI.
struct CommanderViewModel {
// The action
enum Action {
// Display the items in |items|.
kDisplayResults,
......
......@@ -7,10 +7,12 @@
#include "base/bind.h"
#include "base/i18n/case_conversion.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/ui/accelerator_utils.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/commander/fuzzy_finder.h"
#include "chrome/grit/generated_resources.h"
#include "components/strings/grit/components_strings.h"
#include "ui/base/accelerators/accelerator.h"
#include "ui/base/l10n/l10n_util.h"
namespace commander {
......@@ -50,6 +52,15 @@ CommandSource::CommandResults SimpleCommandSource::GetCommands(
item->title = title;
item->score = score;
item->matched_ranges = ranges;
ui::Accelerator accelerator;
ui::AcceleratorProvider* provider =
chrome::AcceleratorProviderForBrowser(browser);
if (provider->GetAcceleratorForCommandId(command_spec.command_id,
&accelerator)) {
item->annotation = accelerator.GetShortcutText();
}
// TODO(lgrey): For base::Unretained to be safe here, we need to ensure
// that if |browser| is destroyed, the palette is reset. It's likely
// that this will be the case anyway, but leaving this comment so:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment