Skip to content

Commit

Permalink
Add Read Later omnibox star entry point.
Browse files Browse the repository at this point in the history
Add a new menu triggered by the StarView which gives the option to
either bookmark the tab or save it for later. If bookmark is selected
the user flow matches previous behavior. If save to read later is
selected the tab will be saved to the reading list model. Note this
change is hidden behind the read later feature flag.

Follow-up work: Update the read later icon in the menu when new icons
are available. Update the omnibox star tooltip. Update StarView/bookmark metrics and
add metrics for read later. Update menu size.

Bug: 1117023
Change-Id: I09c6cf4973338faa272bc6ec6da06e26f0f9aa8f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359550
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: Connie Wan <connily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801449}
  • Loading branch information
Caroline Rising authored and Commit Bot committed Aug 25, 2020
1 parent 091a827 commit 36929b5
Show file tree
Hide file tree
Showing 14 changed files with 322 additions and 3 deletions.
14 changes: 14 additions & 0 deletions chrome/app/generated_resources.grd
Expand Up @@ -5652,6 +5652,20 @@ Keep your key file in a safe place. You will need it to create new versions of y
</message>
</if>

<!-- Star View menu -->
<message name="IDS_STAR_VIEW_MENU_ADD_BOOKMARK" desc="The item label of the menu triggered from the star icon in the location bar for adding a bookmark.">
Add Bookmark
</message>
<message name="IDS_STAR_VIEW_MENU_EDIT_BOOKMARK" desc="The item label of the menu triggered from the star icon in the location bar for editing a bookmark.">
Edit Bookmark
</message>
<message name="IDS_STAR_VIEW_MENU_MOVE_TO_READ_LATER" desc="The item label of the menu triggered from the star icon in the location bar for moving the current tab to read later.">
Add to Read later
</message>
<message name="IDS_STAR_VIEW_MENU_MARK_AS_READ" desc="The item label of the menu triggered from the star icon in the location bar for marking the current tab's read later entry as read.">
Mark as Read
</message>

<!--Tooltip strings-->
<message name="IDS_TOOLTIP_BACK" desc="The tooltip for back button">
Click to go back, hold to see history
Expand Down
@@ -0,0 +1 @@
cd19bd38c1c505428e1b952dff29ea2dcc23a749
@@ -0,0 +1 @@
ca95daa02a7ba6b6dcc742fa2359f58058ac8025
@@ -0,0 +1 @@
ca95daa02a7ba6b6dcc742fa2359f58058ac8025
@@ -0,0 +1 @@
cd19bd38c1c505428e1b952dff29ea2dcc23a749
2 changes: 2 additions & 0 deletions chrome/browser/ui/BUILD.gn
Expand Up @@ -3571,6 +3571,8 @@ static_library("ui") {
"views/location_bar/permission_chip.h",
"views/location_bar/selected_keyword_view.cc",
"views/location_bar/selected_keyword_view.h",
"views/location_bar/star_menu_model.cc",
"views/location_bar/star_menu_model.h",
"views/location_bar/star_view.cc",
"views/location_bar/star_view.h",
"views/location_bar/zoom_bubble_view.cc",
Expand Down
63 changes: 63 additions & 0 deletions chrome/browser/ui/browser_commands.cc
Expand Up @@ -57,6 +57,7 @@
#include "chrome/browser/ui/location_bar/location_bar.h"
#include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h"
#include "chrome/browser/ui/qrcode_generator/qrcode_generator_bubble_controller.h"
#include "chrome/browser/ui/read_later/reading_list_model_factory.h"
#include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
#include "chrome/browser/ui/send_tab_to_self/send_tab_to_self_bubble_controller.h"
#include "chrome/browser/ui/status_bubble.h"
Expand Down Expand Up @@ -87,6 +88,8 @@
#include "components/google/core/common/google_util.h"
#include "components/omnibox/browser/omnibox_prefs.h"
#include "components/prefs/pref_service.h"
#include "components/reading_list/core/reading_list_entry.h"
#include "components/reading_list/core/reading_list_model.h"
#include "components/services/app_service/public/mojom/types.mojom.h"
#include "components/sessions/core/live_tab_context.h"
#include "components/sessions/core/tab_restore_service.h"
Expand Down Expand Up @@ -208,6 +211,27 @@ void CreateAndShowNewWindowWithContents(
TabStripModel::ADD_ACTIVE);
}

bool GetActiveTabURLAndTitleToSave(Browser* browser,
GURL* url,
base::string16* title) {
content::WebContents* web_contents =
browser->tab_strip_model()->GetActiveWebContents();
// |web_contents| can be nullptr if the last tab in the browser was closed
// but the browser wasn't closed yet. https://crbug.com/799668
if (!web_contents)
return false;
chrome::GetURLAndTitleToBookmark(web_contents, url, title);
return true;
}

ReadingListModel* GetReadingListModel(Browser* browser) {
ReadingListModel* model =
ReadingListModelFactory::GetForBrowserContext(browser->profile());
if (!model || !model->loaded())
return nullptr; // Ignore requests until model has loaded.
return model;
}

} // namespace

using base::UserMetricsAction;
Expand Down Expand Up @@ -1006,6 +1030,45 @@ bool CanBookmarkAllTabs(const Browser* browser) {
CanBookmarkCurrentTab(browser);
}

bool MoveCurrentTabToReadLater(Browser* browser) {
GURL url;
base::string16 title;
ReadingListModel* model = GetReadingListModel(browser);
if (!model || !GetActiveTabURLAndTitleToSave(browser, &url, &title))
return false;
model->AddEntry(url, base::UTF16ToUTF8(title),
reading_list::EntrySource::ADDED_VIA_CURRENT_APP);
// Close current tab.
int index = browser->tab_strip_model()->active_index();
browser->tab_strip_model()->CloseWebContentsAt(
index, TabStripModel::CLOSE_CREATE_HISTORICAL_TAB |
TabStripModel::CLOSE_USER_GESTURE);
return true;
}

bool MarkCurrentTabAsReadInReadLater(Browser* browser) {
GURL url;
base::string16 title;
ReadingListModel* model = GetReadingListModel(browser);
if (!model || !GetActiveTabURLAndTitleToSave(browser, &url, &title))
return false;
const ReadingListEntry* entry = model->GetEntryByURL(url);
// Mark current tab as read.
if (entry && !entry->IsRead())
model->SetReadStatus(url, true);
return entry != nullptr;
}

bool IsCurrentTabUnreadInReadLater(Browser* browser) {
GURL url;
base::string16 title;
ReadingListModel* model = GetReadingListModel(browser);
if (!model || !GetActiveTabURLAndTitleToSave(browser, &url, &title))
return false;
const ReadingListEntry* entry = model->GetEntryByURL(url);
return entry && !entry->IsRead();
}

void SaveCreditCard(Browser* browser) {
WebContents* web_contents =
browser->tab_strip_model()->GetActiveWebContents();
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ui/browser_commands.h
Expand Up @@ -140,6 +140,9 @@ void BookmarkCurrentTab(Browser* browser);
bool CanBookmarkCurrentTab(const Browser* browser);
void BookmarkAllTabs(Browser* browser);
bool CanBookmarkAllTabs(const Browser* browser);
bool MoveCurrentTabToReadLater(Browser* browser);
bool MarkCurrentTabAsReadInReadLater(Browser* browser);
bool IsCurrentTabUnreadInReadLater(Browser* browser);
void SaveCreditCard(Browser* browser);
void MigrateLocalCards(Browser* browser);
void MaybeShowSaveLocalCardSignInPromo(Browser* browser);
Expand Down
36 changes: 36 additions & 0 deletions chrome/browser/ui/views/location_bar/star_menu_model.cc
@@ -0,0 +1,36 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ui/views/location_bar/star_menu_model.h"

#include "chrome/grit/generated_resources.h"
#include "components/omnibox/browser/vector_icons.h"
#include "components/vector_icons/vector_icons.h"
#include "ui/base/models/image_model.h"

StarMenuModel::StarMenuModel(ui::SimpleMenuModel::Delegate* delegate,
bool bookmarked,
bool exists_as_unread_in_read_later)
: SimpleMenuModel(delegate) {
Build(bookmarked, exists_as_unread_in_read_later);
}

StarMenuModel::~StarMenuModel() = default;

void StarMenuModel::Build(bool bookmarked,
bool exists_as_unread_in_read_later) {
AddItemWithStringIdAndIcon(
CommandBookmark,
bookmarked ? IDS_STAR_VIEW_MENU_EDIT_BOOKMARK
: IDS_STAR_VIEW_MENU_ADD_BOOKMARK,
ui::ImageModel::FromVectorIcon(omnibox::kStarIcon));
// TODO(corising): Replace placeholder folder icon with read-later icon once
// available.
AddItemWithStringIdAndIcon(
exists_as_unread_in_read_later ? CommandMarkAsRead
: CommandMoveToReadLater,
exists_as_unread_in_read_later ? IDS_STAR_VIEW_MENU_MARK_AS_READ
: IDS_STAR_VIEW_MENU_MOVE_TO_READ_LATER,
ui::ImageModel::FromVectorIcon(vector_icons::kFolderIcon));
}
30 changes: 30 additions & 0 deletions chrome/browser/ui/views/location_bar/star_menu_model.h
@@ -0,0 +1,30 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_MENU_MODEL_H_
#define CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_MENU_MODEL_H_

#include "base/macros.h"
#include "ui/base/models/simple_menu_model.h"

class StarMenuModel : public ui::SimpleMenuModel {
public:
StarMenuModel(ui::SimpleMenuModel::Delegate* delegate,
bool bookmarked,
bool exists_as_unread_in_read_later);
StarMenuModel(const StarMenuModel&) = delete;
StarMenuModel& operator=(const StarMenuModel&) = delete;
~StarMenuModel() override;

enum StarMenuCommand {
CommandBookmark,
CommandMoveToReadLater,
CommandMarkAsRead
};

private:
void Build(bool bookmarked, bool exists_as_unread_in_read_later);
};

#endif // CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_MENU_MODEL_H_
43 changes: 41 additions & 2 deletions chrome/browser/ui/views/location_bar/star_view.cc
Expand Up @@ -15,9 +15,11 @@
#include "chrome/browser/ui/bookmarks/bookmark_stats.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/view_ids.h"
#include "chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h"
#include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_view.h"
#include "chrome/browser/ui/views/location_bar/star_menu_model.h"
#include "chrome/grit/generated_resources.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/omnibox/browser/vector_icons.h"
Expand All @@ -27,6 +29,7 @@
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/color_utils.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/controls/menu/menu_runner.h"

StarView::StarView(CommandUpdater* command_updater,
Browser* browser,
Expand All @@ -38,6 +41,7 @@ StarView::StarView(CommandUpdater* command_updater,
page_action_icon_delegate),
browser_(browser) {
DCHECK(browser_);

edit_bookmarks_enabled_.Init(
bookmarks::prefs::kEditBookmarksEnabled, browser_->profile()->GetPrefs(),
base::BindRepeating(&StarView::EditBookmarksPrefUpdated,
Expand Down Expand Up @@ -71,8 +75,19 @@ void StarView::OnExecuting(PageActionIconView::ExecuteSource execute_source) {
}

void StarView::ExecuteCommand(ExecuteSource source) {
OnExecuting(source);
chrome::BookmarkCurrentTab(browser_);
if (base::FeatureList::IsEnabled(features::kReadLater)) {
menu_model_ = std::make_unique<StarMenuModel>(
this, active(), chrome::IsCurrentTabUnreadInReadLater(browser_));
menu_runner_ = std::make_unique<views::MenuRunner>(
menu_model_.get(),
views::MenuRunner::HAS_MNEMONICS | views::MenuRunner::FIXED_ANCHOR);
menu_runner_->RunMenuAt(GetWidget(), nullptr, GetAnchorBoundsInScreen(),
views::MenuAnchorPosition::kTopRight,
ui::MENU_SOURCE_NONE);
} else {
OnExecuting(source);
chrome::BookmarkCurrentTab(browser_);
}
}

views::BubbleDialogDelegate* StarView::GetBubble() const {
Expand All @@ -95,3 +110,27 @@ const char* StarView::GetClassName() const {
void StarView::EditBookmarksPrefUpdated() {
Update();
}

void StarView::ExecuteCommand(int command_id, int event_flags) {
switch (command_id) {
case StarMenuModel::CommandBookmark:
chrome::BookmarkCurrentTab(browser_);
break;
case StarMenuModel::CommandMoveToReadLater:
chrome::MoveCurrentTabToReadLater(browser_);
break;
case StarMenuModel::CommandMarkAsRead:
chrome::MarkCurrentTabAsReadInReadLater(browser_);
break;
default:
NOTREACHED();
}
}

void StarView::MenuClosed(ui::SimpleMenuModel* source) {
if (!GetBubble() || !GetBubble()->GetWidget() ||
!GetBubble()->GetWidget()->IsVisible()) {
SetHighlighted(false);
}
menu_runner_.reset();
}
16 changes: 15 additions & 1 deletion chrome/browser/ui/views/location_bar/star_view.h
Expand Up @@ -5,23 +5,30 @@
#ifndef CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_VIEW_H_
#define CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_VIEW_H_

#include <memory>

#include "base/macros.h"
#include "base/scoped_observer.h"
#include "chrome/browser/ui/views/page_action/page_action_icon_view.h"
#include "components/prefs/pref_member.h"
#include "ui/base/models/simple_menu_model.h"

class Browser;
class CommandUpdater;
class StarMenuModel;

// The star icon to show a bookmark bubble.
class StarView : public PageActionIconView {
class StarView : public PageActionIconView,
public ui::SimpleMenuModel::Delegate {
public:
StarView(CommandUpdater* command_updater,
Browser* browser,
IconLabelBubbleView::Delegate* icon_label_bubble_delegate,
PageActionIconView::Delegate* page_action_icon_delegate);
~StarView() override;

StarMenuModel* menu_model_for_test() { return menu_model_.get(); }

protected:
// PageActionIconView:
void UpdateImpl() override;
Expand All @@ -36,8 +43,15 @@ class StarView : public PageActionIconView {
void EditBookmarksPrefUpdated();
bool IsBookmarkStarHiddenByExtension() const;

// ui::SimpleMenuModel::Delegate:
void ExecuteCommand(int command_id, int event_flags) override;
void MenuClosed(ui::SimpleMenuModel* source) override;

Browser* const browser_;

std::unique_ptr<views::MenuRunner> menu_runner_;
std::unique_ptr<StarMenuModel> menu_model_;

BooleanPrefMember edit_bookmarks_enabled_;

DISALLOW_COPY_AND_ASSIGN(StarView);
Expand Down

0 comments on commit 36929b5

Please sign in to comment.