From 125613754be8e61fa24a5cdda83a3ff8b1eb5119 Mon Sep 17 00:00:00 2001 From: Kevin Kuehler Date: Sat, 10 Jul 2021 10:29:23 -0700 Subject: [PATCH] speedreader: icon: Fix accessible text In 85a5f8b we removed the label from the Speedreader icon. To make up for this, we need more descriptive accessible text. This is taken from the figma document linked in the issue. Resolves https://github.com/brave/brave-browser/issues/16881 --- app/brave_generated_resources.grd | 9 ++++ browser/speedreader/speedreader_tab_helper.h | 3 ++ .../speedreader/speedreader_icon_view.cc | 50 +++++++++++-------- .../views/speedreader/speedreader_icon_view.h | 5 ++ 4 files changed, 45 insertions(+), 22 deletions(-) diff --git a/app/brave_generated_resources.grd b/app/brave_generated_resources.grd index cd2e08eb564..009f2e5a473 100644 --- a/app/brave_generated_resources.grd +++ b/app/brave_generated_resources.grd @@ -1189,6 +1189,15 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U in Speedreader + + Turn off reader mode + + + Turn on reader mode + + + Speedreader settings + diff --git a/browser/speedreader/speedreader_tab_helper.h b/browser/speedreader/speedreader_tab_helper.h index 31f18cfd879..52c1f33b127 100644 --- a/browser/speedreader/speedreader_tab_helper.h +++ b/browser/speedreader/speedreader_tab_helper.h @@ -24,6 +24,9 @@ class SpeedreaderTabHelper public content::WebContentsUserData { public: enum class DistillState { + // Used as an initialization state + kUnknown, + // The web contents is not distilled kNone, diff --git a/browser/ui/views/speedreader/speedreader_icon_view.cc b/browser/ui/views/speedreader/speedreader_icon_view.cc index e2aa5543102..029d1a414ad 100644 --- a/browser/ui/views/speedreader/speedreader_icon_view.cc +++ b/browser/ui/views/speedreader/speedreader_icon_view.cc @@ -27,8 +27,6 @@ #include "ui/views/animation/ink_drop_host_view.h" #include "ui/views/animation/ink_drop_state.h" -using DistillState = speedreader::SpeedreaderTabHelper::DistillState; - SpeedreaderIconView::SpeedreaderIconView( CommandUpdater* command_updater, IconLabelBubbleView::Delegate* icon_label_bubble_delegate, @@ -58,15 +56,8 @@ void SpeedreaderIconView::UpdateImpl() { if (ink_drop()->GetHighlighted() && !IsBubbleShowing()) ink_drop()->AnimateToState(views::InkDropState::HIDDEN, nullptr); - auto* tab_helper = - speedreader::SpeedreaderTabHelper::FromWebContents(contents); - if (!tab_helper) { - SetVisible(false); - return; - } - const ui::ThemeProvider* theme_provider = GetThemeProvider(); - const DistillState state = tab_helper->PageDistillState(); + const DistillState state = GetDistillState(); const bool is_distilled = speedreader::SpeedreaderTabHelper::PageStateIsDistilled(state); @@ -99,16 +90,7 @@ void SpeedreaderIconView::UpdateImpl() { } const gfx::VectorIcon& SpeedreaderIconView::GetVectorIcon() const { - auto* web_contents = GetWebContents(); - if (!web_contents) - return kBraveReaderModeIcon; - - auto* tab_helper = - speedreader::SpeedreaderTabHelper::FromWebContents(web_contents); - if (!tab_helper) - return kBraveReaderModeIcon; - - const DistillState state = tab_helper->PageDistillState(); + const DistillState state = GetDistillState(); if (state == DistillState::kSpeedreaderMode || state == DistillState::kSpeedreaderOnDisabledPage) { return kBraveSpeedreaderModeIcon; @@ -118,8 +100,20 @@ const gfx::VectorIcon& SpeedreaderIconView::GetVectorIcon() const { } std::u16string SpeedreaderIconView::GetTextForTooltipAndAccessibleName() const { - return l10n_util::GetStringUTF16(GetActive() ? IDS_EXIT_DISTILLED_PAGE - : IDS_DISTILL_PAGE); + int id; + const DistillState state = GetDistillState(); + switch (state) { + case DistillState::kSpeedreaderMode: + case DistillState::kSpeedreaderOnDisabledPage: + id = IDS_SPEEDREADER_ICON_SPEEDREADER_SETTINGS; + break; + case DistillState::kReaderMode: + id = IDS_SPEEDREADER_ICON_TURN_OFF_READER_MODE; + break; + default: + id = IDS_SPEEDREADER_ICON_TURN_ON_READER_MODE; + } + return l10n_util::GetStringUTF16(id); } void SpeedreaderIconView::OnExecuting( @@ -139,5 +133,17 @@ views::BubbleDialogDelegate* SpeedreaderIconView::GetBubble() const { tab_helper->speedreader_bubble_view()); } +DistillState SpeedreaderIconView::GetDistillState() const { + DistillState state = DistillState::kUnknown; + auto* web_contents = GetWebContents(); + if (web_contents) { + auto* tab_helper = + speedreader::SpeedreaderTabHelper::FromWebContents(web_contents); + if (tab_helper) + state = tab_helper->PageDistillState(); + } + return state; +} + BEGIN_METADATA(SpeedreaderIconView, PageActionIconView) END_METADATA diff --git a/browser/ui/views/speedreader/speedreader_icon_view.h b/browser/ui/views/speedreader/speedreader_icon_view.h index 3b85f013cbe..5a31953af08 100644 --- a/browser/ui/views/speedreader/speedreader_icon_view.h +++ b/browser/ui/views/speedreader/speedreader_icon_view.h @@ -6,10 +6,13 @@ #ifndef BRAVE_BROWSER_UI_VIEWS_SPEEDREADER_SPEEDREADER_ICON_VIEW_H_ #define BRAVE_BROWSER_UI_VIEWS_SPEEDREADER_SPEEDREADER_ICON_VIEW_H_ +#include "brave/browser/speedreader/speedreader_tab_helper.h" #include "chrome/browser/ui/views/page_action/page_action_icon_view.h" #include "content/public/browser/web_contents_observer.h" #include "ui/base/metadata/metadata_header_macros.h" +using DistillState = speedreader::SpeedreaderTabHelper::DistillState; + namespace content { class NavigationHandle; } // namespace content @@ -34,6 +37,8 @@ class SpeedreaderIconView : public PageActionIconView { views::BubbleDialogDelegate* GetBubble() const override; std::u16string GetTextForTooltipAndAccessibleName() const override; void UpdateImpl() override; + private: + DistillState GetDistillState() const; }; #endif // BRAVE_BROWSER_UI_VIEWS_SPEEDREADER_SPEEDREADER_ICON_VIEW_H_