[Commander]: Add custom icon (#19985)

This commit is contained in:
Jay Harris
2023-09-06 13:29:12 +09:00
committed by GitHub
parent e0334b1b6f
commit 12aad1f8d2
7 changed files with 113 additions and 58 deletions
@@ -0,0 +1,26 @@
// Copyright (c) 2023 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.
#include "components/omnibox/browser/autocomplete_match.h"
#include "brave/components/omnibox/browser/commander_provider.h"
#include "brave/components/vector_icons/vector_icons.h"
#if (!BUILDFLAG(IS_ANDROID) || BUILDFLAG(ENABLE_VR)) && !BUILDFLAG(IS_IOS)
const gfx::VectorIcon& AutocompleteMatch::GetVectorIcon(
bool is_bookmark,
const TemplateURL* turl) const {
if (!GetAdditionalInfo(commander::kCommanderMatchMarker).empty()) {
return kLeoCaratRightIcon;
}
return GetVectorIcon_Chromium(is_bookmark, turl);
}
#define GetVectorIcon GetVectorIcon_Chromium
#endif
#include "src/components/omnibox/browser/autocomplete_match.cc" // IWYU pragma: export
#undef GetVectorIcon
@@ -0,0 +1,17 @@
// Copyright (c) 2023 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.
#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_OMNIBOX_BROWSER_AUTOCOMPLETE_MATCH_H_
#define BRAVE_CHROMIUM_SRC_COMPONENTS_OMNIBOX_BROWSER_AUTOCOMPLETE_MATCH_H_
#define GetVectorIcon \
GetVectorIcon_Chromium(bool is_bookmark, const TemplateURL* turl) const; \
const gfx::VectorIcon& GetVectorIcon
#include "src/components/omnibox/browser/autocomplete_match.h" // IWYU pragma: export
#undef GetVectorIcon
#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_OMNIBOX_BROWSER_AUTOCOMPLETE_MATCH_H_
+4 -1
View File
@@ -48,6 +48,9 @@ source_set("unit_tests") {
sources +=
[ "//brave/components/omnibox/browser/commander_provider_unittest.cc" ]
deps += [ "//brave/components/commander/browser" ]
deps += [
"//brave/components/commander/browser",
"//brave/components/vector_icons",
]
}
}
@@ -72,6 +72,7 @@ void CommanderProvider::OnCommanderUpdated() {
const auto& option = items[i];
AutocompleteMatch match(this, rank--, false,
AutocompleteMatchType::BOOKMARK_TITLE);
match.RecordAdditionalInfo(kCommanderMatchMarker, true);
match.takeover_action =
base::MakeRefCounted<CommanderAction>(i, delegate->GetResultSetId());
@@ -82,8 +83,7 @@ void CommanderProvider::OnCommanderUpdated() {
match.contents_class = {
ACMatchClassification(0, ACMatchClassification::DIM)};
}
match.description =
base::StrCat({commander::kCommandPrefix, u" ", option.title});
match.description = option.title;
match.allowed_to_be_default_match = true;
match.swap_contents_and_description = true;
// We don't want to change the prompt at all while the user is going through
@@ -92,9 +92,6 @@ void CommanderProvider::OnCommanderUpdated() {
match.description_class = {
ACMatchClassification(0, ACMatchClassification::DIM)};
// All commands have a ":> " prefix added to them, so make sure we take it
// into account when mapping over the matched ranges.
const int offset = commander::kCommandPrefix.size() + 1;
for (size_t j = 0; j < option.matched_ranges.size(); ++j) {
auto range = option.matched_ranges[j];
// If the match has no length (as in the case of the empty string match)
@@ -103,18 +100,24 @@ void CommanderProvider::OnCommanderUpdated() {
continue;
}
// Start the match classification at the start of the matching range.
match.description_class.push_back(ACMatchClassification(
range.start() + offset, ACMatchClassification::MATCH));
// If the match starts from the beginning of the text, convert our
// starting style to MATCH.
if (range.start() == 0) {
match.description_class[0].style = ACMatchClassification::MATCH;
} else {
// Otherwise, change the style to be match, from this token onwards.
match.description_class.push_back(
ACMatchClassification(range.start(), ACMatchClassification::MATCH));
}
// If the end of the range isn't the last character in the string, and
// this range doesn't intersect with the next one, change the
// classification back to DIM from the end of this range.
if (range.end() + offset < match.description.size() &&
if (range.end() < match.description.size() &&
(j + 1 >= option.matched_ranges.size() ||
option.matched_ranges[j + 1].start() > range.end())) {
match.description_class.push_back(ACMatchClassification(
range.end() + offset, ACMatchClassification::DIM));
match.description_class.push_back(
ACMatchClassification(range.end(), ACMatchClassification::DIM));
}
}
matches_.push_back(match);
@@ -17,6 +17,13 @@ class AutocompleteProviderClient;
class AutocompleteProviderListener;
namespace commander {
// Used to distinguish matches for command items from regular
// AutocompleteMatches. Other match types are defined in an enum, but to avoid
// patching as much as possible, we're just using this marker in the
// additional_info lookup to determine whether to show our custom icon.
constexpr char kCommanderMatchMarker[] = "command-match";
class CommanderProvider
: public AutocompleteProvider,
public commander::CommanderFrontendDelegate::Observer {
@@ -21,6 +21,7 @@
#include "brave/components/commander/common/constants.h"
#include "brave/components/commander/common/features.h"
#include "brave/components/omnibox/browser/brave_fake_autocomplete_provider_client.h"
#include "brave/components/vector_icons/vector_icons.h"
#include "components/omnibox/browser/autocomplete_input.h"
#include "components/omnibox/browser/autocomplete_match.h"
#include "components/omnibox/browser/test_scheme_classifier.h"
@@ -29,8 +30,6 @@
#include "ui/gfx/range/range.h"
namespace {
const unsigned int kClassificationOffset =
1u + commander::kCommandPrefix.size();
class FakeCommanderDelegate : public commander::CommanderFrontendDelegate {
public:
@@ -157,10 +156,10 @@ TEST_F(CommanderProviderTest, ItemsAreConvertedToMatches) {
EXPECT_EQ(2u, provider()->matches().size());
EXPECT_EQ(u":> First", provider()->matches()[0].description);
EXPECT_EQ(u"First", provider()->matches()[0].description);
EXPECT_EQ(u"Ctrl+F", provider()->matches()[0].contents);
EXPECT_EQ(u":> Second", provider()->matches()[1].description);
EXPECT_EQ(u"Second", provider()->matches()[1].description);
EXPECT_EQ(u"Ctrl+S", provider()->matches()[1].contents);
for (const auto& match : provider()->matches()) {
@@ -231,19 +230,15 @@ TEST_F(CommanderProviderTest, OneCharMatchIsHighlighted) {
EXPECT_EQ(1u, provider()->matches().size());
const auto& c = provider()->matches()[0].description_class;
ASSERT_EQ(3u, c.size());
// :> is DIM
EXPECT_EQ(0u, c[0].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[0].style);
ASSERT_EQ(2u, c.size());
// F is MATCH
EXPECT_EQ(kClassificationOffset, c[1].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[1].style);
EXPECT_EQ(0u, c[0].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[0].style);
// oo should be DIM, as it didn't match
EXPECT_EQ(1u + kClassificationOffset, c[2].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[2].style);
EXPECT_EQ(1u, c[1].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[1].style);
}
// Note: The AutocompleteClassifier gets unhappy if the style switches back and
@@ -260,24 +255,19 @@ TEST_F(CommanderProviderTest, AdjacentMatchesDontSwitchBackAndForth) {
const auto& c = provider()->matches()[0].description_class;
EXPECT_EQ(1u, provider()->matches().size());
ASSERT_EQ(4u, c.size());
// :> Foo
// :> is DIM
EXPECT_EQ(0u, c[0].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[0].style);
ASSERT_EQ(3u, c.size());
// F is MATCH, but shouldn't add a closing DIM
EXPECT_EQ(kClassificationOffset, c[1].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[1].style);
EXPECT_EQ(0u, c[0].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[0].style);
// first o is also MATCH
EXPECT_EQ(1u + kClassificationOffset, c[2].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[2].style);
EXPECT_EQ(1u, c[1].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[1].style);
// second o should be DIM, as it didn't match
EXPECT_EQ(2u + kClassificationOffset, c[3].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[3].style);
EXPECT_EQ(2u, c[2].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[2].style);
}
TEST_F(CommanderProviderTest, FullLengthMatchIsApplied) {
@@ -291,16 +281,11 @@ TEST_F(CommanderProviderTest, FullLengthMatchIsApplied) {
const auto& c = provider()->matches()[0].description_class;
EXPECT_EQ(1u, provider()->matches().size());
ASSERT_EQ(2u, c.size());
// :> Foo
// :> is DIM
EXPECT_EQ(0u, c[0].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[0].style);
ASSERT_EQ(1u, c.size());
// Foo is MATCH
EXPECT_EQ(kClassificationOffset, c[1].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[1].style);
EXPECT_EQ(0u, c[0].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[0].style);
}
TEST_F(CommanderProviderTest, MatchesCanHaveGaps) {
@@ -314,23 +299,36 @@ TEST_F(CommanderProviderTest, MatchesCanHaveGaps) {
const auto& c = provider()->matches()[0].description_class;
EXPECT_EQ(1u, provider()->matches().size());
ASSERT_EQ(5u, c.size());
ASSERT_EQ(4u, c.size());
// |:> |Foo Bar is DIM
// |Fo|o Bar is MATCH
EXPECT_EQ(0u, c[0].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[0].style);
EXPECT_EQ(ACMatchClassification::MATCH, c[0].style);
// :> |Fo|o Bar is MATCH
EXPECT_EQ(kClassificationOffset, c[1].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[1].style);
EXPECT_EQ(2u, c[1].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[1].style);
EXPECT_EQ(2u + kClassificationOffset, c[2].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[2].style);
// Foo |Ba|r is MATCH
EXPECT_EQ(4u, c[2].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[2].style);
// :> Foo |Ba|r is MATCH
EXPECT_EQ(4u + kClassificationOffset, c[3].offset);
EXPECT_EQ(ACMatchClassification::MATCH, c[3].style);
EXPECT_EQ(6u, c[3].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[3].style);
}
EXPECT_EQ(6u + kClassificationOffset, c[4].offset);
EXPECT_EQ(ACMatchClassification::DIM, c[4].style);
TEST_F(CommanderProviderTest, MatchesHaveCustomIcon) {
provider()->Start(
CreateInput(base::StrCat({commander::kCommandPrefix, u"FoBa"})), false);
delegate()->Notify(
{commander::CommandItemModel(u"Foo Bar",
{gfx::Range(0, 2), gfx::Range(4, 6)}, u""),
commander::CommandItemModel(u"Fizz Bazz",
{gfx::Range(0, 2), gfx::Range(4, 6)}, u"")},
u"What thing?");
EXPECT_EQ(2u, provider()->matches().size());
for (const auto& result : provider()->matches()) {
EXPECT_EQ(&kLeoCaratRightIcon, &result.GetVectorIcon(false, nullptr));
}
}
+1
View File
@@ -24,6 +24,7 @@ aggregate_vector_icons("brave_components_vector_icons") {
"leo_browser_bookmark_plural.icon",
"leo_browser_extensions.icon",
"leo_browser_sidebar_right.icon",
"leo_carat_right.icon",
"leo_check_circle_filled.icon",
"leo_check_circle_outline.icon",
"leo_chrome_cast.icon",