Skip to content

Commit

Permalink
[Payments] Fix invisible overflowed legal message in Migration dialog
Browse files Browse the repository at this point in the history
The BoxLayout that contains the legal messages improperly uses the
horizontal orientation (the default orientation), but the parent does
not have enough width. As a result, long legal messages are clipped out.

This CL fixes it by using vertical orientation. Also added a pixel test.

Screenshot: https://screenshot.googleplex.com/7VPPY8pHrNRNYYB

(cherry picked from commit f4df959)

Bug: 1306662
Change-Id: I560b867699cf2f009c1273653d067f83ccd1a2c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3600416
Auto-Submit: Keren Zhu <kerenzhu@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#995340}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3606198
Reviewed-by: Keren Zhu <kerenzhu@chromium.org>
Commit-Queue: Siyu An <siyua@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/branch-heads/5005@{#204}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
  • Loading branch information
naeioi authored and Chromium LUCI CQ committed Apr 27, 2022
1 parent 11fada6 commit 3638d12
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 5 deletions.
Expand Up @@ -32,6 +32,7 @@
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/location_bar/location_bar.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/test/test_browser_dialog.h"
#include "chrome/browser/ui/views/autofill/payments/dialog_view_ids.h"
#include "chrome/browser/ui/views/autofill/payments/local_card_migration_bubble_views.h"
#include "chrome/browser/ui/views/autofill/payments/local_card_migration_dialog_view.h"
Expand Down Expand Up @@ -114,6 +115,12 @@ constexpr char kResponseGetUploadDetailsSuccess[] =
"link: "
"{0}.\",\"template_parameter\":[{\"display_text\":\"Link\",\"url\":\"https:"
"//www.example.com/\"}]}]},\"context_token\":\"dummy_context_token\"}";
constexpr char kResponseGetUploadDetailsSuccessLong[] =
"{\"legal_message\":{\"line\":[{\"template\":\"Long message 1 long message "
"2 long message 3 long message 4 long message 5 long message 6 long "
"message 7 long message 8 long message 9 long message 10 long message 11 "
"long message 12 long message 13 long message "
"14\"}]},\"context_token\":\"dummy_context_token\"}";
constexpr char kResponseGetUploadDetailsFailure[] =
"{\"error\":{\"code\":\"FAILED_PRECONDITION\",\"user_error_message\":\"An "
"unexpected error has occurred. Please try again later.\"}}";
Expand Down Expand Up @@ -543,6 +550,29 @@ class LocalCardMigrationBrowserTestForStatusChip
base::test::ScopedFeatureList feature_list_;
};

class LocalCardMigrationBrowserUiTest
: public SupportsTestDialog<LocalCardMigrationBrowserTest> {
public:
LocalCardMigrationBrowserUiTest(const LocalCardMigrationBrowserUiTest&) =
delete;
LocalCardMigrationBrowserUiTest& operator=(
const LocalCardMigrationBrowserUiTest&) = delete;

protected:
LocalCardMigrationBrowserUiTest() = default;
~LocalCardMigrationBrowserUiTest() override = default;

// SupportsTestDialog:
void ShowUi(const std::string& name) override {
test_url_loader_factory()->AddResponse(
kURLGetUploadDetailsRequest, kResponseGetUploadDetailsSuccessLong);
SaveLocalCard(kFirstCardNumber);
SaveLocalCard(kSecondCardNumber);
UseCardAndWaitForMigrationOffer(kFirstCardNumber);
ClickOnOkButton(GetLocalCardMigrationOfferBubbleViews());
}
};

// Ensures that migration is not offered when user saves a new card.
IN_PROC_BROWSER_TEST_F(LocalCardMigrationBrowserTest,
UsingNewCardDoesNotShowIntermediateMigrationOffer) {
Expand Down Expand Up @@ -1230,6 +1260,10 @@ IN_PROC_BROWSER_TEST_F(LocalCardMigrationBrowserTest, CardIdentifierString) {
first_card.NicknameAndLastFourDigitsForTesting());
}

IN_PROC_BROWSER_TEST_F(LocalCardMigrationBrowserUiTest, InvokeUi_default) {
ShowAndVerifyUi();
}

// TODO(crbug.com/897998):
// - Add more tests for feedback dialog.

Expand Down
Expand Up @@ -166,12 +166,8 @@ LegalMessageView::LegalMessageView(const LegalMessageLines& legal_message_lines,
SetBetweenChildSpacing(ChromeLayoutProvider::Get()->GetDistanceMetric(
DISTANCE_RELATED_CONTROL_VERTICAL_SMALL));
for (const LegalMessageLine& line : legal_message_lines) {
// To ensure spacing between child paragraphs, all legal message lines are
// enclosed in their own view.
auto* line_view_ptr =
AddChildView(std::make_unique<views::BoxLayoutView>());
views::StyledLabel* label =
line_view_ptr->AddChildView(std::make_unique<views::StyledLabel>());
AddChildView(std::make_unique<views::StyledLabel>());
label->SetText(line.text());
label->SetTextContext(views::style::CONTEXT_DIALOG_BODY_TEXT);
label->SetDefaultTextStyle(views::style::STYLE_SECONDARY);
Expand Down
1 change: 1 addition & 0 deletions testing/buildbot/filters/pixel_browser_tests.filter
Expand Up @@ -24,6 +24,7 @@ HatsBubbleTest.*
HungRendererDialogViewBrowserTest.*
ImportLockDialogViewBrowserTest.*
InlineLoginHelperBrowserTest.InvokeUi_*
LocalCardMigrationBrowserUiTest.*
NewTabPageTest.*
OneTimePermissionPromptBubbleViewBrowserTest.*
OutdatedUpgradeBubbleTest.*
Expand Down

0 comments on commit 3638d12

Please sign in to comment.