Skip to content

GitLab

T
tangled
Commit d843c3ec authored by Richard Knoll's avatar Richard Knoll Committed by Commit Bot
Browse files
Options

Move origin view below list in SharingDialogView. 

To match the updated mocks we need to show the initiating origin below
the list of apps and devices. This also updates the paddings to match
the mocks.

Screenshot: https://imgur.com/mFoeyfg

Bug: 1011286
Change-Id: Id35fa58658364cc20f93181ebbc5651ee456c05c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1878189
Commit-Queue: Richard Knoll <knollr@chromium.org>
Reviewed-by: default avatarMichael van Ouwerkerk <mvanouwerkerk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708976}
parent c0f61670
Hide whitespace changes
Inline Side-by-side
Showing with 75 additions and 132 deletions
chrome/browser/ui/views/sharing/sharing_dialog_view.cc
View file @ d843c3ec
......@@ -22,10 +22,8 @@
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/color_utils.h"
#include "ui/gfx/font_list.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/strings/grit/ui_strings.h"
#include "ui/views/background.h"
#include "ui/views/border.h"
#include "ui/views/bubble/bubble_frame_view.h"
#include "ui/views/controls/styled_label.h"
......@@ -40,6 +38,8 @@
namespace {
constexpr int kSharingDialogSpacing = 8;
SkColor GetColorFromTheme() {
const ui::NativeTheme* native_theme =
ui::NativeTheme::GetInstanceForNativeUi();
......@@ -50,8 +50,6 @@ SkColor GetColorFromTheme() {
std::unique_ptr<views::ImageView> CreateIconView(const gfx::ImageSkia& icon) {
auto icon_view = std::make_unique<views::ImageView>();
icon_view->SetImage(icon);
constexpr auto kPrimaryIconBorder = gfx::Insets(8);
icon_view->SetBorder(views::CreateEmptyBorder(kPrimaryIconBorder));
return icon_view;
}
......@@ -98,42 +96,19 @@ std::unique_ptr<views::StyledLabel> CreateHelpText(
return label;
}
std::unique_ptr<views::View> MaybeCreateOriginView(
const SharingDialogData& data) {
if (!data.initiating_origin || !data.origin_text_id)
return nullptr;
std::unique_ptr<views::View> CreateOriginView(const SharingDialogData& data) {
DCHECK(data.initiating_origin);
DCHECK_NE(0, data.origin_text_id);
auto label = std::make_unique<views::Label>(
l10n_util::GetStringFUTF16(data.origin_text_id,
url_formatter::FormatOriginForSecurityDisplay(
*data.initiating_origin)),
views::style::CONTEXT_LABEL, views::style::STYLE_SECONDARY);
ChromeTextContext::CONTEXT_BODY_TEXT_SMALL,
views::style::STYLE_SECONDARY);
label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
label->SetElideBehavior(gfx::ELIDE_HEAD);
label->SetMultiLine(false);
label->SetFontList(label->font_list().DeriveWithSizeDelta(-2));
auto background_color =
ui::NativeTheme::GetInstanceForNativeUi()->GetSystemColor(
ui::NativeTheme::kColorId_BubbleFooterBackground);
label->SetBackgroundColor(background_color);
label->SetBackground(
views::CreateRoundedRectBackground(background_color, /*radius=*/8));
gfx::Insets insets =
ChromeLayoutProvider::Get()->GetDialogInsetsForContentType(views::TEXT,
views::TEXT);
gfx::Insets border_insets(4, insets.width() / 4, 4, insets.width() / 4);
gfx::Insets container_insets(insets.top(),
insets.left() - border_insets.left(), 0,
insets.right() - border_insets.right());
label->SetBorder(views::CreateEmptyBorder(border_insets));
auto container = std::make_unique<views::View>();
container->SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical, container_insets));
container->AddChildView(std::move(label));
return container;
return label;
}
std::unique_ptr<views::View> MaybeCreateImageView(int image_id) {
......@@ -211,11 +186,7 @@ void SharingDialogView::Init() {
case SharingDialogType::kDialogWithoutDevicesWithApp:
case SharingDialogType::kDialogWithDevicesMaybeApps:
// Spread buttons across the whole dialog width.
int top_margin = provider->GetDistanceMetric(
views::DISTANCE_DIALOG_CONTENT_MARGIN_TOP_CONTROL);
int bottom_margin = provider->GetDistanceMetric(
views::DISTANCE_DIALOG_CONTENT_MARGIN_BOTTOM_CONTROL);
insets = gfx::Insets(top_margin, 0, bottom_margin, 0);
insets = gfx::Insets(kSharingDialogSpacing, 0, kSharingDialogSpacing, 0);
InitListView();
break;
}
......@@ -260,22 +231,7 @@ void SharingDialogView::MaybeShowHeaderImage() {
? data_.header_image_dark
: data_.header_image_light;
std::unique_ptr<views::View> image_view = MaybeCreateImageView(image_id);
std::unique_ptr<views::View> origin_view = MaybeCreateOriginView(data_);
std::unique_ptr<views::View> header_view = std::make_unique<views::View>();
header_view->SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical));
if (image_view)
header_view->AddChildView(std::move(image_view));
if (origin_view)
header_view->AddChildView(std::move(origin_view));
// Clear header if it has no content.
if (header_view->children().empty())
header_view = nullptr;
frame_view->SetHeaderView(std::move(header_view));
frame_view->SetHeaderView(MaybeCreateImageView(image_id));
}
void SharingDialogView::AddedToWidget() {
......@@ -305,6 +261,11 @@ void SharingDialogView::OnThemeChanged() {
void SharingDialogView::InitListView() {
int tag = 0;
const gfx::Insets device_border =
gfx::Insets(kSharingDialogSpacing, kSharingDialogSpacing * 2,
kSharingDialogSpacing, 0);
// Apps need more padding at the top and bottom as they only have one line.
const gfx::Insets app_border = device_border + gfx::Insets(2, 0, 2, 0);
// Devices:
LogSharingDevicesToShow(data_.prefix, kSharingUiDialog, data_.devices.size());
......@@ -316,6 +277,7 @@ void SharingDialogView::InitListView() {
GetLastUpdatedTimeInDays(device->last_updated_timestamp()));
dialog_button->SetEnabled(true);
dialog_button->set_tag(tag++);
dialog_button->SetBorder(views::CreateEmptyBorder(device_border));
dialog_buttons_.push_back(AddChildView(std::move(dialog_button)));
}
......@@ -329,8 +291,24 @@ void SharingDialogView::InitListView() {
/* subtitle= */ base::string16());
dialog_button->SetEnabled(true);
dialog_button->set_tag(tag++);
dialog_button->SetBorder(views::CreateEmptyBorder(app_border));
dialog_buttons_.push_back(AddChildView(std::move(dialog_button)));
}
// Origin:
if (data_.initiating_origin &&
!data_.initiating_origin->IsSameOriginWith(
web_contents()->GetMainFrame()->GetLastCommittedOrigin())) {
auto* provider = ChromeLayoutProvider::Get();
const gfx::Insets dialog_insets =
provider->GetDialogInsetsForContentType(views::TEXT, views::TEXT);
const gfx::Insets origin_border = gfx::Insets(
kSharingDialogSpacing, dialog_insets.left(), 0, dialog_insets.right());
std::unique_ptr<views::View> origin_view = CreateOriginView(data_);
origin_view->SetBorder(views::CreateEmptyBorder(origin_border));
AddChildView(std::move(origin_view));
}
}
void SharingDialogView::InitEmptyView() {
......
chrome/browser/ui/views/sharing/sharing_dialog_view_unittest.cc
View file @ d843c3ec
......@@ -13,7 +13,7 @@
#include "chrome/browser/sharing/sharing_app.h"
#include "chrome/browser/sharing/sharing_metrics.h"
#include "chrome/browser/ui/views/hover_button.h"
#include "chrome/test/views/chrome_views_test_base.h"
#include "chrome/test/base/browser_with_test_window_test.h"
#include "components/sync_device_info/device_info.h"
#include "components/vector_icons/vector_icons.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -22,24 +22,23 @@
#include "ui/strings/grit/ui_strings.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h"
#include "ui/views/bubble/bubble_frame_view.h"
#include "ui/views/test/widget_test.h"
#include "url/gurl.h"
#include "url/origin.h"
namespace {
class SharingDialogViewMock : public SharingDialogView {
class SharingDialogViewFake : public SharingDialogView {
public:
SharingDialogViewMock(views::View* anchor_view,
SharingDialogViewFake(views::View* anchor_view,
content::WebContents* web_contents,
SharingDialogData data)
: SharingDialogView(anchor_view, web_contents, std::move(data)) {}
~SharingDialogViewMock() override = default;
~SharingDialogViewFake() override = default;
// The delegate cannot find widget since it is created from a null profile.
// This method will be called inside ButtonPressed(). Unit tests will
// crash without mocking.
MOCK_METHOD0(CloseBubble, void());
void CloseBubble() override {}
};
} // namespace
......@@ -52,22 +51,18 @@ MATCHER_P(AppEquals, app, "") {
return app->name == arg.name;
}
class SharingDialogViewTest : public ChromeViewsTestBase {
class SharingDialogViewTest : public BrowserWithTestWindowTest {
protected:
void SetUp() override {
ChromeViewsTestBase::SetUp();
// Create an anchor for the bubble.
views::Widget::InitParams params =
CreateParams(views::Widget::InitParams::TYPE_WINDOW);
params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
anchor_widget_ = std::make_unique<views::Widget>();
anchor_widget_->Init(std::move(params));
}
void TearDown() override {
anchor_widget_.reset();
ChromeViewsTestBase::TearDown();
BrowserWithTestWindowTest::SetUp();
// We create |web_contents| to have a valid commited page origin to check
// against when showing the origin view.
GURL url("https://google.com");
web_contents_ = browser()->OpenURL(content::OpenURLParams(
url, content::Referrer(), WindowOpenDisposition::CURRENT_TAB,
ui::PAGE_TRANSITION_TYPED, false));
CommitPendingLoad(&web_contents_->GetController());
}
std::vector<std::unique_ptr<syncer::DeviceInfo>> CreateDevices(int count) {
......@@ -98,9 +93,8 @@ class SharingDialogViewTest : public ChromeViewsTestBase {
std::unique_ptr<SharingDialogView> CreateDialogView(
SharingDialogData dialog_data) {
auto dialog = std::make_unique<SharingDialogViewMock>(
anchor_widget_->GetContentsView(), /*web_contents=*/nullptr,
std::move(dialog_data));
auto dialog = std::make_unique<SharingDialogViewFake>(
/*anchor_view=*/nullptr, web_contents_, std::move(dialog_data));
dialog->Init();
return dialog;
}
......@@ -123,23 +117,25 @@ class SharingDialogViewTest : public ChromeViewsTestBase {
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES;
data.help_link_text_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_TROUBLESHOOT_LINK;
data.origin_text_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_INITIATING_ORIGIN;
data.help_callback = base::BindLambdaForTesting(
[&](SharingDialogType type) { help_callback.Call(type); });
[&](SharingDialogType type) { help_callback_.Call(type); });
data.device_callback =
base::BindLambdaForTesting([&](const syncer::DeviceInfo& device) {
device_callback.Call(device);
device_callback_.Call(device);
});
data.app_callback = base::BindLambdaForTesting(
[&](const SharingApp& app) { app_callback.Call(app); });
[&](const SharingApp& app) { app_callback_.Call(app); });
return data;
}
std::unique_ptr<views::Widget> anchor_widget_;
testing::MockFunction<void(SharingDialogType)> help_callback;
testing::MockFunction<void(const syncer::DeviceInfo&)> device_callback;
testing::MockFunction<void(const SharingApp&)> app_callback;
testing::MockFunction<void(SharingDialogType)> help_callback_;
testing::MockFunction<void(const syncer::DeviceInfo&)> device_callback_;
testing::MockFunction<void(const SharingApp&)> app_callback_;
content::WebContents* web_contents_ = nullptr;
};
TEST_F(SharingDialogViewTest, PopulateDialogView) {
......@@ -157,7 +153,7 @@ TEST_F(SharingDialogViewTest, DevicePressed) {
/*last_updated_timestamp=*/base::Time::Now(),
/*send_tab_to_self_receiving_enabled=*/false,
/*sharing_info=*/base::nullopt);
EXPECT_CALL(device_callback, Call(DeviceEquals(&device_info)));
EXPECT_CALL(device_callback_, Call(DeviceEquals(&device_info)));
auto dialog_data = CreateDialogData(/*devices=*/3, /*apps=*/2);
auto dialog = CreateDialogView(std::move(dialog_data));
......@@ -172,7 +168,7 @@ TEST_F(SharingDialogViewTest, DevicePressed) {
TEST_F(SharingDialogViewTest, AppPressed) {
SharingApp app(&vector_icons::kOpenInNewIcon, gfx::Image(),
base::UTF8ToUTF16("app0"), std::string());
EXPECT_CALL(app_callback, Call(AppEquals(&app)));
EXPECT_CALL(app_callback_, Call(AppEquals(&app)));
auto dialog_data = CreateDialogData(/*devices=*/3, /*apps=*/2);
auto dialog = CreateDialogView(std::move(dialog_data));
......@@ -186,7 +182,7 @@ TEST_F(SharingDialogViewTest, AppPressed) {
}
TEST_F(SharingDialogViewTest, HelpTextClickedEmpty) {
EXPECT_CALL(help_callback, Call(SharingDialogType::kEducationalDialog));
EXPECT_CALL(help_callback_, Call(SharingDialogType::kEducationalDialog));
auto dialog_data = CreateDialogData(/*devices=*/0, /*apps=*/0);
auto dialog = CreateDialogView(std::move(dialog_data));
......@@ -196,7 +192,7 @@ TEST_F(SharingDialogViewTest, HelpTextClickedEmpty) {
}
TEST_F(SharingDialogViewTest, HelpTextClickedOnlyApps) {
EXPECT_CALL(help_callback,
EXPECT_CALL(help_callback_,
Call(SharingDialogType::kDialogWithoutDevicesWithApp));
auto dialog_data = CreateDialogData(/*devices=*/0, /*apps=*/1);
......@@ -217,53 +213,22 @@ TEST_F(SharingDialogViewTest, ThemeChangedEmptyList) {
dialog->OnThemeChanged();
}
TEST_F(SharingDialogViewTest, OriginViewShown) {
auto dialog_data = CreateDialogData(/*devices=*/1, /*apps=*/1);
dialog_data.origin_text_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_INITIATING_ORIGIN;
dialog_data.initiating_origin =
url::Origin::Create(GURL("https://example.com"));
views::test::WidgetTest::WidgetAutoclosePtr bubble_widget(
views::BubbleDialogDelegateView::CreateBubble(
CreateDialogView(std::move(dialog_data)).release()));
auto* frame_view = static_cast<views::BubbleFrameView*>(
bubble_widget->non_client_view()->frame_view());
EXPECT_NE(nullptr, frame_view->GetHeaderViewForTesting());
}
TEST_F(SharingDialogViewTest, OriginViewHiddenIfNoOrigin) {
TEST_F(SharingDialogViewTest, OriginView) {
auto dialog_data = CreateDialogData(/*devices=*/1, /*apps=*/1);
dialog_data.origin_text_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_INITIATING_ORIGIN;
// Do not set an origin.
dialog_data.initiating_origin = base::nullopt;
views::test::WidgetTest::WidgetAutoclosePtr bubble_widget(
views::BubbleDialogDelegateView::CreateBubble(
CreateDialogView(std::move(dialog_data)).release()));
auto* frame_view = static_cast<views::BubbleFrameView*>(
bubble_widget->non_client_view()->frame_view());
EXPECT_EQ(nullptr, frame_view->GetHeaderViewForTesting());
}
auto dialog = CreateDialogView(std::move(dialog_data));
const int children_without_origin = dialog->children().size();
TEST_F(SharingDialogViewTest, OriginViewHiddenIfNoOriginText) {
auto dialog_data = CreateDialogData(/*devices=*/1, /*apps=*/1);
// Do not set an origin text.
dialog_data.origin_text_id = 0;
dialog_data = CreateDialogData(/*devices=*/1, /*apps=*/1);
dialog_data.initiating_origin =
url::Origin::Create(GURL("https://example.com"));
dialog = CreateDialogView(std::move(dialog_data));
const int children_with_origin = dialog->children().size();
EXPECT_EQ(children_without_origin + 1, children_with_origin);
views::test::WidgetTest::WidgetAutoclosePtr bubble_widget(
views::BubbleDialogDelegateView::CreateBubble(
CreateDialogView(std::move(dialog_data)).release()));
auto* frame_view = static_cast<views::BubbleFrameView*>(
bubble_widget->non_client_view()->frame_view());
EXPECT_EQ(nullptr, frame_view->GetHeaderViewForTesting());
dialog_data = CreateDialogData(/*devices=*/1, /*apps=*/1);
dialog_data.initiating_origin =
url::Origin::Create(GURL("https://google.com"));
dialog = CreateDialogView(std::move(dialog_data));
const int children_with_same_origin = dialog->children().size();
EXPECT_EQ(children_without_origin, children_with_same_origin);
}
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