Commit 152e647d authored by Tommy C. Li's avatar Tommy C. Li Committed by Commit Bot

[omnibox] Suggestion Transparency: Separate Info and Remove dialogs

Separate the "More Info / Remove..." context menu entry for
Suggestion Transparency into two context menu entries:

 - Why this suggestion
 - Remove suggestion

This was vetted by our UX folks, and so we are also committing these
strings to our translated strings.

The implementation of the "Why this suggestion" info bubble will be in
a separate CL.

Bug: 929477
Change-Id: Ia406107bf9f6b50354f2b8f9ccf6b472571d799d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1610846
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: default avatarmanuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659298}
parent 02f33c86
...@@ -4915,6 +4915,12 @@ Keep your key file in a safe place. You will need it to create new versions of y ...@@ -4915,6 +4915,12 @@ Keep your key file in a safe place. You will need it to create new versions of y
Search or type web address Search or type web address
</message> </message>
</if> </if>
<message name="IDS_OMNIBOX_WHY_THIS_SUGGESTION" desc="The title of a menu entry and bubble that explains why this suggestion appears in the omnibox popup.">
Why this suggestion?
</message>
<message name="IDS_OMNIBOX_REMOVE_SUGGESTION" desc="The title of a menu entry and bubble used to delete omnibox suggestions.">
Remove suggestion
</message>
<!-- NTP --> <!-- NTP -->
<message name="IDS_GOOGLE_SEARCH_BOX_EMPTY_HINT" desc="The text displayed in the fakebox (on the New Tab page) when it is empty, and Google is the default search engine."> <message name="IDS_GOOGLE_SEARCH_BOX_EMPTY_HINT" desc="The text displayed in the fakebox (on the New Tab page) when it is empty, and Google is the default search engine.">
......
fbf9ef5e0af0fb251a41993ae4e833710716439f
\ No newline at end of file
fbf9ef5e0af0fb251a41993ae4e833710716439f
\ No newline at end of file
...@@ -65,9 +65,10 @@ OmniboxResultView::OmniboxResultView( ...@@ -65,9 +65,10 @@ OmniboxResultView::OmniboxResultView(
if (base::FeatureList::IsEnabled( if (base::FeatureList::IsEnabled(
omnibox::kOmniboxSuggestionTransparencyOptions)) { omnibox::kOmniboxSuggestionTransparencyOptions)) {
// TODO(tommycli): Replace this with the real translated string from UX. context_menu_contents_.AddItemWithStringId(IDS_OMNIBOX_WHY_THIS_SUGGESTION,
context_menu_contents_.AddItem(COMMAND_REMOVE_SUGGESTION, IDS_OMNIBOX_WHY_THIS_SUGGESTION);
base::ASCIIToUTF16("Remove suggestion...")); context_menu_contents_.AddItemWithStringId(IDS_OMNIBOX_REMOVE_SUGGESTION,
IDS_OMNIBOX_REMOVE_SUGGESTION);
set_context_menu_controller(this); set_context_menu_controller(this);
} }
} }
...@@ -446,21 +447,15 @@ void OmniboxResultView::ShowContextMenuForViewImpl( ...@@ -446,21 +447,15 @@ void OmniboxResultView::ShowContextMenuForViewImpl(
} }
// ui::SimpleMenuModel::Delegate overrides: // ui::SimpleMenuModel::Delegate overrides:
bool OmniboxResultView::IsItemForCommandIdDynamic(int command_id) const { bool OmniboxResultView::IsCommandIdVisible(int command_id) const {
DCHECK_EQ(COMMAND_REMOVE_SUGGESTION, command_id); if (command_id == IDS_OMNIBOX_REMOVE_SUGGESTION)
return true; return match_.SupportsDeletion();
}
base::string16 OmniboxResultView::GetLabelForCommandId(int command_id) const { DCHECK(command_id == IDS_OMNIBOX_WHY_THIS_SUGGESTION);
DCHECK_EQ(COMMAND_REMOVE_SUGGESTION, command_id); return true;
// TODO(tommycli): Replace this with the real translated string from UX.
return base::ASCIIToUTF16(match_.SupportsDeletion() ? "More info / Remove..."
: "More info...");
} }
void OmniboxResultView::ExecuteCommand(int command_id, int event_flags) { void OmniboxResultView::ExecuteCommand(int command_id, int event_flags) {
DCHECK_EQ(COMMAND_REMOVE_SUGGESTION, command_id);
// Temporarily inhibit the popup closing on blur while we open the remove // Temporarily inhibit the popup closing on blur while we open the remove
// suggestion confirmation bubble. // suggestion confirmation bubble.
popup_contents_view_->model()->set_popup_closes_on_blur(false); popup_contents_view_->model()->set_popup_closes_on_blur(false);
...@@ -470,9 +465,14 @@ void OmniboxResultView::ExecuteCommand(int command_id, int event_flags) { ...@@ -470,9 +465,14 @@ void OmniboxResultView::ExecuteCommand(int command_id, int event_flags) {
// class, and we don't want that for the bubble. We should improve this. // class, and we don't want that for the bubble. We should improve this.
AutocompleteMatch raw_match = AutocompleteMatch raw_match =
popup_contents_view_->model()->result().match_at(model_index_); popup_contents_view_->model()->result().match_at(model_index_);
ShowRemoveSuggestion(this, raw_match,
base::BindOnce(&OmniboxResultView::RemoveSuggestion, if (command_id == IDS_OMNIBOX_REMOVE_SUGGESTION) {
weak_factory_.GetWeakPtr())); ShowRemoveSuggestion(this, raw_match,
base::BindOnce(&OmniboxResultView::RemoveSuggestion,
weak_factory_.GetWeakPtr()));
} else if (command_id == IDS_OMNIBOX_WHY_THIS_SUGGESTION) {
ShowWhyThisSuggestion(this, raw_match);
}
popup_contents_view_->model()->set_popup_closes_on_blur(true); popup_contents_view_->model()->set_popup_closes_on_blur(true);
} }
......
...@@ -105,14 +105,10 @@ class OmniboxResultView : public views::View, ...@@ -105,14 +105,10 @@ class OmniboxResultView : public views::View,
ui::MenuSourceType source_type) override; ui::MenuSourceType source_type) override;
// ui::SimpleMenuModel::Delegate overrides: // ui::SimpleMenuModel::Delegate overrides:
bool IsItemForCommandIdDynamic(int command_id) const override; bool IsCommandIdVisible(int command_id) const override;
base::string16 GetLabelForCommandId(int command_id) const override;
void ExecuteCommand(int command_id, int event_flags) override; void ExecuteCommand(int command_id, int event_flags) override;
private: private:
// TODO(tommycli): This will be removed once we get final strings from UX.
enum CommandID { COMMAND_REMOVE_SUGGESTION };
// Returns the height of the text portion of the result view. // Returns the height of the text portion of the result view.
int GetTextHeight() const; int GetTextHeight() const;
......
...@@ -93,3 +93,8 @@ void ShowRemoveSuggestion(views::View* anchor_view, ...@@ -93,3 +93,8 @@ void ShowRemoveSuggestion(views::View* anchor_view,
std::move(remove_closure))) std::move(remove_closure)))
->Show(); ->Show();
} }
void ShowWhyThisSuggestion(views::View* anchor_view,
const AutocompleteMatch& match) {
// TODO(tommycli): Implement this.
}
...@@ -20,4 +20,8 @@ void ShowRemoveSuggestion(views::View* anchor_view, ...@@ -20,4 +20,8 @@ void ShowRemoveSuggestion(views::View* anchor_view,
const AutocompleteMatch& match, const AutocompleteMatch& match,
base::OnceClosure remove_closure); base::OnceClosure remove_closure);
// Shows a simple "Why this suggestion" info bubble for |match|.
void ShowWhyThisSuggestion(views::View* anchor_view,
const AutocompleteMatch& match);
#endif // CHROME_BROWSER_UI_VIEWS_OMNIBOX_REMOVE_SUGGESTION_BUBBLE_H_ #endif // CHROME_BROWSER_UI_VIEWS_OMNIBOX_REMOVE_SUGGESTION_BUBBLE_H_
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