Skip to content

Commit

Permalink
Fix Accessibility Issues for Migration UIs
Browse files Browse the repository at this point in the history
1) Associate card network and card number to checkbox.
2) Add accessible text for Google Pay logo and tooltip text for
all credit card images.
3) Make migration bubble sticky when first shown.

!!!: The problem that checkbox reads "unchecked" even if it is
actually checked has not been fixed yet. Has
filed bug 889602. Would like to submit this CL first to unblock
the browser-tests CL.

Bug: 852904
Change-Id: I48ea4a92e7d95327be432bb10126e8338496be97
Reviewed-on: https://chromium-review.googlesource.com/1244888
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Siyu An <siyua@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595231}
  • Loading branch information
siyua authored and Commit Bot committed Sep 28, 2018
1 parent 39d0a67 commit 20ce298
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ void LocalCardMigrationBubbleControllerImpl::ShowBubbleImplementation() {
Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
local_card_migration_bubble_ =
browser->window()->ShowLocalCardMigrationBubble(web_contents(), this,
true);
is_reshow_);
DCHECK(local_card_migration_bubble_);
UpdateIcon();
timer_.reset(new base::ElapsedTimer());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ void LocalCardMigrationBubbleViews::AddedToWidget() {
kMigrationBubbleGooglePayLogoHeight);
views::ImageView* icon_view = new views::ImageView();
icon_view->SetImage(&image);
icon_view->SetAccessibleName(
l10n_util::GetStringUTF16(IDS_AUTOFILL_GOOGLE_PAY_LOGO_ACCESSIBLE_NAME));
title_container->AddChildView(icon_view);
GetBubbleFrameView()->SetTitleView(std::move(title_container));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ void LocalCardMigrationDialogView::Init() {
std::unique_ptr<views::ImageView> image =
std::make_unique<views::ImageView>();
image->SetImage(rb.GetImageSkiaNamed(GetHeaderImageId()));
image->SetAccessibleName(
l10n_util::GetStringUTF16(IDS_AUTOFILL_GOOGLE_PAY_LOGO_ACCESSIBLE_NAME));
image_container->AddChildView(image.release());
main_container->AddChildView(image_container.release());

Expand Down
12 changes: 7 additions & 5 deletions chrome/browser/ui/views/autofill/migratable_card_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,14 @@ void MigratableCardView::Init(
checkbox_ = new views::Checkbox(base::string16(), listener);
checkbox_->SetChecked(true);
checkbox_->set_tag(card_index);
checkbox_->SetVisible(true);
// TODO(crbug/867194): Currently the ink drop animation circle is cut by the
// border of scroll bar view. Find a way to adjust the format.
checkbox_->SetInkDropMode(views::InkDropHostView::InkDropMode::OFF);
std::unique_ptr<views::Label> card_description =
std::make_unique<views::Label>(
migratable_credit_card.credit_card().NetworkAndLastFourDigits(),
views::style::CONTEXT_LABEL);
checkbox_->SetAssociatedLabel(card_description.get());
AddChildView(checkbox_);

constexpr int kMigrationResultImageSize = 16;
Expand Down Expand Up @@ -116,12 +120,10 @@ void MigratableCardView::Init(
rb.GetImageNamed(CreditCard::IconResourceId(
migratable_credit_card.credit_card().network()))
.AsImageSkia());
card_image->SetAccessibleName(
migratable_credit_card.credit_card().NetworkForDisplay());
card_network_and_last_four_digits->AddChildView(card_image.release());

std::unique_ptr<views::Label> card_description =
std::make_unique<views::Label>(
migratable_credit_card.credit_card().NetworkAndLastFourDigits(),
views::style::CONTEXT_LABEL);
card_network_and_last_four_digits->AddChildView(card_description.release());

std::unique_ptr<views::Label> card_expiration =
Expand Down
3 changes: 3 additions & 0 deletions components/autofill_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,9 @@

<!-- Autofill Local card migration bubble or dialog -->
<if expr="not is_ios and not is_android">
<message name="IDS_AUTOFILL_GOOGLE_PAY_LOGO_ACCESSIBLE_NAME" desc="The accessible name for the Google Pay logo in the local card migration bubble or dialog.">
Google Pay logo
</message>
<message name="IDS_AUTOFILL_LOCAL_CARD_MIGRATION_BUBBLE_TITLE" desc="The title text for a bubble that offers users to start the process of migrating local cards to cloud.">
Save all your cards in one place?
</message>
Expand Down
13 changes: 12 additions & 1 deletion ui/views/controls/image_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,17 @@ void ImageView::OnPaint(gfx::Canvas* canvas) {
OnPaintImage(canvas);
}

void ImageView::SetAccessibleName(const base::string16& accessible_name) {
accessible_name_ = accessible_name;
}

base::string16 ImageView::GetAccessibleName() const {
return accessible_name_;
}

void ImageView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->role = ax::mojom::Role::kImage;
node_data->SetName(tooltip_text_);
node_data->SetName(accessible_name_);
}

const char* ImageView::GetClassName() const {
Expand Down Expand Up @@ -169,8 +177,11 @@ ImageView::Alignment ImageView::GetVerticalAlignment() const {
return vertical_alignment_;
}

// TODO(crbug.com/890465): Update the duplicate code here and in views::Button.
void ImageView::SetTooltipText(const base::string16& tooltip) {
tooltip_text_ = tooltip;
if (accessible_name_.empty())
accessible_name_ = tooltip_text_;
}

base::string16 ImageView::GetTooltipText() const {
Expand Down
7 changes: 7 additions & 0 deletions ui/views/controls/image_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ class VIEWS_EXPORT ImageView : public View {
void SetTooltipText(const base::string16& tooltip);
base::string16 GetTooltipText() const;

// Set / Get the accessible name text.
void SetAccessibleName(const base::string16& name);
base::string16 GetAccessibleName() const;

// Overriden from View:
void OnPaint(gfx::Canvas* canvas) override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
Expand Down Expand Up @@ -120,6 +124,9 @@ class VIEWS_EXPORT ImageView : public View {
// The current tooltip text.
base::string16 tooltip_text_;

// The current accessible name text.
base::string16 accessible_name_;

// Scale last painted at.
float last_paint_scale_;

Expand Down

0 comments on commit 20ce298

Please sign in to comment.