Read later: Add menu close button.

Screenshot can be found in bug.

Bug: 1146024
Change-Id: If5d7977068051b8c96e4c36104655580d6bde29e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2510410Reviewed-by: Thomas Lukaszewicz <tluk@chromium.org>
Reviewed-by: Connie Wan <connily@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Commit-Queue: Caroline Rising <corising@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824641}

chrome/browser/resources/read_later/BUILD.gn

@@ -97,6 +97,7 @@ js_library("app") {
     ":read_later_api_proxy",
     ":read_later_item",
     "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
+    "//ui/webui/resources/cr_elements/cr_icon_button:cr_icon_button.m",
   ]
 }
 

chrome/browser/resources/read_later/app.html

 <style include="mwb-shared-style">
+  #top-container {
+    display: flex;
+  }
+
   #header {
     align-items: center;
     display: flex;
+    flex-grow: 1;
     font-size: 15px;
     height: var(--mwb-item-height);
     margin: 0;
     padding-inline-start: var(--mwb-list-item-horizontal-margin);
   }
 
+  cr-icon-button {
+    border-radius: 50%;
+    margin-inline-end: 4px;
+    margin-top: 4px;
+    --cr-icon-button-fill-color: var(--google-grey-refresh-700);
+    --cr-icon-button-icon-size: var(--mwb-icon-size);
+    --cr-icon-button-size: 24px;
+  }
+
+  cr-icon-button:hover {
+    background-color: rgba(var(--google-grey-900-rgb), .1);
+  }
+
   #read-later-list {
     max-height: 444px;
     overflow-x: hidden;
@@ -26,7 +44,13 @@
   }
 </style>
 
-<div id="header">$i18n{title}</div>
+<div id="top-container">
+  <div id="header">$i18n{title}</div>
+  <cr-icon-button id="closeButton" aria-label="$i18n{tooltipClose}"
+      disable-ripple iron-icon="cr:close" on-click="onCloseClick_"
+      title="$i18n{tooltipClose}">
+  </cr-icon-button>
+</div>
 <div id="read-later-list">
   <div class="sub-heading">$i18n{unreadHeader}</div>
   <template id="ureadItemsList" is="dom-repeat" items="[[unreadItems_]]">

chrome/browser/resources/read_later/app.js

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.m.js';
 import 'chrome://resources/cr_elements/shared_vars_css.m.js';
 import 'chrome://resources/cr_elements/mwb_shared_style.js';
 import 'chrome://resources/cr_elements/mwb_shared_vars.js';
@@ -74,6 +75,15 @@ export class ReadLaterAppElement extends PolymerElement {
       this.readItems_ = entries.readEntries;
     });
   }
+
+  /**
+   * @param {!Event} e
+   * @private
+   */
+  onCloseClick_(e) {
+    e.stopPropagation();
+    this.apiProxy_.closeUI();
+  }
 }
 
 customElements.define(ReadLaterAppElement.is, ReadLaterAppElement);

chrome/browser/resources/read_later/read_later_api_proxy.js

@@ -30,6 +30,8 @@ export class ReadLaterApiProxy {
 
   showUI() {}
 
+  closeUI() {}
+
   /** @return {!readLater.mojom.PageCallbackRouter} */
   getCallbackRouter() {}
 }
@@ -74,6 +76,11 @@ export class ReadLaterApiProxyImpl {
     this.handler.showUI();
   }
 
+  /** @override */
+  closeUI() {
+    this.handler.closeUI();
+  }
+
   /** @override */
   getCallbackRouter() {
     return this.callbackRouter;

chrome/browser/ui/views/bubble/webui_bubble_dialog_view.cc

@@ -70,6 +70,12 @@ void WebUIBubbleDialogView::ShowUI() {
   web_view_->GetWebContents()->Focus();
 }
 
+void WebUIBubbleDialogView::CloseUI() {
+  DCHECK(GetWidget());
+  GetWidget()->CloseWithReason(
+      views::Widget::ClosedReason::kCloseButtonClicked);
+}
+
 void WebUIBubbleDialogView::OnWebViewSizeChanged() {
   SizeToContents();
 }

chrome/browser/ui/views/bubble/webui_bubble_dialog_view.h

@@ -31,6 +31,7 @@ class WebUIBubbleDialogView : public views::BubbleDialogDelegateView,
 
   // WebUIBubbleView::Host:
   void ShowUI() override;
+  void CloseUI() override;
   void OnWebViewSizeChanged() override;
 
  private:

chrome/browser/ui/views/bubble/webui_bubble_view.cc

@@ -73,3 +73,8 @@ void WebUIBubbleView::ShowUI() {
   if (host_)
     host_->ShowUI();
 }
+
+void WebUIBubbleView::CloseUI() {
+  if (host_)
+    host_->CloseUI();
+}

chrome/browser/ui/views/bubble/webui_bubble_view.h

@@ -26,6 +26,7 @@ class WebUIBubbleView : public views::WebView,
   class Host {
    public:
     virtual void ShowUI() = 0;
+    virtual void CloseUI() = 0;
     virtual void OnWebViewSizeChanged() = 0;
   };
 
@@ -63,6 +64,7 @@ class WebUIBubbleView : public views::WebView,
 
   // MojoBubbleWebUIController::Embedder:
   void ShowUI() override;
+  void CloseUI() override;
 
  private:
   // |host_| does not always have to be set. The WebUIBubbleView can be cached

chrome/browser/ui/views/bubble/webui_bubble_view_unittest.cc

@@ -27,13 +27,16 @@ namespace {
 class MockHost : public WebUIBubbleView::Host {
  public:
   void ShowUI() override { ++show_ui_called_; }
+  void CloseUI() override { ++close_ui_called_; }
   void OnWebViewSizeChanged() override { ++size_changed_called_; }
 
   int show_ui_called() const { return show_ui_called_; }
+  int close_ui_called() const { return close_ui_called_; }
   int size_changed_called() const { return size_changed_called_; }
 
  private:
   int show_ui_called_ = 0;
+  int close_ui_called_ = 0;
   int size_changed_called_ = 0;
 };
 
@@ -118,5 +121,11 @@ TEST_F(WebUIBubbleViewTest, ShowUINotifiesHost) {
   EXPECT_EQ(1, host().show_ui_called());
 }
 
+TEST_F(WebUIBubbleViewTest, CloseUINotifiesHost) {
+  EXPECT_EQ(0, host().close_ui_called());
+  web_view()->CloseUI();
+  EXPECT_EQ(1, host().close_ui_called());
+}
+
 }  // namespace test
 }  // namespace views

chrome/browser/ui/webui/read_later/read_later.mojom

@@ -46,6 +46,9 @@ interface PageHandler {
 
   // Notify the backend that the UI is ready to be shown.
   ShowUI();
+
+  // Notify the backend that the dialog should be closed.
+  CloseUI();
 };
 
 // WebUI-side handler for requests from the browser.

chrome/browser/ui/webui/read_later/read_later_page_handler.cc

@@ -102,6 +102,12 @@ void ReadLaterPageHandler::ShowUI() {
     embedder->ShowUI();
 }
 
+void ReadLaterPageHandler::CloseUI() {
+  auto embedder = read_later_ui_->embedder();
+  if (embedder)
+    embedder->CloseUI();
+}
+
 read_later::mojom::ReadLaterEntryPtr ReadLaterPageHandler::GetEntryData(
     const ReadingListEntry* entry) {
   auto entry_data = read_later::mojom::ReadLaterEntry::New();

chrome/browser/ui/webui/read_later/read_later_page_handler.h

@@ -39,6 +39,7 @@ class ReadLaterPageHandler : public read_later::mojom::PageHandler {
   void UpdateReadStatus(const GURL& url, bool read) override;
   void RemoveEntry(const GURL& url) override;
   void ShowUI() override;
+  void CloseUI() override;
 
  private:
   // Gets the reading list entry data used for displaying to the user and

chrome/browser/ui/webui/read_later/read_later_ui.cc

@@ -37,6 +37,7 @@ ReadLaterUI::ReadLaterUI(content::WebUI* web_ui)
   static constexpr webui::LocalizedString kLocalizedStrings[] = {
       {"readHeader", IDS_READ_LATER_MENU_READ_HEADER},
       {"title", IDS_READ_LATER_TITLE},
+      {"tooltipClose", IDS_CLOSE},
       {"tooltipDelete", IDS_DELETE},
       {"tooltipMarkAsRead", IDS_READ_LATER_MENU_TOOLTIP_MARK_AS_READ},
       {"tooltipMarkAsUnread", IDS_READ_LATER_MENU_TOOLTIP_MARK_AS_UNREAD},

chrome/test/data/webui/read_later/read_later_app_test.js

@@ -145,4 +145,11 @@ suite('ReadLaterAppTest', () => {
     const url = await testProxy.whenCalled('removeEntry');
     assertEquals(expectedUrl, url);
   });
+
+  test('Click on menu button triggers actions', async () => {
+    const readLaterCloseButton =
+        readLaterApp.shadowRoot.querySelector('#closeButton');
+    readLaterCloseButton.click();
+    await testProxy.whenCalled('closeUI');
+  });
 });

chrome/test/data/webui/read_later/test_read_later_api_proxy.js

@@ -16,6 +16,7 @@ export class TestReadLaterApiProxy extends TestBrowserProxy {
       'updateReadStatus',
       'removeEntry',
       'showUI',
+      'closeUI',
     ]);
 
     /** @type {!readLater.mojom.PageCallbackRouter} */
@@ -51,6 +52,11 @@ export class TestReadLaterApiProxy extends TestBrowserProxy {
     this.methodCalled('showUI');
   }
 
+  /** @override */
+  closeUI() {
+    this.methodCalled('closeUI');
+  }
+
   /** @override */
   getCallbackRouter() {
     return this.callbackRouter;

ui/webui/mojo_bubble_web_ui_controller.h

@@ -20,6 +20,7 @@ class MojoBubbleWebUIController : public MojoWebUIController {
   class Embedder {
    public:
     virtual void ShowUI() = 0;
+    virtual void CloseUI() = 0;
   };
 
   // By default MojoBubbleWebUIController do not have normal WebUI bindings.