Commit 06704df5 authored by Peter Boström's avatar Peter Boström Committed by Commit Bot

Harmonize Translate UI Bubble

Adds somewhat-compliant Harmony style to the translate bubble. This UI
is in the process of being reworked, so minor issues may be allowed to
fall through the cracks in the interrim.

Bug: chromium:730521
Change-Id: Ic8274ad285e41ce7750c41caaeb67010fd9e153c
Reviewed-on: https://chromium-review.googlesource.com/815292
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Reviewed-by: default avatarRachel Blum <groby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524313}
parent 94a5b918
......@@ -8485,13 +8485,19 @@ Please help our engineers fix this problem. Tell us what happened right before y
<!-- Translate Bubble -->
<if expr="toolkit_views or is_macosx">
<if expr="not use_titlecase">
<message name="IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE" desc="Text to show for the translate bubble label when that page is in specified language and ask if should translate.">
Would you like to translate this page?
<message name="IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_TITLE" desc="Title text for the translate bubble when asking to translate a page.">
Do you want to translate this page?
</message>
<message name="IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_NEW" desc="Text to show for the translate bubble label when that page is in specified language and ask if should translate.">
Do you want Google to translate this page from <ph name="source_language">$1<ex>Spanish</ex></ph> to <ph name="target_language">$2<ex>English</ex></ph>?
</message>
<message name="IDS_TRANSLATE_BUBBLE_ADVANCED" desc="Text to show for the translate bubble link label to jump to the advanced panel.">
<message name="IDS_TRANSLATE_BUBBLE_ADVANCED_LINK" desc="Text to show for the translate bubble link label to jump to the advanced panel.">
Options
</message>
<message name="IDS_TRANSLATE_BUBBLE_ADVANCED_BUTTON" desc="Text to show for the translate bubble button to jump to the advanced panel.">
More options
</message>
<message name="IDS_TRANSLATE_BUBBLE_ADVANCED_TITLE" desc="Title text for the options panel.">
Options
</message>
<message name="IDS_TRANSLATE_BUBBLE_ACCEPT" desc="Text to show for the translate bubble button to accept translation.">
......@@ -8512,6 +8518,9 @@ Please help our engineers fix this problem. Tell us what happened right before y
<message name="IDS_TRANSLATE_BUBBLE_TRANSLATED" desc="Text to show for the translate bubble label when the page has been translated from one language to another">
This page has been translated.
</message>
<message name="IDS_TRANSLATE_BUBBLE_TRANSLATED_TITLE" desc="Title text for the translate bubble label when the page has been translated from one language to another">
This page has been translated
</message>
<message name="IDS_TRANSLATE_BUBBLE_REVERT" desc="Text to show for the translate bubble button to revert translation of translated page">
Show original
</message>
......@@ -8530,6 +8539,9 @@ Please help our engineers fix this problem. Tell us what happened right before y
<message name="IDS_TRANSLATE_BUBBLE_COULD_NOT_TRANSLATE" desc="Text to show for the translate bubble label when the page could not be translated.">
This page could not be translated.
</message>
<message name="IDS_TRANSLATE_BUBBLE_COULD_NOT_TRANSLATE_TITLE" desc="Title text for the translate bubble label when the page could not be translated.">
This page could not be translated
</message>
<message name="IDS_TRANSLATE_BUBBLE_PAGE_LANGUAGE" desc="Text to show for the translate bubble label next to the combobox of the page language">
Page language:
</message>
......@@ -8541,13 +8553,19 @@ Please help our engineers fix this problem. Tell us what happened right before y
</message>
</if>
<if expr="use_titlecase">
<message name="IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE" desc="In Title Case: Text to show for the translate bubble label when that page is in specified language and ask if should translate.">
Would You Like To Translate This Page?
<message name="IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_TITLE" desc="In Title Case: Title text for the translate bubble when asking to translate a page.">
Do You Want To Translate This Page?
</message>
<message name="IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_NEW" desc="In Title Case: Text to show for the translate bubble label when that page is in specified language and ask if should translate.">
Do You Want Google To Translate This Page From <ph name="source_language">$1<ex>Spanish</ex></ph> To <ph name="target_language">$2<ex>English</ex></ph>?
</message>
<message name="IDS_TRANSLATE_BUBBLE_ADVANCED" desc="In Title Case: Text to show for the translate bubble link label to jump to the advanced panel.">
<message name="IDS_TRANSLATE_BUBBLE_ADVANCED_LINK" desc="In Title Case: Text to show for the translate bubble link label to jump to the advanced panel.">
Options
</message>
<message name="IDS_TRANSLATE_BUBBLE_ADVANCED_BUTTON" desc="In Title Case: Text to show for the translate bubble button to jump to the advanced panel.">
More Options
</message>
<message name="IDS_TRANSLATE_BUBBLE_ADVANCED_TITLE" desc="In Title Case: Title text for the options panel.">
Options
</message>
<message name="IDS_TRANSLATE_BUBBLE_ACCEPT" desc="In Title Case: Text to show for the translate bubble button to accept translation.">
......@@ -8568,6 +8586,9 @@ Please help our engineers fix this problem. Tell us what happened right before y
<message name="IDS_TRANSLATE_BUBBLE_TRANSLATED" desc="In Title Case: Text to show for the translate bubble label when the page has been translated from one language to another">
This Page Has Been Translated.
</message>
<message name="IDS_TRANSLATE_BUBBLE_TRANSLATED_TITLE" desc="In Title Case: Title text for the translate bubble label when the page has been translated from one language to another">
This Page Has Been Translated
</message>
<message name="IDS_TRANSLATE_BUBBLE_REVERT" desc="In Title Case: Text to show for the translate bubble button to revert translation of translated page">
Show Original
</message>
......@@ -8586,6 +8607,9 @@ Please help our engineers fix this problem. Tell us what happened right before y
<message name="IDS_TRANSLATE_BUBBLE_COULD_NOT_TRANSLATE" desc="In Title Case: Text to show for the translate bubble label when the page could not be translated.">
This Page Could Not Be Translated.
</message>
<message name="IDS_TRANSLATE_BUBBLE_COULD_NOT_TRANSLATE_TITLE" desc="In Title Case: Title text for the translate bubble label when the page could not be translated.">
This Page Could Not Be Translated
</message>
<message name="IDS_TRANSLATE_BUBBLE_PAGE_LANGUAGE" desc="In Title Case: Text to show for the translate bubble label next to the combobox of the page language">
Page Language:
</message>
......
......@@ -426,7 +426,7 @@ const CGFloat kContentWidth = kWindowWidth - 2 * kFramePadding;
l10n_util::GetNSStringWithFixup(IDS_TRANSLATE_BUBBLE_TRANSLATED);
NSTextField* textLabel = [self addText:message
toView:view];
message = l10n_util::GetNSStringWithFixup(IDS_TRANSLATE_BUBBLE_ADVANCED);
message = l10n_util::GetNSStringWithFixup(IDS_TRANSLATE_BUBBLE_ADVANCED_LINK);
NSButton* advancedLinkButton =
[self addLinkButtonWithText:message
action:@selector(handleAdvancedLinkButtonPressed:)
......@@ -471,7 +471,7 @@ const CGFloat kContentWidth = kWindowWidth - 2 * kFramePadding;
NSString* message =
l10n_util::GetNSString(IDS_TRANSLATE_BUBBLE_COULD_NOT_TRANSLATE);
NSTextField* textLabel = [self addText:message toView:view];
message = l10n_util::GetNSStringWithFixup(IDS_TRANSLATE_BUBBLE_ADVANCED);
message = l10n_util::GetNSStringWithFixup(IDS_TRANSLATE_BUBBLE_ADVANCED_LINK);
NSButton* advancedLinkButton =
[self addLinkButtonWithText:message
action:@selector(handleAdvancedLinkButtonPressed:)
......
......@@ -79,6 +79,12 @@ enum TranslateBubbleUiEvent {
BUBBLE_NOT_SHOWN_WEB_CONTENTS_NOT_ACTIVE,
BUBBLE_NOT_SHOWN_EDITABLE_FIELD_IS_ACTIVE,
// The user clicked the advanced menu item.
ADVANCED_MENU_CLICKED,
// The user clicked the advanced button.
ADVANCED_BUTTON_CLICKED,
TRANSLATE_BUBBLE_UI_EVENT_MAX
};
......
......@@ -36,10 +36,6 @@ class LabelButton;
class View;
}
namespace ui {
class SimpleComboboxModel;
}
class TranslateBubbleView : public LocationBarBubbleDelegateView,
public views::ButtonListener,
public views::ComboboxListener,
......@@ -49,19 +45,12 @@ class TranslateBubbleView : public LocationBarBubbleDelegateView,
public views::StyledLabelListener,
public content::WebContentsObserver {
public:
// Commands shown in the action-style combobox. The value corresponds to the
// position in the combobox menu.
enum class DenialComboboxIndex {
DONT_TRANSLATE = 0,
NEVER_TRANSLATE_LANGUAGE = 1,
SEPARATOR = 2,
MENU_SIZE_NO_BLACKLIST = SEPARATOR,
NEVER_TRANSLATE_SITE = 3,
MENU_SIZE = 4,
};
// Item IDs for the denial button's menu.
enum DenialMenuItem { NEVER_TRANSLATE_LANGUAGE, NEVER_TRANSLATE_SITE };
enum DenialMenuItem {
NEVER_TRANSLATE_LANGUAGE,
NEVER_TRANSLATE_SITE,
MORE_OPTIONS
};
~TranslateBubbleView() override;
......@@ -88,6 +77,7 @@ class TranslateBubbleView : public LocationBarBubbleDelegateView,
// views::BubbleDialogDelegateView methods.
int GetDialogButtons() const override;
base::string16 GetWindowTitle() const override;
void Init() override;
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
......@@ -147,10 +137,10 @@ class TranslateBubbleView : public LocationBarBubbleDelegateView,
BUTTON_ID_SHOW_ORIGINAL,
BUTTON_ID_TRY_AGAIN,
BUTTON_ID_ALWAYS_TRANSLATE,
BUTTON_ID_ADVANCED,
};
enum ComboboxID {
COMBOBOX_ID_DENIAL,
COMBOBOX_ID_SOURCE_LANGUAGE,
COMBOBOX_ID_TARGET_LANGUAGE,
};
......@@ -163,6 +153,7 @@ class TranslateBubbleView : public LocationBarBubbleDelegateView,
const ::base::string16&);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest, TranslateButton);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest, TranslateButtonIn2016Q2UI);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest, CloseButton);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest, CloseButtonIn2016Q2UI);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest, AdvancedLink);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest, AdvancedLinkIn2016Q2UI);
......@@ -180,6 +171,12 @@ class TranslateBubbleView : public LocationBarBubbleDelegateView,
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest,
CancelButtonReturningAfterTranslate);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest, CancelButtonReturningError);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest,
DenialMenuNeverTranslateLanguage);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest,
DenialMenuRespectsBlacklistSite);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewTest,
DenialMenuNeverTranslateSite);
FRIEND_TEST_ALL_PREFIXES(TranslateLanguageBrowserTest, TranslateAndRevert);
FRIEND_TEST_ALL_PREFIXES(TranslateBubbleViewBrowserTest,
CheckNeverTranslateThisSiteBlacklist);
......@@ -243,11 +240,9 @@ class TranslateBubbleView : public LocationBarBubbleDelegateView,
views::View* error_view_;
views::View* advanced_view_;
std::unique_ptr<ui::SimpleComboboxModel> denial_combobox_model_;
std::unique_ptr<LanguageComboboxModel> source_language_combobox_model_;
std::unique_ptr<LanguageComboboxModel> target_language_combobox_model_;
views::Combobox* denial_combobox_;
views::Combobox* source_language_combobox_;
views::Combobox* target_language_combobox_;
......
......@@ -25,8 +25,8 @@
#include "components/translate/core/common/language_detection_details.h"
#include "content/public/browser/notification_details.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/models/simple_combobox_model.h"
#include "ui/views/controls/combobox/combobox.h"
#include "ui/events/keycodes/dom/dom_code.h"
#include "ui/views/controls/button/menu_button.h"
class TranslateBubbleViewBrowserTest : public InProcessBrowserTest {
public:
......@@ -132,37 +132,3 @@ IN_PROC_BROWSER_TEST_F(TranslateBubbleViewBrowserTest,
browser()->tab_strip_model()->GetActiveWebContents(),
false);
}
IN_PROC_BROWSER_TEST_F(TranslateBubbleViewBrowserTest,
CheckNeverTranslateThisSiteBlacklist) {
EXPECT_FALSE(TranslateBubbleView::GetCurrentBubble());
// Show a French page and wait until the bubble is shown.
GURL french_url = ui_test_utils::GetTestUrl(
base::FilePath(), base::FilePath(FILE_PATH_LITERAL("french_page.html")));
NavigateAndWaitForLanguageDetection(french_url, "fr");
const TranslateBubbleView* const bubble =
TranslateBubbleView::GetCurrentBubble();
EXPECT_TRUE(bubble);
// Since this is a file:/// URL, we should not be able to blacklist.
EXPECT_FALSE(bubble->model_->CanBlacklistSite());
ASSERT_FALSE(base::FeatureList::IsEnabled(translate::kTranslateUI2016Q2));
EXPECT_TRUE(bubble->denial_combobox_);
// Check the combobox contains the correct items.
EXPECT_EQ(
bubble->denial_combobox_->GetRowCount(),
static_cast<int>(
TranslateBubbleView::DenialComboboxIndex::MENU_SIZE_NO_BLACKLIST));
EXPECT_EQ(bubble->denial_combobox_->GetTextForRow(0),
l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_DENY));
EXPECT_EQ(
bubble->denial_combobox_->GetTextForRow(1),
l10n_util::GetStringFUTF16(IDS_TRANSLATE_BUBBLE_NEVER_TRANSLATE_LANG,
base::ASCIIToUTF16("French")));
chrome::CloseWindow(browser());
EXPECT_FALSE(TranslateBubbleView::GetCurrentBubble());
}
......@@ -18,12 +18,13 @@
#include "ui/base/l10n/l10n_util.h"
#include "ui/events/event_constants.h"
#include "ui/events/event_utils.h"
#include "ui/events/keycodes/dom/dom_code.h"
#include "ui/gfx/range/range.h"
#include "ui/views/bubble/bubble_frame_view.h"
#include "ui/views/controls/button/checkbox.h"
#include "ui/views/controls/button/menu_button.h"
#include "ui/views/controls/combobox/combobox.h"
#include "ui/views/controls/styled_label.h"
#include "ui/views/test/combobox_test_api.h"
#include "ui/views/test/views_test_base.h"
#include "ui/views/widget/widget.h"
......@@ -189,8 +190,15 @@ class TranslateBubbleViewTest : public views::ViewsTestBase {
views::ViewsTestBase::TearDown();
}
views::Combobox* denial_combobox() { return bubble_->denial_combobox_; }
bool denial_button_clicked() { return mock_model_->translation_declined_; }
void TriggerDenialMenu() {
bubble_->denial_menu_button_->OnKeyPressed(ui::KeyEvent(
ui::ET_KEY_PRESSED, ui::VKEY_RETURN, ui::DomCode::ENTER, ui::EF_NONE));
}
ui::SimpleMenuModel* denial_menu_model() {
return bubble_->denial_menu_model_.get();
}
std::unique_ptr<views::Widget> anchor_widget_;
MockTranslateBubbleModel* mock_model_;
......@@ -234,41 +242,54 @@ TEST_F(TranslateBubbleViewTest, CloseButtonIn2016Q2UI) {
EXPECT_TRUE(mock_model_->translation_declined_);
}
TEST_F(TranslateBubbleViewTest, ComboboxNope) {
TEST_F(TranslateBubbleViewTest, CloseButton) {
TurnOnTranslate2016Q2UIFlag();
CreateAndShowBubble();
views::test::ComboboxTestApi test_api(denial_combobox());
EXPECT_FALSE(denial_button_clicked());
EXPECT_FALSE(mock_model_->translate_called_);
EXPECT_FALSE(mock_model_->translation_declined_);
EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
test_api.PerformActionAt(static_cast<int>(
TranslateBubbleView::DenialComboboxIndex::DONT_TRANSLATE));
EXPECT_TRUE(denial_button_clicked());
EXPECT_TRUE(bubble_->GetWidget()->IsClosed());
// Press the "Close" button.
bubble_->GetBubbleFrameView()->ButtonPressed(
bubble_->GetBubbleFrameView()->GetCloseButtonForTest(),
ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
ui::EventTimeForNow(), ui::EF_NONE, ui::EF_NONE));
EXPECT_FALSE(mock_model_->translate_called_);
EXPECT_TRUE(mock_model_->translation_declined_);
}
TEST_F(TranslateBubbleViewTest, ComboboxNeverTranslateLanguage) {
TEST_F(TranslateBubbleViewTest, DenialMenuNeverTranslateLanguage) {
CreateAndShowBubble();
views::test::ComboboxTestApi test_api(denial_combobox());
EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
EXPECT_FALSE(mock_model_->never_translate_language_);
EXPECT_FALSE(denial_button_clicked());
test_api.PerformActionAt(static_cast<int>(
TranslateBubbleView::DenialComboboxIndex::NEVER_TRANSLATE_LANGUAGE));
TriggerDenialMenu();
const int index = bubble_->denial_menu_model_->GetIndexOfCommandId(
TranslateBubbleView::NEVER_TRANSLATE_LANGUAGE);
bubble_->denial_menu_model_->ActivatedAt(index);
EXPECT_TRUE(denial_button_clicked());
EXPECT_TRUE(mock_model_->never_translate_language_);
EXPECT_TRUE(bubble_->GetWidget()->IsClosed());
}
TEST_F(TranslateBubbleViewTest, ComboboxNeverTranslateSite) {
TEST_F(TranslateBubbleViewTest, DenialMenuNeverTranslateSite) {
// NEVER_TRANSLATE_SITE should only show up for sites that can be blacklisted.
mock_model_->SetCanBlacklistSite(true);
CreateAndShowBubble();
views::test::ComboboxTestApi test_api(denial_combobox());
EXPECT_FALSE(mock_model_->never_translate_site_);
EXPECT_FALSE(denial_button_clicked());
EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
test_api.PerformActionAt(static_cast<int>(
TranslateBubbleView::DenialComboboxIndex::NEVER_TRANSLATE_SITE));
TriggerDenialMenu();
const int index = bubble_->denial_menu_model_->GetIndexOfCommandId(
TranslateBubbleView::NEVER_TRANSLATE_SITE);
bubble_->denial_menu_model_->ActivatedAt(index);
EXPECT_TRUE(denial_button_clicked());
EXPECT_TRUE(mock_model_->never_translate_site_);
EXPECT_TRUE(bubble_->GetWidget()->IsClosed());
......@@ -478,44 +499,15 @@ TEST_F(TranslateBubbleViewTest, CancelButtonReturningError) {
EXPECT_EQ(TranslateBubbleModel::VIEW_STATE_ERROR, bubble_->GetViewState());
}
TEST_F(TranslateBubbleViewTest, ComboboxCanBlacklistSite) {
EXPECT_TRUE(mock_model_->CanBlacklistSite());
CreateAndShowBubble();
views::Combobox* const combobox = denial_combobox();
EXPECT_FALSE(denial_button_clicked());
EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
// Check menu rows are DENY, NEVER_TRANSLATE_LANG, [SEPARATOR], and
// NEVER_TRANSLATE_SITE.
EXPECT_EQ(4, combobox->GetRowCount());
EXPECT_EQ(l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_DENY),
combobox->GetTextForRow(static_cast<size_t>(
TranslateBubbleView::DenialComboboxIndex::DONT_TRANSLATE)));
EXPECT_EQ(
l10n_util::GetStringFUTF16(IDS_TRANSLATE_BUBBLE_NEVER_TRANSLATE_LANG,
base::ASCIIToUTF16("English")),
combobox->GetTextForRow(static_cast<size_t>(
TranslateBubbleView::DenialComboboxIndex::NEVER_TRANSLATE_LANGUAGE)));
EXPECT_EQ(
l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_NEVER_TRANSLATE_SITE),
combobox->GetTextForRow(static_cast<size_t>(
TranslateBubbleView::DenialComboboxIndex::NEVER_TRANSLATE_SITE)));
}
TEST_F(TranslateBubbleViewTest, ComboboxCantBlacklistSite) {
TEST_F(TranslateBubbleViewTest, DenialMenuRespectsBlacklistSite) {
mock_model_->SetCanBlacklistSite(false);
CreateAndShowBubble();
views::Combobox* const combobox = denial_combobox();
// views::test::ComboboxTestApi test_api(combobox);
EXPECT_FALSE(denial_button_clicked());
EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
// Check the menu rows are DENY and NEVER_TRANSLATE_LANG.
EXPECT_EQ(2, combobox->GetRowCount());
EXPECT_EQ(l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_DENY),
combobox->GetTextForRow(static_cast<size_t>(
TranslateBubbleView::DenialComboboxIndex::DONT_TRANSLATE)));
EXPECT_EQ(
l10n_util::GetStringFUTF16(IDS_TRANSLATE_BUBBLE_NEVER_TRANSLATE_LANG,
base::ASCIIToUTF16("English")),
combobox->GetTextForRow(static_cast<size_t>(
TranslateBubbleView::DenialComboboxIndex::NEVER_TRANSLATE_LANGUAGE)));
TriggerDenialMenu();
// NEVER_TRANSLATE_SITE shouldn't show up for sites that can't be blacklisted.
EXPECT_EQ(-1, bubble_->denial_menu_model_->GetIndexOfCommandId(
TranslateBubbleView::NEVER_TRANSLATE_SITE));
// Verify that the menu is populated so previous check makes sense.
EXPECT_GE(0, bubble_->denial_menu_model_->GetIndexOfCommandId(
TranslateBubbleView::NEVER_TRANSLATE_LANGUAGE));
}
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